feat: Update reconciler / reify to use CreateMeshPartAsync to create mesh parts

plugin/src/ChangeBatcher/encodePatchUpdate.lua

@@ -8,28 +8,33 @@ return function(instance, instanceId, properties)
 	local update = {
 		id = instanceId,
 		changedProperties = {},
+		requiresRecreate = false
 	}
 
 	for propertyName in pairs(properties) do
 		if propertyName == "Name" then
 			update.changedName = instance.Name
-		else
-			local descriptor = RbxDom.findCanonicalPropertyDescriptor(instance.ClassName, propertyName)
-
-			if not descriptor then
-				Log.debug("Could not sync back property {:?}.{}", instance, propertyName)
-				continue
-			end
+		elseif propertyName == "MeshId" then
+			update.requiresRecreate = true
+		elseif propertyName == "ClassName" then
+			update.requiresRecreate, update.changedClassName = true, instance.ClassName
+		end
+
+		local descriptor = RbxDom.findCanonicalPropertyDescriptor(instance.ClassName, propertyName)
 
-			local encodeSuccess, encodeResult = encodeProperty(instance, propertyName, descriptor)
+		if not descriptor then
+			Log.debug("Could not sync back property {:?}.{}", instance, propertyName)
+			continue
+		end
 
-			if not encodeSuccess then
-				Log.debug("Could not sync back property {:?}.{}: {}", instance, propertyName, encodeResult)
-				continue
-			end
+		local encodeSuccess, encodeResult = encodeProperty(instance, propertyName, descriptor)
 
-			update.changedProperties[propertyName] = encodeResult
+		if not encodeSuccess then
+			Log.debug("Could not sync back property {:?}.{}: {}", instance, propertyName, encodeResult)
+			continue
 		end
+
+		update.changedProperties[propertyName] = encodeResult
 	end
 
 	if next(update.changedProperties) == nil and update.changedName == nil then

plugin/src/ChangeBatcher/encodePatchUpdate.spec.lua

@@ -37,6 +37,18 @@ return function()
 		expect(update.changedName).to.equal("We'reGettingToTheCoolPart")
 	end)
 
+	it("should recreate instance in the update when the instance's ClassName changes", function()
+		local part = Instance.new("Part")
+		local properties = {
+			ClassName = true
+		}
+
+		local update = encodePatchUpdate(part, "PART", properties)
+
+		expect(update.changedProperties.ClassName).to.be.ok()
+		expect(update.requiresRecreate).to.equal(true)
+	end)
+
 	it("should correctly encode property values", function()
 		local part = Instance.new("Part")
 		local properties = {

plugin/src/Reconciler/applyPatch.lua

@@ -93,10 +93,10 @@ local function applyPatch(instanceMap, patch)
 		}
 		local partiallyApplied = false
 
-		-- If the instance's className changed, we have a bumpy ride ahead while
-		-- we recreate this instance and move all of its children into the new
-		-- version atomically...ish.
-		if update.changedClassName ~= nil then
+		-- If the instance's className changed, or there is custom recreate logic,
+		-- we have a bumpy ride ahead while we recreate this instance and move all 
+		-- of its children into the new version atomically...ish.
+		if update.requiresRecreate then
 			-- If the instance's name also changed, we'll do it here, since this
 			-- branch will skip the rest of the loop iteration.
 			local newName = update.changedName or instance.Name
@@ -121,7 +121,7 @@ local function applyPatch(instanceMap, patch)
 			local mockVirtualInstance = {
 				Id = update.id,
 				Name = newName,
-				ClassName = update.changedClassName,
+				ClassName = update.changedClassName or instance.ClassName,
 				Properties = newProperties,
 				Children = {},
 			}

plugin/src/Reconciler/applyPatch.spec.lua

@@ -1,6 +1,8 @@
 return function()
 	local applyPatch = require(script.Parent.applyPatch)
 
+	local InsertService = game:GetService("InsertService")
+
 	local InstanceMap = require(script.Parent.Parent.InstanceMap)
 	local PatchSet = require(script.Parent.Parent.PatchSet)
 
@@ -156,7 +158,33 @@ return function()
 		expect(value.Value).to.equal("WORLD")
 	end)
 
-	it("should recreate instances when changedClassName is set, preserving children", function()
+	it("should recreate instances for MeshId updates", function()
+		local mesh = InsertService:CreateMeshPartAsync("rbxassetid://3229650568", Enum.CollisionFidelity.Default, Enum.RenderFidelity.Automatic)
+
+		local instanceMap = InstanceMap.new()
+		instanceMap:insert("VALUE", mesh)
+
+		local patch = PatchSet.newEmpty()
+		table.insert(patch.updated, {
+			id = "VALUE",
+			changedProperties = {
+				MeshId = {
+					Content = "rbxassetid://4868357305",
+				},
+			},
+			requiresRecreate = true
+		})
+
+		local unapplied = applyPatch(instanceMap, patch)
+		local newInstance = instanceMap.fromIds["VALUE"]
+
+		expect(mesh.Parent).never.to.be.ok()
+		expect(mesh).never.to.equal(newInstance)
+		expect(newInstance).to.be.ok()
+		expect(newInstance.MeshId).to.equal("rbxassetid://4868357305")
+	end)
+
+	it("should recreate instances when requiresRecreate is set, preserving children", function()
 		local root = Instance.new("Folder")
 		root.Name = "Initial Root Name"
 
@@ -173,6 +201,7 @@ return function()
 			id = "ROOT",
 			changedName = "Updated Root Name",
 			changedClassName = "StringValue",
+			requiresRecreate = true,
 			changedProperties = {
 				Value = {
 					String = "I am Root",

plugin/src/Reconciler/diff.lua

@@ -136,8 +136,10 @@ local function diff(instanceMap, virtualInstances, rootId)
 		end
 
 		local changedClassName = nil
+		local requiresRecreate = false
 		if virtualInstance.ClassName ~= instance.ClassName then
 			changedClassName = virtualInstance.ClassName
+			requiresRecreate = true
 		end
 
 		local changedName = nil
@@ -154,7 +156,7 @@ local function diff(instanceMap, virtualInstances, rootId)
 				local ok, decodedValue = decodeValue(virtualValue, instanceMap)
 
 				if ok then
-					if not trueEquals(existingValue, decodedValue) then
+					if not trueEquals(existingValue, decodedValue) or requiresRecreate then
 						Log.debug(
 							"{}.{} changed from '{}' to '{}'",
 							instance:GetFullName(),
@@ -163,6 +165,9 @@ local function diff(instanceMap, virtualInstances, rootId)
 							decodedValue
 						)
 						changedProperties[propertyName] = virtualValue
+						if propertyName == "MeshId" then
+							requiresRecreate = true
+						end
 					end
 				else
 					local propertyType = next(virtualValue)
@@ -186,10 +191,11 @@ local function diff(instanceMap, virtualInstances, rootId)
 			end
 		end
 
-		if changedName ~= nil or changedClassName ~= nil or not isEmpty(changedProperties) then
+		if changedName ~= nil or requiresRecreate or not isEmpty(changedProperties) then
 			table.insert(patch.updated, {
 				id = id,
 				changedName = changedName,
+				requiresRecreate = requiresRecreate,
 				changedClassName = changedClassName,
 				changedProperties = changedProperties,
 				changedMetadata = nil,

plugin/src/Reconciler/reify.lua

@@ -2,6 +2,8 @@
 	"Reifies" a virtual DOM, constructing a real DOM with the same shape.
 ]]
 
+local InsertService = game:GetService('InsertService')
+
 local invariant = require(script.Parent.Parent.invariant)
 local PatchSet = require(script.Parent.Parent.PatchSet)
 local setProperty = require(script.Parent.setProperty)
@@ -40,6 +42,20 @@ local function addAllToPatch(patchSet, virtualInstances, id)
 	end
 end
 
+local function createInstance(instanceMap, className, properties)
+	-- Custom logic for MeshParts
+	-- see: https://github.com/rojo-rbx/rojo/issues/402
+	if className == "MeshPart" then
+		local okMeshId, meshId = decodeValue(properties.MeshId, instanceMap)
+
+		-- It is okay to provide default properties here, because 
+		-- they will be overriden on the update cycle
+		return InsertService:CreateMeshPartAsync(meshId, Enum.CollisionFidelity.Default, Enum.RenderFidelity.Automatic)
+	else
+		return Instance.new(className)
+	end
+end
+
 --[[
 	Inner function that defines the core routine.
 ]]
@@ -53,7 +69,7 @@ function reifyInner(instanceMap, virtualInstances, id, parentInstance, unapplied
 	-- Instance.new can fail if we're passing in something that can't be
 	-- created, like a service, something enabled with a feature flag, or
 	-- something that requires higher security than we have.
-	local ok, instance = pcall(Instance.new, virtualInstance.ClassName)
+	local ok, instance = pcall(createInstance, instanceMap, virtualInstance.ClassName, virtualInstance.Properties)
 
 	if not ok then
 		addAllToPatch(unappliedPatch, virtualInstances, id)

plugin/src/Types.lua

@@ -25,6 +25,7 @@ local ApiInstanceUpdate = t.interface({
 	id = RbxId,
 	changedName = t.optional(t.string),
 	changedClassName = t.optional(t.string),
+	requiresRecreate = t.optional(t.boolean),
 	changedProperties = t.map(t.string, ApiValue),
 	changedMetadata = t.optional(ApiInstanceMetadata),
 })

test-projects/mesh_parts/default.project.json

@@ -0,0 +1,12 @@
+{
+  "name": "mesh_parts",
+  "tree": {
+    "$className": "DataModel",
+
+    "Workspace": {
+      "TwoParts": {
+        "$path": "src"
+      }
+    }
+  }
+}