Hydrate by best match

CHANGELOG.md

@@ -39,6 +39,7 @@ Making a new release? Simply add the new header with the version and date undern
 * Fixed a bug where the notification timeout thread would fail to cancel on unmount ([#1211])
 * Added a "Forget" option to the sync reminder notification to avoid being reminded for that place in the future ([#1215])
 * Improves relative path calculation for sourcemap generation to avoid issues with Windows UNC paths. ([#1217])
+* Instances that share a name and class are now robustly matched on resync by comparing their properties, instead of relying on child order alone. ([#1266])
 
 [#1176]: https://github.com/rojo-rbx/rojo/pull/1176
 [#1179]: https://github.com/rojo-rbx/rojo/pull/1179
@@ -48,6 +49,7 @@ Making a new release? Simply add the new header with the version and date undern
 [#1211]: https://github.com/rojo-rbx/rojo/pull/1211
 [#1215]: https://github.com/rojo-rbx/rojo/pull/1215
 [#1217]: https://github.com/rojo-rbx/rojo/pull/1217
+[#1266]: https://github.com/rojo-rbx/rojo/pull/1266
 
 ## [7.7.0-rc.1] (November 27th, 2025)
 

plugin/src/Reconciler/countMatchingProperties.lua

@@ -0,0 +1,44 @@
+--[[
+	Counts how many of a virtual instance's properties match the live values on a
+	candidate Roblox instance. `hydrate` uses this to break ties when several
+	existing children share the same Name and ClassName.
+
+	This mirrors the read -> decode -> compare flow that `diff` uses, reusing the
+	same `getProperty`, `decodeValue`, and `trueEquals` helpers.
+]]
+
+local getProperty = require(script.Parent.getProperty)
+local decodeValue = require(script.Parent.decodeValue)
+local trueEquals = require(script.Parent.trueEquals)
+
+local function countMatchingProperties(instance, virtualInstance, instanceMap)
+	local score = 0
+
+	for propertyName, virtualValue in virtualInstance.Properties do
+		-- Skip refs. During hydration the instanceMap is still being built
+		-- top-down, so a ref may point at an instance we haven't hydrated yet
+		-- and therefore can't decode reliably. Refs are also a poor
+		-- disambiguator between same-named siblings.
+		if next(virtualValue) == "Ref" then
+			continue
+		end
+
+		local getSuccess, existingValue = getProperty(instance, propertyName)
+		if not getSuccess then
+			continue
+		end
+
+		local decodeSuccess, decodedValue = decodeValue(virtualValue, instanceMap)
+		if not decodeSuccess then
+			continue
+		end
+
+		if trueEquals(existingValue, decodedValue) then
+			score += 1
+		end
+	end
+
+	return score
+end
+
+return countMatchingProperties

plugin/src/Reconciler/countMatchingProperties.spec.lua

@@ -0,0 +1,91 @@
+return function()
+	local countMatchingProperties = require(script.Parent.countMatchingProperties)
+
+	local InstanceMap = require(script.Parent.Parent.InstanceMap)
+
+	it("counts properties whose values match the instance", function()
+		local instance = Instance.new("StringValue")
+		instance.Value = "hello"
+
+		local virtualInstance = {
+			ClassName = "StringValue",
+			Name = "Value",
+			Properties = {
+				Value = { String = "hello" },
+			},
+			Children = {},
+		}
+
+		expect(countMatchingProperties(instance, virtualInstance, InstanceMap.new())).to.equal(1)
+	end)
+
+	it("does not count properties whose values differ", function()
+		local instance = Instance.new("StringValue")
+		instance.Value = "hello"
+
+		local virtualInstance = {
+			ClassName = "StringValue",
+			Name = "Value",
+			Properties = {
+				Value = { String = "different" },
+			},
+			Children = {},
+		}
+
+		expect(countMatchingProperties(instance, virtualInstance, InstanceMap.new())).to.equal(0)
+	end)
+
+	it("counts multiple matching properties independently", function()
+		local instance = Instance.new("Part")
+		instance.Anchored = true
+		instance.CanCollide = false
+
+		local virtualInstance = {
+			ClassName = "Part",
+			Name = "Part",
+			Properties = {
+				Anchored = { Bool = true },
+				CanCollide = { Bool = false },
+			},
+			Children = {},
+		}
+
+		expect(countMatchingProperties(instance, virtualInstance, InstanceMap.new())).to.equal(2)
+
+		-- Flip one so only a single property matches.
+		instance.CanCollide = true
+		expect(countMatchingProperties(instance, virtualInstance, InstanceMap.new())).to.equal(1)
+	end)
+
+	it("skips unknown properties without counting or erroring", function()
+		local instance = Instance.new("Folder")
+
+		local virtualInstance = {
+			ClassName = "Folder",
+			Name = "Folder",
+			Properties = {
+				FAKE_PROPERTY = { String = "nope" },
+			},
+			Children = {},
+		}
+
+		expect(countMatchingProperties(instance, virtualInstance, InstanceMap.new())).to.equal(0)
+	end)
+
+	it("skips Ref properties without counting or erroring", function()
+		local instance = Instance.new("ObjectValue")
+
+		local virtualInstance = {
+			ClassName = "ObjectValue",
+			Name = "ObjectValue",
+			Properties = {
+				-- A ref must be skipped rather than decoded: during hydration
+				-- the target may not be in the map yet.
+				Value = { Ref = "00000000000000000000000000000000" },
+			},
+			Children = {},
+		}
+
+		expect(countMatchingProperties(instance, virtualInstance, InstanceMap.new())).to.equal(0)
+	end)
+end

plugin/src/Reconciler/diff.lua

@@ -10,104 +10,12 @@ local invariant = require(script.Parent.Parent.invariant)
 local getProperty = require(script.Parent.getProperty)
 local Error = require(script.Parent.Error)
 local decodeValue = require(script.Parent.decodeValue)
+local trueEquals = require(script.Parent.trueEquals)
 
 local function isEmpty(table)
 	return next(table) == nil
 end
 
-local function fuzzyEq(a: number, b: number, epsilon: number): boolean
-	return math.abs(a - b) < epsilon
-end
-
-local function trueEquals(a, b): boolean
-	-- Exit early for simple equality values
-	if a == b then
-		return true
-	end
-
-	-- Treat nil and { Ref = "000...0" } as equal
-	if
-		(a == nil and type(b) == "table" and b.Ref == "00000000000000000000000000000000")
-		or (b == nil and type(a) == "table" and a.Ref == "00000000000000000000000000000000")
-	then
-		return true
-	end
-
-	local typeA, typeB = typeof(a), typeof(b)
-
-	-- For tables, try recursive deep equality
-	if typeA == "table" and typeB == "table" then
-		local checkedKeys = {}
-		for key, value in a do
-			checkedKeys[key] = true
-			if not trueEquals(value, b[key]) then
-				return false
-			end
-		end
-		for key, value in b do
-			if checkedKeys[key] then
-				continue
-			end
-			if not trueEquals(value, a[key]) then
-				return false
-			end
-		end
-		return true
-
-	-- For NaN, check if both values are not equal to themselves
-	elseif a ~= a and b ~= b then
-		return true
-
-	-- For numbers, compare with epsilon of 0.0001 to avoid floating point inequality
-	elseif typeA == "number" and typeB == "number" then
-		return fuzzyEq(a, b, 0.0001)
-
-	-- For EnumItem->number, compare the EnumItem's value
-	elseif typeA == "number" and typeB == "EnumItem" then
-		return a == b.Value
-	elseif typeA == "EnumItem" and typeB == "number" then
-		return a.Value == b
-
-	-- For Color3s, compare to RGB ints to avoid floating point inequality
-	elseif typeA == "Color3" and typeB == "Color3" then
-		local aR, aG, aB = math.floor(a.R * 255), math.floor(a.G * 255), math.floor(a.B * 255)
-		local bR, bG, bB = math.floor(b.R * 255), math.floor(b.G * 255), math.floor(b.B * 255)
-		return aR == bR and aG == bG and aB == bB
-
-	-- For CFrames, compare to components with epsilon of 0.0001 to avoid floating point inequality
-	elseif typeA == "CFrame" and typeB == "CFrame" then
-		local aComponents, bComponents = { a:GetComponents() }, { b:GetComponents() }
-		for i, aComponent in aComponents do
-			if not fuzzyEq(aComponent, bComponents[i], 0.0001) then
-				return false
-			end
-		end
-		return true
-
-	-- For Vector3s, compare to components with epsilon of 0.0001 to avoid floating point inequality
-	elseif typeA == "Vector3" and typeB == "Vector3" then
-		local aComponents, bComponents = { a.X, a.Y, a.Z }, { b.X, b.Y, b.Z }
-		for i, aComponent in aComponents do
-			if not fuzzyEq(aComponent, bComponents[i], 0.0001) then
-				return false
-			end
-		end
-		return true
-
-	-- For Vector2s, compare to components with epsilon of 0.0001 to avoid floating point inequality
-	elseif typeA == "Vector2" and typeB == "Vector2" then
-		local aComponents, bComponents = { a.X, a.Y }, { b.X, b.Y }
-		for i, aComponent in aComponents do
-			if not fuzzyEq(aComponent, bComponents[i], 0.0001) then
-				return false
-			end
-		end
-		return true
-	end
-
-	return false
-end
-
 local function shouldDeleteUnknownInstances(virtualInstance)
 	if virtualInstance.Metadata ~= nil then
 		return not virtualInstance.Metadata.ignoreUnknownInstances

plugin/src/Reconciler/hydrate.lua

@@ -4,6 +4,16 @@
 ]]
 
 local invariant = require(script.Parent.Parent.invariant)
+local countMatchingProperties = require(script.Parent.countMatchingProperties)
+
+-- When several existing children share a Name and ClassName we disambiguate
+-- them by scoring how well each one's properties match the virtual instance.
+-- That scoring is far more expensive than a Name/ClassName check, so we only do
+-- it for reasonably-sized groups. Larger groups (e.g. a folder with thousands of
+-- identically-named parts) fall back to the original order-based matching, which
+-- bounds the added work to roughly MAX_CANDIDATES_TO_SCORE^2 property reads per
+-- group regardless of how large the group is.
+local MAX_CANDIDATES_TO_SCORE = 32
 
 local function hydrate(instanceMap, virtualInstances, rootId, rootInstance)
 	local virtualInstance = virtualInstances[rootId]
@@ -16,34 +26,106 @@ local function hydrate(instanceMap, virtualInstances, rootId, rootInstance)
 
 	local existingChildren = rootInstance:GetChildren()
 
-	-- For each existing child, we'll track whether it's been paired with an
-	-- instance that the Rojo server knows about.
-	local isExistingChildVisited = {}
-	for i = 1, #existingChildren do
-		isExistingChildVisited[i] = false
+	-- Group existing children by Name then ClassName so each virtual child can
+	-- find its candidate matches without scanning every sibling. This is what
+	-- keeps hydration fast for parents with thousands of children. Nesting the
+	-- two tables (rather than a combined key) keeps the Name and ClassName checks
+	-- exact, with no way for one to bleed into the other.
+	local buckets = {}
+	for _, childInstance in existingChildren do
+		-- We guard accessing Name and ClassName in order to avoid tripping over
+		-- children of DataModel that Rojo won't have permissions to access at all.
+		local accessSuccess, name, className = pcall(function()
+			return childInstance.Name, childInstance.ClassName
+		end)
+		if not accessSuccess then
+			continue
+		end
+
+		local bucketsByClassName = buckets[name]
+		if bucketsByClassName == nil then
+			bucketsByClassName = {}
+			buckets[name] = bucketsByClassName
+		end
+
+		local bucket = bucketsByClassName[className]
+		if bucket == nil then
+			bucket = { cursor = 1, instances = {} }
+			bucketsByClassName[className] = bucket
+		end
+
+		table.insert(bucket.instances, childInstance)
 	end
 
+	-- Tracks which existing children have already been paired, so one instance
+	-- isn't matched to two different virtual instances.
+	local visited = {}
+
 	for _, childId in ipairs(virtualInstance.Children) do
 		local virtualChild = virtualInstances[childId]
 
-		for childIndex, childInstance in existingChildren do
-			if not isExistingChildVisited[childIndex] then
-				-- We guard accessing Name and ClassName in order to avoid
-				-- tripping over children of DataModel that Rojo won't have
-				-- permissions to access at all.
-				local accessSuccess, name, className = pcall(function()
-					return childInstance.Name, childInstance.ClassName
-				end)
+		local bucketsByClassName = buckets[virtualChild.Name]
+		local bucket = bucketsByClassName and bucketsByClassName[virtualChild.ClassName]
+		if bucket == nil then
+			continue
+		end
 
-				-- This rule is very conservative and could be loosened in the
-				-- future, or more heuristics could be introduced.
-				if accessSuccess and name == virtualChild.Name and className == virtualChild.ClassName then
-					isExistingChildVisited[childIndex] = true
-					hydrate(instanceMap, virtualInstances, childId, childInstance)
-					break
+		local instances = bucket.instances
+
+		-- Advance past any leading children that have already been paired. The
+		-- cursor makes order-based matching amortized O(1) per child even for
+		-- very large groups, rather than rescanning the visited prefix.
+		while bucket.cursor <= #instances and visited[instances[bucket.cursor]] do
+			bucket.cursor += 1
+		end
+		if bucket.cursor > #instances then
+			continue
+		end
+
+		-- The cursor points at the earliest unvisited child, so the slots from
+		-- here to the end bound how many candidates remain. Visited children
+		-- after the cursor (gaps) only appear once a group is small enough to be
+		-- scored -- the order-based path below always takes the earliest, which
+		-- keeps the visited region a contiguous prefix. So whenever this count
+		-- exceeds the cap it is exact, and we can pick the earliest match without
+		-- collecting anything.
+		local remaining = #instances - bucket.cursor + 1
+
+		local match
+		if remaining > MAX_CANDIDATES_TO_SCORE then
+			-- Too many to score affordably; take the earliest in child order,
+			-- reproducing the original Name + ClassName behavior.
+			match = instances[bucket.cursor]
+		else
+			-- Collect the (at most `remaining`) unvisited candidates.
+			local candidates = {}
+			for index = bucket.cursor, #instances do
+				local childInstance = instances[index]
+				if not visited[childInstance] then
+					table.insert(candidates, childInstance)
+				end
+			end
+
+			if #candidates == 1 then
+				-- Only one candidate, so there's nothing to disambiguate.
+				match = candidates[1]
+			else
+				-- Break the tie by choosing the candidate whose properties best
+				-- match the virtual instance, falling back to the earliest in
+				-- child order when scores are equal.
+				local bestScore = -1
+				for _, childInstance in candidates do
+					local score = countMatchingProperties(childInstance, virtualChild, instanceMap)
+					if score > bestScore then
+						bestScore = score
+						match = childInstance
+					end
 				end
 			end
 		end
+
+		visited[match] = true
+		hydrate(instanceMap, virtualInstances, childId, match)
 	end
 end
 

plugin/src/Reconciler/hydrate.spec.lua

@@ -126,4 +126,140 @@ return function()
 		expect(knownInstances.fromIds["CHILD1"]).to.equal(child1)
 		expect(knownInstances.fromIds["CHILD2"]).to.equal(child2)
 	end)
+
+	it("should disambiguate duplicate-named siblings by matching properties", function()
+		local knownInstances = InstanceMap.new()
+		local virtualInstances = {
+			ROOT = {
+				ClassName = "Folder",
+				Name = "Root",
+				Properties = {},
+				Children = { "CHILD_A", "CHILD_B" },
+			},
+
+			CHILD_A = {
+				ClassName = "StringValue",
+				Name = "a",
+				Properties = { Value = { String = "first" } },
+				Children = {},
+			},
+
+			CHILD_B = {
+				ClassName = "StringValue",
+				Name = "a",
+				Properties = { Value = { String = "second" } },
+				Children = {},
+			},
+		}
+
+		local rootInstance = Instance.new("Folder")
+
+		-- Created in the reverse order of the virtual children, so a purely
+		-- order-based tiebreak would mis-pair them.
+		local child1 = Instance.new("StringValue")
+		child1.Name = "a"
+		child1.Value = "second"
+		child1.Parent = rootInstance
+
+		local child2 = Instance.new("StringValue")
+		child2.Name = "a"
+		child2.Value = "first"
+		child2.Parent = rootInstance
+
+		hydrate(knownInstances, virtualInstances, "ROOT", rootInstance)
+
+		expect(knownInstances:size()).to.equal(3)
+		expect(knownInstances.fromIds["CHILD_A"]).to.equal(child2)
+		expect(knownInstances.fromIds["CHILD_B"]).to.equal(child1)
+	end)
+
+	it("should fall back to child order for duplicate-named siblings with no distinguishing properties", function()
+		local knownInstances = InstanceMap.new()
+		local virtualInstances = {
+			ROOT = {
+				ClassName = "Folder",
+				Name = "Root",
+				Properties = {},
+				Children = { "CHILD_A", "CHILD_B" },
+			},
+
+			CHILD_A = {
+				ClassName = "Folder",
+				Name = "a",
+				Properties = {},
+				Children = {},
+			},
+
+			CHILD_B = {
+				ClassName = "Folder",
+				Name = "a",
+				Properties = {},
+				Children = {},
+			},
+		}
+
+		local rootInstance = Instance.new("Folder")
+
+		local child1 = Instance.new("Folder")
+		child1.Name = "a"
+		child1.Parent = rootInstance
+
+		local child2 = Instance.new("Folder")
+		child2.Name = "a"
+		child2.Parent = rootInstance
+
+		hydrate(knownInstances, virtualInstances, "ROOT", rootInstance)
+
+		expect(knownInstances:size()).to.equal(3)
+		-- With equal scores the earliest unvisited child wins, preserving the
+		-- original order-based behavior.
+		expect(knownInstances.fromIds["CHILD_A"]).to.equal(child1)
+		expect(knownInstances.fromIds["CHILD_B"]).to.equal(child2)
+	end)
+
+	it("should fall back to child order for very large duplicate-named groups", function()
+		-- More candidates than hydrate is willing to score at once. The group
+		-- must fall back to order-based matching, so virtual child N pairs with
+		-- existing child N regardless of properties.
+		local count = 64
+
+		local knownInstances = InstanceMap.new()
+		local virtualInstances = {
+			ROOT = {
+				ClassName = "Folder",
+				Name = "Root",
+				Properties = {},
+				Children = {},
+			},
+		}
+
+		local rootInstance = Instance.new("Folder")
+
+		local expectedInstances = {}
+		for i = 1, count do
+			local id = "CHILD_" .. i
+			table.insert(virtualInstances.ROOT.Children, id)
+			virtualInstances[id] = {
+				ClassName = "StringValue",
+				Name = "a",
+				-- Distinct values that, if scored, would pair by value rather
+				-- than by order.
+				Properties = { Value = { String = "value " .. i } },
+				Children = {},
+			}
+
+			local child = Instance.new("StringValue")
+			child.Name = "a"
+			child.Value = "value " .. (count - i + 1)
+			child.Parent = rootInstance
+			expectedInstances[id] = child
+		end
+
+		hydrate(knownInstances, virtualInstances, "ROOT", rootInstance)
+
+		expect(knownInstances:size()).to.equal(count + 1)
+		for id, expectedInstance in expectedInstances do
+			expect(knownInstances.fromIds[id]).to.equal(expectedInstance)
+		end
+	end)
 end

plugin/src/Reconciler/trueEquals.lua

@@ -0,0 +1,100 @@
+--[[
+	Fuzzy value-equality used to compare a decoded virtual property value against
+	the live value read from a real instance. Shared by `diff` (to decide whether
+	a property changed) and `hydrate` (to score candidate instances).
+]]
+
+local function fuzzyEq(a: number, b: number, epsilon: number): boolean
+	return math.abs(a - b) < epsilon
+end
+
+local function trueEquals(a, b): boolean
+	-- Exit early for simple equality values
+	if a == b then
+		return true
+	end
+
+	-- Treat nil and { Ref = "000...0" } as equal
+	if
+		(a == nil and type(b) == "table" and b.Ref == "00000000000000000000000000000000")
+		or (b == nil and type(a) == "table" and a.Ref == "00000000000000000000000000000000")
+	then
+		return true
+	end
+
+	local typeA, typeB = typeof(a), typeof(b)
+
+	-- For tables, try recursive deep equality
+	if typeA == "table" and typeB == "table" then
+		local checkedKeys = {}
+		for key, value in a do
+			checkedKeys[key] = true
+			if not trueEquals(value, b[key]) then
+				return false
+			end
+		end
+		for key, value in b do
+			if checkedKeys[key] then
+				continue
+			end
+			if not trueEquals(value, a[key]) then
+				return false
+			end
+		end
+		return true
+
+	-- For NaN, check if both values are not equal to themselves
+	elseif a ~= a and b ~= b then
+		return true
+
+	-- For numbers, compare with epsilon of 0.0001 to avoid floating point inequality
+	elseif typeA == "number" and typeB == "number" then
+		return fuzzyEq(a, b, 0.0001)
+
+	-- For EnumItem->number, compare the EnumItem's value
+	elseif typeA == "number" and typeB == "EnumItem" then
+		return a == b.Value
+	elseif typeA == "EnumItem" and typeB == "number" then
+		return a.Value == b
+
+	-- For Color3s, compare to RGB ints to avoid floating point inequality
+	elseif typeA == "Color3" and typeB == "Color3" then
+		local aR, aG, aB = math.floor(a.R * 255), math.floor(a.G * 255), math.floor(a.B * 255)
+		local bR, bG, bB = math.floor(b.R * 255), math.floor(b.G * 255), math.floor(b.B * 255)
+		return aR == bR and aG == bG and aB == bB
+
+	-- For CFrames, compare to components with epsilon of 0.0001 to avoid floating point inequality
+	elseif typeA == "CFrame" and typeB == "CFrame" then
+		local aComponents, bComponents = { a:GetComponents() }, { b:GetComponents() }
+		for i, aComponent in aComponents do
+			if not fuzzyEq(aComponent, bComponents[i], 0.0001) then
+				return false
+			end
+		end
+		return true
+
+	-- For Vector3s, compare to components with epsilon of 0.0001 to avoid floating point inequality
+	elseif typeA == "Vector3" and typeB == "Vector3" then
+		local aComponents, bComponents = { a.X, a.Y, a.Z }, { b.X, b.Y, b.Z }
+		for i, aComponent in aComponents do
+			if not fuzzyEq(aComponent, bComponents[i], 0.0001) then
+				return false
+			end
+		end
+		return true
+
+	-- For Vector2s, compare to components with epsilon of 0.0001 to avoid floating point inequality
+	elseif typeA == "Vector2" and typeB == "Vector2" then
+		local aComponents, bComponents = { a.X, a.Y }, { b.X, b.Y }
+		for i, aComponent in aComponents do
+			if not fuzzyEq(aComponent, bComponents[i], 0.0001) then
+				return false
+			end
+		end
+		return true
+	end
+
+	return false
+end
+
+return trueEquals