From 017517fbb9c1b1dfa96ff23ed9e5f5f8363b23da Mon Sep 17 00:00:00 2001 From: MortenHusted Date: Sun, 18 Oct 2026 23:44:37 +0200 Subject: [PATCH] #847: an instance's values are kept under the original definition An assignment and the layers that read it can name different copies of one definition (the set's own id on one side, a variant's copy on the other). The values were kept under the id the assignment names and read under the id the layer names, so such a value never reached its layer. Both sides now go through prop_root, the definition the copies were made from (CodeRabbit on #847). In Untitled UI, 143 assignments name a copy that no layer reads while another copy of it is read. Co-Authored-By: Claude Opus 5.5 --- crates/format/src/fig/map.rs | 22 ++++++++++++++----- crates/format/src/fig/prop_value_tests.rs | 26 ++++++++++++++++++----- 2 files changed, 38 insertions(+), 10 deletions(-) diff --git a/crates/format/src/fig/map.rs b/crates/format/src/fig/map.rs index 22043650..8e467151 100644 --- a/crates/format/src/fig/map.rs +++ b/crates/format/src/fig/map.rs @@ -660,6 +660,18 @@ impl<'a> Mapper<'a> { matches!(self.prop_def(&guid(def)?), Some((_, "INSTANCE_SWAP"))).then_some(()) } + /// The definition a copy of it was made from, followed to the original: the values an instance + /// sets are kept under it, since an assignment and the layers that read it may name different + /// copies of one definition. + fn prop_root(&self, def: &str) -> String { + let mut def = def; + for _ in 0..=MAX_DEPTH { + let Some(parent) = self.prop_parents.get(def) else { break }; + def = parent; + } + def.to_string() + } + /// The definition `def` names: its own, or the set's for a variant's copy of it. fn prop_def(&self, def: &str) -> Option<&(String, &'a str)> { let mut def = def; @@ -926,7 +938,7 @@ impl<'a> Mapper<'a> { } self.prop_refs(&mut o, &view, inst); let slot_content = if own == "FRAME" { - prop_ref(&view, "SLOT_CONTENT_ID").and_then(|def| inst.and_then(|i| i.slot_contents.get(&def).cloned())) + prop_ref(&view, "SLOT_CONTENT_ID").and_then(|def| inst.and_then(|i| i.slot_contents.get(&self.prop_root(&def)).cloned())) } else { None }; @@ -1914,7 +1926,7 @@ impl<'a> Mapper<'a> { { refs.insert(field.into(), json!(name)); } - let Some(val) = inst.and_then(|i| i.props.get(&def)) else { continue }; + let Some(val) = inst.and_then(|i| i.props.get(&self.prop_root(&def))) else { continue }; match field { "visible" => { if let Some(b) = val.get("boolValue").and_then(Kv::bool) { @@ -1951,7 +1963,7 @@ impl<'a> Mapper<'a> { // An instance swap property value picks the main. if let Some(i) = outer && let Some(def) = prop_ref(v, "OVERRIDDEN_SYMBOL_ID") - && let Some(g) = i.props.get(&def).and_then(|p| prop_guid(p)) + && let Some(g) = i.props.get(&self.prop_root(&def)).and_then(|p| prop_guid(p)) { sym = Some(g); } @@ -2046,7 +2058,7 @@ impl<'a> Mapper<'a> { let mut slot_contents: BTreeMap = outer.map(|i| (*i.slot_contents).clone()).unwrap_or_default(); for a in assignments { if let (Some(d), Some(val)) = (a.get("defID").and_then(guid), assigned(a)) { - all_props.entry(d).or_insert(val); + all_props.entry(self.prop_root(&d)).or_insert(val); } let Some(def) = a.get("defID").and_then(guid) else { continue }; if self.prop_def(&def).is_none_or(|(_, ty)| *ty != "SLOT") { @@ -2070,7 +2082,7 @@ impl<'a> Mapper<'a> { self.warn("slot content assignment target is missing or is not slot content"); continue; } - slot_contents.entry(def).or_insert(target); + slot_contents.entry(self.prop_root(&def)).or_insert(target); } let mut symbols = outer.map(|i| i.symbols.clone()).unwrap_or_default(); symbols.push(sym.clone()); diff --git a/crates/format/src/fig/prop_value_tests.rs b/crates/format/src/fig/prop_value_tests.rs index 577b811a..24248d32 100644 --- a/crates/format/src/fig/prop_value_tests.rs +++ b/crates/format/src/fig/prop_value_tests.rs @@ -73,9 +73,10 @@ fn instance_props(doc: &figcraft_doc::Document) -> &std::collections::BTreeMap Kv<'static> { + message(vec![ node( 9, "FRAME", @@ -101,10 +102,15 @@ fn variant_layers_bind_to_the_set_properties_their_copies_name() { Some(2), vec![ ("symbolData", o(vec![("symbolID", g(10))])), - ("componentPropAssignments", Kv::Array(vec![o(vec![("defID", g(71)), ("value", o(vec![("boolValue", Kv::Bool(false))]))])])), + ("componentPropAssignments", Kv::Array(vec![o(vec![("defID", g(assigned)), ("value", o(vec![("boolValue", Kv::Bool(false))]))])])), ], ), - ]); + ]) +} + +#[test] +fn variant_layers_bind_to_the_set_properties_their_copies_name() { + let msg = field_set(71); let (rest, _, _) = to_rest(&msg, "Synthetic", crate::rest::MAX_NODES); assert_eq!(by_id(&rest["document"], "9:9")["componentPropertyDefinitions"]["Hint#9:70"]["type"], "BOOLEAN"); assert_eq!(by_id(&rest["document"], "9:11")["componentPropertyReferences"]["visible"], "Hint#9:70"); @@ -116,6 +122,16 @@ fn variant_layers_bind_to_the_set_properties_their_copies_name() { assert_eq!(instance_props(&doc).get("Hint"), Some(&Value::Bool(false))); } +/// An assignment reaches layers that read another copy of its definition: here the instance sets +/// the set's own definition while the variant's layer reads the variant's copy. +#[test] +fn assignments_reach_layers_that_read_another_copy_of_their_definition() { + let (rest, _, _) = to_rest(&field_set(70), "Synthetic", crate::rest::MAX_NODES); + let instance = by_id(&rest["document"], "9:20"); + assert_eq!(instance["componentProperties"]["Hint#9:70"]["value"], false); + assert_eq!(by_id(instance, "I9:20;9:11")["visible"], false); +} + #[test] fn property_values_written_only_as_variables_are_applied() { let msg = message(vec![