From eb114ac76e5c2280ffa50c057a724b77de721ab5 Mon Sep 17 00:00:00 2001 From: Joe Date: Thu, 19 Feb 2026 07:33:32 -0600 Subject: [PATCH] Refactor snapshot attribute handling in patch computation - Removed redundant Rojo reference attributes from snapshot comparisons to prevent spurious diffs during VFS events. - Updated the logic for interned instance additions to ensure deterministic ordering by redacted parent and name. These changes aim to improve the accuracy of snapshot comparisons and enhance the handling of interned instances. --- ...__serve__ref_path_change_target_patch.snap | 4 --- ..._serve__ref_path_multiple_attrs_patch.snap | 6 ----- ...tests__serve__ref_path_new_file_patch.snap | 11 +------- ...ts__serve__ref_path_nonexistent_patch.snap | 4 --- ...ref_properties_patch_update_subscribe.snap | 4 --- src/snapshot/patch_compute.rs | 26 ++++++++++++++++++- tests/rojo_test/internable.rs | 25 ++++++++++-------- 7 files changed, 40 insertions(+), 40 deletions(-) diff --git a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_path_change_target_patch.snap b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_path_change_target_patch.snap index 6ce2b91d..c7f44710 100644 --- a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_path_change_target_patch.snap +++ b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_path_change_target_patch.snap @@ -12,10 +12,6 @@ body: changedMetadata: ~ changedName: ~ changedProperties: - Attributes: - "!Attributes": - Rojo_Ref_PrimaryPart: - "!String": "@self/OtherPart" PrimaryPart: "!Ref": id-4 id: id-3 diff --git a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_path_multiple_attrs_patch.snap b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_path_multiple_attrs_patch.snap index b369de5c..165b123f 100644 --- a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_path_multiple_attrs_patch.snap +++ b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_path_multiple_attrs_patch.snap @@ -12,12 +12,6 @@ body: changedMetadata: ~ changedName: ~ changedProperties: - Attributes: - "!Attributes": - Rojo_Ref_CustomRef: - "!String": "@self/OtherPart" - Rojo_Ref_PrimaryPart: - "!String": "@self/Target" CustomRef: "!Ref": id-4 id: id-3 diff --git a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_path_new_file_patch.snap b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_path_new_file_patch.snap index c38c7864..a7cda951 100644 --- a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_path_new_file_patch.snap +++ b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_path_new_file_patch.snap @@ -20,15 +20,6 @@ body: Value: "!Ref": id-5 removed: [] - updated: - - changedClassName: ~ - changedMetadata: ~ - changedName: ~ - changedProperties: - Attributes: - "!Attributes": - Rojo_Ref_PrimaryPart: - "!String": "@self/Target" - id: id-3 + updated: [] packetType: messages sessionId: id-1 diff --git a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_path_nonexistent_patch.snap b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_path_nonexistent_patch.snap index 2cafeb60..eb8114f2 100644 --- a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_path_nonexistent_patch.snap +++ b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_path_nonexistent_patch.snap @@ -12,10 +12,6 @@ body: changedMetadata: ~ changedName: ~ changedProperties: - Attributes: - "!Attributes": - Rojo_Ref_PrimaryPart: - "!String": "./NonExistent/Part" PrimaryPart: ~ id: id-3 packetType: messages diff --git a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_properties_patch_update_subscribe.snap b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_properties_patch_update_subscribe.snap index f159fa44..d56e1e32 100644 --- a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_properties_patch_update_subscribe.snap +++ b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_properties_patch_update_subscribe.snap @@ -12,10 +12,6 @@ body: changedMetadata: ~ changedName: ~ changedProperties: - Attributes: - "!Attributes": - Rojo_Target_PrimaryPart: - "!String": model target Scale: "!Float32": 1 id: id-8 diff --git a/src/snapshot/patch_compute.rs b/src/snapshot/patch_compute.rs index c49ebc19..bf2e5f0b 100644 --- a/src/snapshot/patch_compute.rs +++ b/src/snapshot/patch_compute.rs @@ -9,7 +9,8 @@ use rbx_dom_weak::{ }; use crate::{ - variant_eq::variant_eq, RojoRef, REF_PATH_ATTRIBUTE_PREFIX, REF_POINTER_ATTRIBUTE_PREFIX, + variant_eq::variant_eq, RojoRef, REF_ID_ATTRIBUTE_NAME, REF_PATH_ATTRIBUTE_PREFIX, + REF_POINTER_ATTRIBUTE_PREFIX, }; use super::{ @@ -110,6 +111,29 @@ fn compute_property_patches( let attribute_ref_properties = compute_ref_properties(snapshot, tree, instance.id()); + // Strip Rojo ref transport attributes from the snapshot before comparison. + // finalize_patch_application strips these from the tree after resolving refs, + // so the snapshot must be stripped too to prevent spurious diffs on duplicate + // VFS events (common on macOS). The refs were already extracted above. + if let Some(Variant::Attributes(attrs)) = snapshot.properties.get_mut(&ustr("Attributes")) { + let keys_to_remove: Vec = attrs + .iter() + .filter_map(|(name, _)| { + if name == REF_ID_ATTRIBUTE_NAME + || name.starts_with(REF_POINTER_ATTRIBUTE_PREFIX) + || name.starts_with(REF_PATH_ATTRIBUTE_PREFIX) + { + Some(name.to_string()) + } else { + None + } + }) + .collect(); + for key in keys_to_remove { + attrs.remove(key); + } + } + let changed_name = if snapshot.name == instance.name() { None } else { diff --git a/tests/rojo_test/internable.rs b/tests/rojo_test/internable.rs index a409071e..0ad48889 100644 --- a/tests/rojo_test/internable.rs +++ b/tests/rojo_test/internable.rs @@ -104,19 +104,22 @@ fn intern_instance_additions( for (id, added) in additions { let parent_id = added.parent; - let parent_redacted = redactions.get_redacted_value(parent_id); - - // Here, we assume that instances are only added to other instances that - // we've already interned. If that's not true, then we'll have some - // dangling unredacted IDs. - if let Some(parent_redacted) = parent_redacted { - added_roots.push((id, parent_redacted)); - } + let parent_redacted = match redactions.get_redacted_value(parent_id) { + Some(v) => v, + None => { + redactions.intern(parent_id); + redactions.get_redacted_value(parent_id).unwrap() + } + }; + added_roots.push((id, parent_redacted)); } - // Sort the input by the redacted key, which should match the traversal - // order we need for the tree. - added_roots.sort_unstable_by(|a, b| a.1.cmp(&b.1)); + // Sort by redacted parent, then by name for deterministic ordering when + // multiple children share the same parent. + added_roots.sort_unstable_by(|a, b| { + a.1.cmp(&b.1) + .then_with(|| additions[a.0].name.cmp(&additions[b.0].name)) + }); for (root_id, _redacted_id) in added_roots { additions[root_id].intern(redactions, additions);