From 7a8fa7a9cb98e228a753e665d463044e8f57e999 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20Moya?= Date: Tue, 26 May 2026 17:12:03 +0200 Subject: [PATCH] :bug: Token remap preserves child component sync after renaming a token group (#9566) (#9878) --- .../main/data/workspace/tokens/remapping.cljs | 122 ++++++++++-------- .../tokens/logic/token_remapping_test.cljs | 109 ++++++++++++++++ 2 files changed, 175 insertions(+), 56 deletions(-) diff --git a/frontend/src/app/main/data/workspace/tokens/remapping.cljs b/frontend/src/app/main/data/workspace/tokens/remapping.cljs index 18dab649ff..0726037d93 100644 --- a/frontend/src/app/main/data/workspace/tokens/remapping.cljs +++ b/frontend/src/app/main/data/workspace/tokens/remapping.cljs @@ -92,70 +92,80 @@ ;; Token Remapping Core Logic ;; ========================== +(defn build-remap-changes + "Build the pending changes required to rename a token from `old-token-name` + to `new-token-name`, covering applied-token references on shapes and alias + references in other tokens. Updates to copy shapes are committed with + `:ignore-touched true` so the rename does not flip sync groups into the + :touched set and silently break later main→copy propagation." + [file-data old-token-name new-token-name & {:keys [undo-group]}] + (let [scan-results (scan-workspace-token-references file-data old-token-name) + tokens-lib (:tokens-lib file-data) + sets (ctob/get-sets tokens-lib) + tokens-with-sets (mapcat (fn [set] + (map (fn [token] + {:token token :set set}) + (vals (ctob/get-tokens tokens-lib (ctob/get-id set))))) + sets) + + ;; Group applied token references by container + refs-by-container (group-by :container (:applied-tokens scan-results)) + + ;; Use apply-token logic to update shapes for both direct and alias references + shape-changes + (reduce-kv + (fn [changes container refs] + (let [shape-ids (map :shape-id refs) + ;; Find the correct token to apply (new or alias) + token (or (some #(when (= (:name (:token %)) new-token-name) %) tokens-with-sets) + (some #(when (= (:name (:token %)) old-token-name) %) tokens-with-sets)) + attributes (set (map :attribute refs))] + (if token + ;; Create a new independent changes so we can call `with-file-data` after `with-container` + ;; otherwise it causes probelms looking up for objects + (let [container-changes + (-> (pcb/empty-changes) + (pcb/with-container container) + (pcb/with-file-data file-data) + (pcb/update-shapes shape-ids + (fn [shape] + (update shape :applied-tokens + #(merge % (cft/attributes-map attributes (:token token))))) + {:ignore-touched true}))] + (pcb/concat-changes changes container-changes)) + changes))) + (-> (pcb/empty-changes) + (pcb/with-library-data file-data)) + refs-by-container) + + ;; Create changes for updating token alias references + token-changes + (reduce + (fn [changes ref] + (let [source-token-id (:source-token-id ref)] + (when-let [{:keys [token set]} (some #(when (= (:id (:token %)) source-token-id) %) tokens-with-sets)] + (let [old-value (:value token) + new-value (cto/update-token-value-references old-value old-token-name new-token-name)] + (pcb/set-token changes (ctob/get-id set) (:id token) + (assoc token :value new-value)))))) + shape-changes + (:token-aliases scan-results))] + + (cond-> token-changes + (some? undo-group) + (assoc :undo-group undo-group)))) + (defn remap-tokens "Main function to remap all token references when a token name changes" [old-token-name new-token-name & {:keys [undo-group]}] (ptk/reify ::remap-tokens ptk/WatchEvent (watch [_ state _] - (let [file-data (dh/lookup-file-data state) - scan-results (scan-workspace-token-references file-data old-token-name) - tokens-lib (:tokens-lib file-data) - sets (ctob/get-sets tokens-lib) - tokens-with-sets (mapcat (fn [set] - (map (fn [token] - {:token token :set set}) - (vals (ctob/get-tokens tokens-lib (ctob/get-id set))))) - sets) - - ;; Group applied token references by container - refs-by-container (group-by :container (:applied-tokens scan-results)) - - ;; Use apply-token logic to update shapes for both direct and alias references - shape-changes (reduce-kv - (fn [changes container refs] - (let [shape-ids (map :shape-id refs) - ;; Find the correct token to apply (new or alias) - token (or (some #(when (= (:name (:token %)) new-token-name) %) tokens-with-sets) - (some #(when (= (:name (:token %)) old-token-name) %) tokens-with-sets)) - attributes (set (map :attribute refs))] - (if token - ;; Create a new independent changes so we can call `with-file-data` after `with-container` - ;; otherwise it causes probelms looking up for objects - (let [container-changes - (-> (pcb/empty-changes) - (pcb/with-container container) - (pcb/with-file-data file-data) - (pcb/update-shapes shape-ids - (fn [shape] - (update shape :applied-tokens - #(merge % (cft/attributes-map attributes (:token token)))))))] - (pcb/concat-changes changes container-changes)) - changes))) - (-> (pcb/empty-changes) - (pcb/with-library-data file-data)) - refs-by-container) - - ;; Create changes for updating token alias references - token-changes (reduce - (fn [changes ref] - (let [source-token-id (:source-token-id ref)] - (when-let [{:keys [token set]} (some #(when (= (:id (:token %)) source-token-id) %) tokens-with-sets)] - (let [old-value (:value token) - new-value (cto/update-token-value-references old-value old-token-name new-token-name)] - (pcb/set-token changes (ctob/get-id set) (:id token) - (assoc token :value new-value)))))) - shape-changes - (:token-aliases scan-results)) - - token-changes - (cond-> token-changes (some? undo-group) (assoc :undo-group undo-group))] - + (let [file-data (dh/lookup-file-data state) + token-changes (build-remap-changes file-data old-token-name new-token-name :undo-group undo-group)] (log/info :hint "token-remapping" :old-name old-token-name - :new-name new-token-name - :references-count (:total-references scan-results)) - + :new-name new-token-name) (rx/of (dch/commit-changes token-changes)))))) (defn bulk-remap-tokens diff --git a/frontend/test/frontend_tests/tokens/logic/token_remapping_test.cljs b/frontend/test/frontend_tests/tokens/logic/token_remapping_test.cljs index 318521f6d7..0d5c2ddf84 100644 --- a/frontend/test/frontend_tests/tokens/logic/token_remapping_test.cljs +++ b/frontend/test/frontend_tests/tokens/logic/token_remapping_test.cljs @@ -9,6 +9,7 @@ [app.common.test-helpers.compositions :as ctho] [app.common.test-helpers.files :as cthf] [app.common.test-helpers.ids-map :as cthi] + [app.common.test-helpers.shapes :as cths] [app.common.test-helpers.tokens :as ctht] [app.common.types.token :as cto] [app.common.types.tokens-lib :as ctob] @@ -116,3 +117,111 @@ (t/testing "valid new name should be valid" (let [result (dwtr/validate-token-remapping "color.primary" "brand.primary")] (t/is (true? (:valid? result))))))) + +(defn- setup-file-with-component-copy-and-token + "Create a file containing a component, an instance copy of it, and a single + `color.primary` token already applied to both the main and the copy shape. + Mirrors the in-app state right after a user applies a token to a main and + component sync has propagated the applied-tokens map to the copy." + [] + (let [color-primary {:id (cthi/new-id! :color-primary) + :name "color.primary" + :value "#FF0000" + :type :color} + color-secondary {:id (cthi/new-id! :color-secondary) + :name "color.secondary" + :value "#00FF00" + :type :color}] + (-> (cthf/sample-file :file-1 :page-label :page-1) + (ctho/add-simple-component-with-copy :component1 + :main-root + :main-child + :copy-root + :copy-root-params {:children-labels [:copy-child]}) + (assoc-in [:data :tokens-lib] + (-> (ctob/make-tokens-lib) + (ctob/add-theme (ctob/make-token-theme :name "Theme A" :sets #{"Set A"})) + (ctob/set-active-themes #{"/Theme A"}) + (ctob/add-set (ctob/make-token-set :id (cthi/new-id! :set-a) + :name "Set A")) + (ctob/add-token (cthi/id :set-a) + (ctob/make-token color-primary)) + (ctob/add-token (cthi/id :set-a) + (ctob/make-token color-secondary)))) + (ctht/apply-token-to-shape :main-child "color.primary" [:fill] [:fill] "#FF0000") + (ctht/apply-token-to-shape :copy-child "color.primary" [:fill] [:fill] "#FF0000")))) + +(t/deftest test-remap-tokens-finds-both-main-and-copy + (t/testing "scan should find applied-token references on both main and copy shapes" + (let [file (setup-file-with-component-copy-and-token) + scan-results (dwtr/scan-workspace-token-references (:data file) "color.primary") + shape-ids (set (map :shape-id (:applied-tokens scan-results)))] + (t/is (= 2 (count (:applied-tokens scan-results)))) + (t/is (contains? shape-ids (cthi/id :main-child))) + (t/is (contains? shape-ids (cthi/id :copy-child)))))) + +(t/deftest test-remap-tokens-does-not-touch-copy + (t/testing "renaming a token must not flip the copy's sync group to :touched" + (let [;; The production flow first renames the token in the tokens-lib, + ;; then dispatches remap-tokens to update applied-token references + ;; on shapes. Mirror that order here. + file (-> (setup-file-with-component-copy-and-token) + (update-in [:data :tokens-lib] + (fn [lib] + (ctob/update-token lib + (cthi/id :set-a) + (cthi/id :color-primary) + #(assoc % :name "colors.primary"))))) + changes (dwtr/build-remap-changes (:data file) + "color.primary" + "colors.primary") + file' (cthf/apply-changes file changes) + main-child' (cths/get-shape file' :main-child) + copy-child' (cths/get-shape file' :copy-child)] + + (t/is (= "colors.primary" (get-in main-child' [:applied-tokens :fill]))) + (t/is (= "colors.primary" (get-in copy-child' [:applied-tokens :fill]))) + + ;; If the rename marks :fill-group as touched on the copy, future + ;; main→copy propagation will skip it — that is the #9495 regression. + (t/is (not (contains? (set (:touched copy-child')) :fill-group)))))) + +(t/deftest test-remap-preserves-copy-sync-of-later-token-apply + (t/testing "End-to-end #9495 scenario: after a token group rename + REMAP, + applying a new token to the main must still propagate to the copy." + (let [;; 1. Build file with two tokens, a component, and color.primary + ;; already applied to both main-child and copy-child. + file (setup-file-with-component-copy-and-token) + + ;; 2. Rename the token group: color.primary -> colors.primary, + ;; color.secondary -> colors.secondary (mimics user editing the + ;; group name in the tokens panel). + file (update-in file [:data :tokens-lib] + (fn [lib] + (-> lib + (ctob/update-token (cthi/id :set-a) (cthi/id :color-primary) + #(assoc % :name "colors.primary")) + (ctob/update-token (cthi/id :set-a) (cthi/id :color-secondary) + #(assoc % :name "colors.secondary"))))) + + ;; 3. REMAP runs to update applied-token names on shapes. + file (cthf/apply-changes + file + (dwtr/build-remap-changes (:data file) + "color.primary" + "colors.primary")) + + ;; 4. User then applies colors.secondary to the main (Step 7). + file (ctht/apply-token-to-shape file :main-child "colors.secondary" + [:fill] [:fill] "#00FF00") + + ;; 5. Component sync propagates the main's change to the copy. + file (ctho/propagate-component-changes file :component1) + + copy-child' (cths/get-shape file :copy-child)] + + ;; Step 8 of the issue: the copy must reflect the newly applied token. + ;; Pre-fix, REMAP marks :fill-group as touched on the copy, so the + ;; subsequent sync silently skips :applied-tokens and the copy is left + ;; pointing at colors.primary. + (t/is (= "colors.secondary" (get-in copy-child' [:applied-tokens :fill]))))))