diff --git a/CHANGELOG.md b/CHANGELOG.md index 3943ae5f..d87801a5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,10 +41,12 @@ Making a new release? Simply add the new header with the version and date undern * Fixed `rojo serve` silently stopping syncing file changes on Windows when the served project path was a verbatim (`\\?\`) path, because tree paths and file-watcher event paths were canonicalized to different forms. ([#1290]) * Fixed `rojo sourcemap --absolute` emitting verbatim (`\\?\`) paths on Windows, which broke require types in luau-lsp. ([#1290]) * The plugin now disables the `Check for Updates` setting if you block access to `api.github.com`. ([#1297]) +* Fixed sync updates including `changedMetadata` when only Rojo's internal metadata changed. ([#1320]) [7.7.1]: https://github.com/rojo-rbx/rojo/releases/tag/v7.7.1 [#1290]: https://github.com/rojo-rbx/rojo/pull/1290 [#1297]: https://github.com/rojo-rbx/rojo/pull/1297 +[#1320]: https://github.com/rojo-rbx/rojo/pull/1320 ## [7.7.0] (July 1st, 2026) diff --git a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__changed_metadata_metadata_only.snap b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__changed_metadata_metadata_only.snap new file mode 100644 index 00000000..3509531c --- /dev/null +++ b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__changed_metadata_metadata_only.snap @@ -0,0 +1,18 @@ +--- +source: tests/tests/serve.rs +expression: "packet.intern_and_redact(&mut redactions, ())" +--- +body: + messageCursor: 2 + messages: + - added: {} + removed: [] + updated: + - changedClassName: ~ + changedMetadata: + ignoreUnknownInstances: false + changedName: ~ + changedProperties: {} + id: id-3 +packetType: messages +sessionId: id-1 diff --git a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__changed_metadata_property_and_metadata.snap b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__changed_metadata_property_and_metadata.snap new file mode 100644 index 00000000..8c05897f --- /dev/null +++ b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__changed_metadata_property_and_metadata.snap @@ -0,0 +1,20 @@ +--- +source: tests/tests/serve.rs +expression: "packet.intern_and_redact(&mut redactions, ())" +--- +body: + messageCursor: 3 + messages: + - added: {} + removed: [] + updated: + - changedClassName: ~ + changedMetadata: + ignoreUnknownInstances: true + changedName: ~ + changedProperties: + Anchored: + Bool: true + id: id-3 +packetType: messages +sessionId: id-1 diff --git a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__changed_metadata_property_only.snap b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__changed_metadata_property_only.snap new file mode 100644 index 00000000..aca54eca --- /dev/null +++ b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__changed_metadata_property_only.snap @@ -0,0 +1,19 @@ +--- +source: tests/tests/serve.rs +expression: "packet.intern_and_redact(&mut redactions, ())" +--- +body: + messageCursor: 1 + messages: + - added: {} + removed: [] + updated: + - changedClassName: ~ + changedMetadata: ~ + changedName: ~ + changedProperties: + Anchored: + Bool: false + id: id-3 +packetType: messages +sessionId: id-1 diff --git a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__changed_metadata_relevant_paths_only.snap b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__changed_metadata_relevant_paths_only.snap new file mode 100644 index 00000000..6e82f240 --- /dev/null +++ b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__changed_metadata_relevant_paths_only.snap @@ -0,0 +1,12 @@ +--- +source: tests/tests/serve.rs +expression: "packet.intern_and_redact(&mut redactions, ())" +--- +body: + messageCursor: 4 + messages: + - added: {} + removed: [] + updated: [] +packetType: messages +sessionId: id-1 diff --git a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__model_pivot_migration_all.snap b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__model_pivot_migration_all.snap index 9b7a6045..abaaeeae 100644 --- a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__model_pivot_migration_all.snap +++ b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__model_pivot_migration_all.snap @@ -18,12 +18,6 @@ body: NeedsPivotMigration: Bool: false removed: [] - updated: - - changedClassName: ~ - changedMetadata: - ignoreUnknownInstances: true - changedName: ~ - changedProperties: {} - id: id-3 + updated: [] packetType: messages sessionId: id-1 diff --git a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_properties_subscribe.snap b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_properties_subscribe.snap index b601ae47..026b2710 100644 --- a/rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_properties_subscribe.snap +++ b/rojo-test/serve-test-snapshots/end_to_end__tests__serve__ref_properties_subscribe.snap @@ -26,8 +26,7 @@ body: removed: [] updated: - changedClassName: ~ - changedMetadata: - ignoreUnknownInstances: false + changedMetadata: ~ changedName: ~ changedProperties: Attributes: diff --git a/rojo-test/serve-tests/changed_metadata/a/mod.lua b/rojo-test/serve-tests/changed_metadata/a/mod.lua new file mode 100644 index 00000000..39fc9635 --- /dev/null +++ b/rojo-test/serve-tests/changed_metadata/a/mod.lua @@ -0,0 +1 @@ +-- shared contents diff --git a/rojo-test/serve-tests/changed_metadata/b/mod.lua b/rojo-test/serve-tests/changed_metadata/b/mod.lua new file mode 100644 index 00000000..39fc9635 --- /dev/null +++ b/rojo-test/serve-tests/changed_metadata/b/mod.lua @@ -0,0 +1 @@ +-- shared contents diff --git a/rojo-test/serve-tests/changed_metadata/default.project.json b/rojo-test/serve-tests/changed_metadata/default.project.json new file mode 100644 index 00000000..24bda700 --- /dev/null +++ b/rojo-test/serve-tests/changed_metadata/default.project.json @@ -0,0 +1,18 @@ +{ + "name": "changed_metadata", + "tree": { + "$className": "DataModel", + "Workspace": { + "$className": "Workspace", + "Baseplate": { + "$className": "Part", + "$properties": { + "Anchored": true + } + }, + "Thing": { + "$path": "a" + } + } + } +} diff --git a/src/cli/sourcemap.rs b/src/cli/sourcemap.rs index 2e4a7ce9..dafee6c3 100644 --- a/src/cli/sourcemap.rs +++ b/src/cli/sourcemap.rs @@ -269,6 +269,39 @@ mod test { use insta::internals::Content; use std::path::Path; + #[test] + fn internal_metadata_change_still_invalidates_sourcemap() { + use crate::cli::sourcemap::{filter_nothing, patch_set_affects_sourcemap}; + use crate::serve_session::ServeSession; + use crate::snapshot::{AppliedPatchSet, AppliedPatchUpdate, InstanceMetadata}; + use memofs::Vfs; + + let project_path = fs_err::canonicalize( + Path::new(env!("CARGO_MANIFEST_DIR")) + .join("test-projects") + .join("relative_paths") + .join("project"), + ) + .unwrap(); + let session = ServeSession::new(Vfs::new_default().unwrap(), project_path).unwrap(); + + let mut update = AppliedPatchUpdate::new(session.tree().get_root_id()); + update.changed_metadata = Some(InstanceMetadata::new()); + assert_eq!(update.changed_ignore_unknown_instances, None); + + let patch_set = vec![AppliedPatchSet { + removed: Vec::new(), + added: Vec::new(), + updated: vec![update], + }]; + + assert!(patch_set_affects_sourcemap( + &session, + &patch_set, + filter_nothing + )); + } + #[test] fn maps_relative_paths() { let sourcemap_dir = tempfile::tempdir().unwrap(); diff --git a/src/snapshot/patch.rs b/src/snapshot/patch.rs index e8899357..5e143688 100644 --- a/src/snapshot/patch.rs +++ b/src/snapshot/patch.rs @@ -92,6 +92,9 @@ pub struct AppliedPatchUpdate { pub changed_class_name: Option, pub changed_properties: UstrMap>, pub changed_metadata: Option, + + #[serde(default, skip_serializing_if = "Option::is_none")] + pub changed_ignore_unknown_instances: Option, } impl AppliedPatchUpdate { @@ -102,6 +105,7 @@ impl AppliedPatchUpdate { changed_class_name: None, changed_properties: UstrMap::new(), changed_metadata: None, + changed_ignore_unknown_instances: None, } } } diff --git a/src/snapshot/patch_apply.rs b/src/snapshot/patch_apply.rs index 8427c1a1..e278f2f2 100644 --- a/src/snapshot/patch_apply.rs +++ b/src/snapshot/patch_apply.rs @@ -175,6 +175,12 @@ fn apply_update_child(context: &mut PatchApplyContext, tree: &mut RojoTree, patc let mut applied_patch = AppliedPatchUpdate::new(patch.id); if let Some(metadata) = patch.changed_metadata { + applied_patch.changed_ignore_unknown_instances = tree + .get_instance(patch.id) + .map(|instance| instance.metadata().ignore_unknown_instances) + .filter(|previous| *previous != metadata.ignore_unknown_instances) + .map(|_| metadata.ignore_unknown_instances); + tree.update_metadata(patch.id, metadata.clone()); applied_patch.changed_metadata = Some(metadata); } diff --git a/src/snapshot/tests/apply.rs b/src/snapshot/tests/apply.rs index 4c3f7f38..151ffecf 100644 --- a/src/snapshot/tests/apply.rs +++ b/src/snapshot/tests/apply.rs @@ -3,8 +3,12 @@ use insta::assert_yaml_snapshot; use rbx_dom_weak::{ustr, UstrMap}; use rojo_insta_ext::RedactionMap; +use std::path::PathBuf; + use crate::{ - snapshot::{apply_patch_set, InstanceSnapshot, PatchSet, PatchUpdate, RojoTree}, + snapshot::{ + apply_patch_set, InstanceMetadata, InstanceSnapshot, PatchSet, PatchUpdate, RojoTree, + }, tree_view::{intern_tree, view_tree}, }; @@ -101,6 +105,54 @@ fn remove_property() { assert_yaml_snapshot!("remove_property_appied_patch", applied_patch_value); } +#[test] +fn changed_ignore_unknown_instances_is_set_when_it_changes() { + let mut tree = empty_tree(); + let id = tree.get_root_id(); + + let patch_set = PatchSet { + updated_instances: vec![PatchUpdate { + id, + changed_name: None, + changed_class_name: None, + changed_properties: Default::default(), + changed_metadata: Some(InstanceMetadata::new().ignore_unknown_instances(true)), + }], + ..Default::default() + }; + + let applied_patch_set = apply_patch_set(&mut tree, patch_set); + let update = &applied_patch_set.updated[0]; + + assert_eq!(update.changed_ignore_unknown_instances, Some(true)); + assert!(update.changed_metadata.is_some()); +} + +#[test] +fn changed_ignore_unknown_instances_is_unset_when_only_internal_metadata_changes() { + let mut tree = empty_tree(); + let id = tree.get_root_id(); + + let patch_set = PatchSet { + updated_instances: vec![PatchUpdate { + id, + changed_name: None, + changed_class_name: None, + changed_properties: Default::default(), + changed_metadata: Some( + InstanceMetadata::new().relevant_paths(vec![PathBuf::from("some/new/path.lua")]), + ), + }], + ..Default::default() + }; + + let applied_patch_set = apply_patch_set(&mut tree, patch_set); + let update = &applied_patch_set.updated[0]; + + assert_eq!(update.changed_ignore_unknown_instances, None); + assert!(update.changed_metadata.is_some()); +} + fn empty_tree() -> RojoTree { RojoTree::new(InstanceSnapshot::new().name("ROOT").class_name("ROOT")) } diff --git a/src/web/interface.rs b/src/web/interface.rs index 8ec332ed..8701ddd0 100644 --- a/src/web/interface.rs +++ b/src/web/interface.rs @@ -54,12 +54,14 @@ impl<'a> SubscribeMessage<'a> { .updated .into_iter() .map(|update| { - let changed_metadata = update - .changed_metadata - .as_ref() - .map(InstanceMetadata::from_rojo_metadata); + let changed_metadata = + update + .changed_ignore_unknown_instances + .map(|ignore_unknown_instances| InstanceMetadata { + ignore_unknown_instances, + }); - let changed_properties = update + let changed_properties: UstrMap> = update .changed_properties .into_iter() .filter(|(_key, value)| property_filter(value.as_ref())) @@ -73,6 +75,12 @@ impl<'a> SubscribeMessage<'a> { changed_metadata, } }) + .filter(|update| { + update.changed_name.is_some() + || update.changed_class_name.is_some() + || !update.changed_properties.is_empty() + || update.changed_metadata.is_some() + }) .collect(); Self { @@ -312,3 +320,71 @@ pub enum ErrorResponseKind { Forbidden, InternalError, } + +#[cfg(test)] +mod tests { + use super::*; + + use rbx_dom_weak::ustr; + + use crate::snapshot::{AppliedPatchUpdate, InstanceSnapshot}; + + fn empty_tree() -> RojoTree { + RojoTree::new(InstanceSnapshot::new().name("ROOT").class_name("ROOT")) + } + + fn update_for(update: AppliedPatchUpdate) -> Vec { + let patch = AppliedPatchSet { + removed: Vec::new(), + added: Vec::new(), + updated: vec![update], + }; + + SubscribeMessage::from_patch_update(&empty_tree(), patch).updated + } + + #[test] + fn name_only_update_is_kept() { + let mut update = AppliedPatchUpdate::new(Ref::none()); + update.changed_name = Some("Renamed".to_owned()); + + let updated = update_for(update); + assert_eq!(updated.len(), 1); + assert_eq!(updated[0].changed_name.as_deref(), Some("Renamed")); + } + + #[test] + fn class_name_only_update_is_kept() { + let mut update = AppliedPatchUpdate::new(Ref::none()); + update.changed_class_name = Some(ustr("Folder")); + + let updated = update_for(update); + assert_eq!(updated.len(), 1); + assert_eq!(updated[0].changed_class_name, Some(ustr("Folder"))); + } + + #[test] + fn ignore_unknown_instances_change_is_reported() { + let mut update = AppliedPatchUpdate::new(Ref::none()); + update.changed_metadata = Some(RojoInstanceMetadata::new().ignore_unknown_instances(true)); + update.changed_ignore_unknown_instances = Some(true); + + let updated = update_for(update); + assert_eq!(updated.len(), 1); + assert_eq!( + updated[0] + .changed_metadata + .as_ref() + .map(|metadata| metadata.ignore_unknown_instances), + Some(true) + ); + } + + #[test] + fn internal_metadata_only_update_is_dropped() { + let mut update = AppliedPatchUpdate::new(Ref::none()); + update.changed_metadata = Some(RojoInstanceMetadata::new()); + + assert!(update_for(update).is_empty()); + } +} diff --git a/tests/tests/serve.rs b/tests/tests/serve.rs index 0d6f75cc..0d6fb25a 100644 --- a/tests/tests/serve.rs +++ b/tests/tests/serve.rs @@ -775,3 +775,80 @@ fn forced_parent() { assert_snapshot!("forced_parent_serialize_model", model); }); } + +#[test] +fn changed_metadata() { + run_serve_test("changed_metadata", |session, mut redactions| { + let _ = session.get_api_rojo().unwrap(); + + let project = |anchored: bool, ignore_unknown: Option, path: &str| { + let ignore_unknown = match ignore_unknown { + Some(value) => format!("\"$ignoreUnknownInstances\": {value},"), + None => String::new(), + }; + + format!( + r#"{{ + "name": "changed_metadata", + "tree": {{ + "$className": "DataModel", + "Workspace": {{ + "$className": "Workspace", + "Baseplate": {{ + "$className": "Part", + {ignore_unknown} + "$properties": {{ + "Anchored": {anchored} + }} + }}, + "Thing": {{ + "$path": "{path}" + }} + }} + }} +}} +"# + ) + }; + + let write_project = |contents: String| { + fs::write(session.path().join("default.project.json"), contents).unwrap(); + }; + + write_project(project(false, None, "a")); + let packet = session + .get_api_socket_packet(SocketPacketType::Messages, 0) + .unwrap(); + assert_yaml_snapshot!( + "changed_metadata_property_only", + packet.intern_and_redact(&mut redactions, ()) + ); + + write_project(project(false, Some(false), "a")); + let packet = session + .get_api_socket_packet(SocketPacketType::Messages, 1) + .unwrap(); + assert_yaml_snapshot!( + "changed_metadata_metadata_only", + packet.intern_and_redact(&mut redactions, ()) + ); + + write_project(project(true, Some(true), "a")); + let packet = session + .get_api_socket_packet(SocketPacketType::Messages, 2) + .unwrap(); + assert_yaml_snapshot!( + "changed_metadata_property_and_metadata", + packet.intern_and_redact(&mut redactions, ()) + ); + + write_project(project(true, Some(true), "b")); + let packet = session + .get_api_socket_packet(SocketPacketType::Messages, 3) + .unwrap(); + assert_yaml_snapshot!( + "changed_metadata_relevant_paths_only", + packet.intern_and_redact(&mut redactions, ()) + ); + }); +}