From 96dcb3eda9bd063b66da13cb20cb7086090b8f38 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Sat, 31 Oct 2026 15:37:26 +0100 Subject: [PATCH] Give each variable a unique custom property in CSS export (#1738) Found while testing crates/engine/src/cmd/variables_io.rs:~80-105 (css, non-tailwind `name` closure using css_var_of) with crates/engine/src/dev/codegen/css.rs:41 (css_var_name) Co-authored-by: Claude Opus 5.5 --- crates/engine/src/cmd/variables_io.rs | 6 ++- crates/engine/src/dev/codegen.rs | 2 +- crates/engine/src/dev/codegen/css.rs | 52 ++++++++++++++++++++++++- crates/engine/src/variables_io_tests.rs | 23 +++++++++++ 4 files changed, 78 insertions(+), 5 deletions(-) diff --git a/crates/engine/src/cmd/variables_io.rs b/crates/engine/src/cmd/variables_io.rs index dfdee7b80..1b3b4ee29 100644 --- a/crates/engine/src/cmd/variables_io.rs +++ b/crates/engine/src/cmd/variables_io.rs @@ -8,7 +8,7 @@ use figcraft_doc::{Collection, CollectionId, Document, Mode, ModeId, Variable, V use serde_json::{Value, json}; use super::CommandSpec; -use crate::codegen::{css_var_name, css_var_of}; +use crate::codegen::{CssNames, css_var_name}; use crate::library::{is_remote_collection, local_collections}; use crate::params::{get, string}; use crate::{EngineError, Result, Session, bad, spec}; @@ -73,7 +73,9 @@ fn css(doc: &Document, colls: &[&Collection], tailwind: bool) -> String { let flattener = vars.flattener(); // Tailwind theme names, unique per file and the same ones page code reads. let theme = crate::tailwind::Theme::new(doc); - let name = |v: &Variable| if tailwind { theme.name(v) } else { css_var_of(v) }; + // CSS names, unique per file and the same ones generated code reads. + let names = CssNames::new(doc); + let name = |v: &Variable| if tailwind { theme.name(v) } else { names.name(v) }; // Lengths need a unit where CSS reads them (`border-radius: var(--radius)`); opacity and // font weight don't. let roles = crate::token_roles::Roles::new(doc); diff --git a/crates/engine/src/dev/codegen.rs b/crates/engine/src/dev/codegen.rs index 925acdd0a..b9df04f12 100644 --- a/crates/engine/src/dev/codegen.rs +++ b/crates/engine/src/dev/codegen.rs @@ -30,8 +30,8 @@ pub(crate) use compose::{compose, kotlin_color}; pub(crate) use css::Tokens; #[cfg(test)] pub(crate) use css::css_decls; +pub use css::{CssNames, css_var_name, css_var_of}; use css::{css, tok}; -pub use css::{css_var_name, css_var_of}; pub(crate) use flutter::{FLUTTER_IMPORT, Place, flutter, flutter_indent}; use html::html; pub(crate) use html::{Markup, class_name}; diff --git a/crates/engine/src/dev/codegen/css.rs b/crates/engine/src/dev/codegen/css.rs index 087eb7905..eb9406f2a 100644 --- a/crates/engine/src/dev/codegen/css.rs +++ b/crates/engine/src/dev/codegen/css.rs @@ -1,3 +1,7 @@ +use std::collections::{HashMap, HashSet}; + +use figcraft_doc::VariableId; + use super::{ ContentAlign, CrossAlign, Document, EffectKind, LayoutMode, LetterSpacing, LineHeight, MainAlign, Node, NodeId, NodeKind, PADDING, PaintKind, TextAlignH, TextCase, TextDecoration, Variable, css_borders, num, @@ -13,13 +17,18 @@ pub(crate) struct Tokens { pub tailwind: bool, /// The file's theme names (computed on first use). theme: Option, + /// The file's CSS names (computed on first use). + names: Option, } impl Tokens { /// `var(--name, value)`, remembering the variable. fn var(&mut self, doc: &Document, variable: &Variable, value: String) -> String { - let name = - if self.tailwind { self.theme.get_or_insert_with(|| crate::tailwind::Theme::new(doc)).name(variable) } else { css_var_of(variable) }; + let name = if self.tailwind { + self.theme.get_or_insert_with(|| crate::tailwind::Theme::new(doc)).name(variable) + } else { + self.names.get_or_insert_with(|| CssNames::new(doc)).name(variable) + }; let out = format!("var({name}, {value})"); if !self.vars.iter().any(|(n, _)| *n == name) { self.vars.push((name, value)); @@ -60,6 +69,45 @@ pub fn css_var_of(v: &Variable) -> String { if valid { inner.chars().take(80).collect() } else { css_var_name(inner) } } +/// The file's variables' CSS custom properties, unique in the file: [`css_var_of`], and where an +/// earlier variable already has that name, the name with a numeric suffix (`--brand-primary-2`). +pub struct CssNames { + names: HashMap, +} + +impl CssNames { + pub fn new(doc: &Document) -> Self { + let vars = &doc.variables.variables; + let mut names = HashMap::with_capacity(vars.len()); + // Every plain name is reserved first, so a suffixed name never takes another's plain one; + // the next suffix per base keeps this linear. + let plain: Vec = vars.iter().map(css_var_of).collect(); + let mut taken: HashSet = HashSet::with_capacity(vars.len()); + let all: HashSet<&str> = plain.iter().map(String::as_str).collect(); + let mut next: HashMap<&str, usize> = HashMap::new(); + for (v, base) in vars.iter().zip(&plain) { + let mut name = base.clone(); + if !taken.insert(name.clone()) { + let k = next.entry(base.as_str()).or_insert(1); + loop { + *k += 1; + name = format!("{base}-{k}"); + if !all.contains(name.as_str()) && taken.insert(name.clone()) { + break; + } + } + } + names.insert(v.id.clone(), name); + } + CssNames { names } + } + + /// The custom property for `v` (its [`css_var_of`] name for a variable not in the file). + pub fn name(&self, v: &Variable) -> String { + self.names.get(&v.id).cloned().unwrap_or_else(|| css_var_of(v)) + } +} + /// `value`, or a reference to the variable bound to the first of `fields` that has one. pub(super) fn tok(doc: &Document, n: &Node, fields: &[&str], value: String, t: &mut Tokens) -> String { match fields.iter().find_map(|f| crate::devinspect::bound_variable(doc, n, f)) { diff --git a/crates/engine/src/variables_io_tests.rs b/crates/engine/src/variables_io_tests.rs index 57473a057..eba692802 100644 --- a/crates/engine/src/variables_io_tests.rs +++ b/crates/engine/src/variables_io_tests.rs @@ -639,3 +639,26 @@ fn stored_variable_ids_reset_only_the_inherited_parent_and_collection() { assert_eq!(result.overrides, std::collections::BTreeSet::from([light])); } } + +#[test] +fn css_export_gives_clashing_variable_names_their_own_property() { + let mut s = Session::new(); + ex(&mut s, "variable.createCollection", json!({"name": "Brand"})); + ex(&mut s, "variable.create", json!({"collection": "Brand", "name": "Brand/Primary", "type": "color", "value": "#ff0000"})); + let blue = ex(&mut s, "variable.create", json!({"collection": "Brand", "name": "Brand Primary", "type": "color", "value": "#0000ff"})); + ex(&mut s, "variable.create", json!({"collection": "Brand", "name": "link", "type": "color", "value": {"alias": blue["id"]}})); + let css = ex(&mut s, "variable.export", json!({"format": "css"}))["text"].as_str().unwrap_or_default().to_string(); + let decls: Vec<(&str, &str)> = + css.lines().filter_map(|l| l.trim().strip_prefix("--").and_then(|d| d.strip_suffix(';')).and_then(|d| d.split_once(": "))).collect(); + let mut names: Vec<&str> = decls.iter().map(|(n, _)| *n).collect(); + names.sort_unstable(); + names.dedup(); + assert_eq!(names.len(), decls.len(), "{css}"); + let red = decls.iter().find(|(_, v)| *v == "#FF0000").map(|(n, _)| *n); + let blue = decls.iter().find(|(_, v)| *v == "#0000FF").map(|(n, _)| *n); + // The first keeps its plain name; the alias reads the one its target is declared under. + assert_eq!(red, Some("brand-primary"), "{css}"); + let blue = blue.unwrap_or_default(); + assert_ne!(blue, "brand-primary", "{css}"); + assert!(css.contains(&format!("--link: var(--{blue});")), "{css}"); +}