From e7a65b44796d8f578f1110416f2112cd2e4fd36d Mon Sep 17 00:00:00 2001 From: "alonso.torres" Date: Tue, 13 Oct 2026 14:14:45 +0200 Subject: [PATCH] :bug: Sync a reordered copy child into its main on update main In an inverse sync, when a copy child paired with a main child further along the main's children, compare-children asked whether the copy child was the main of the main child, with the arguments of ctk/is-main-of? reversed. The answer was always no, so the pair took the swapped callback, which does nothing in an inverse sync, and only the move ran: the overrides of that copy child never reached the main. The UI does not let a copy reorder its children, so it only showed when the order differed for another reason, such as the undo of a sync reorder. The inverse branch now asks synced-pair? with the main child first, as the direct branch does. Benchmark (lower quantile, ms; baseline / now / ratio): deepest-edit-through-three-levels 35.2 / 35.5 / 1.01 edit-through-a-chain-of-dependents 68.9 / 66.1 / 0.96 library-update-into-copies 83.1 / 77.0 / 0.93 main-edit-into-10-copies 1.8 / 1.6 / 0.92 main-edit-into-100-copies 15.4 / 16.2 / 1.05 main-edit-into-1000-copies 153.1 / 156.4 / 1.02 variant-switch-into-copies 32.1 / 28.3 / 0.88 AI-assisted-by: claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 (1M context) --- common/src/app/common/logic/libraries.cljc | 6 +-- .../common_tests/logic/comp_sync_test.cljc | 42 +++++++++++++++++++ 2 files changed, 45 insertions(+), 3 deletions(-) diff --git a/common/src/app/common/logic/libraries.cljc b/common/src/app/common/logic/libraries.cljc index 16e237a4f9..c193d900f3 100644 --- a/common/src/app/common/logic/libraries.cljc +++ b/common/src/app/common/logic/libraries.cljc @@ -1256,13 +1256,13 @@ :else (if inverse? - (let [is-main? (ctk/is-main-of? child-inst child-main')] + (let [synced? (synced-pair? child-main' child-inst)] (recur (next children-inst) (remove #(= (:id %) (:id child-main')) children-main) (cond-> changes - is-main? + synced? (both-cb child-inst child-main') - (not is-main?) + (not synced?) (swapped-cb child-inst child-main') :always (moved-cb child-inst child-main')))) diff --git a/common/test/common_tests/logic/comp_sync_test.cljc b/common/test/common_tests/logic/comp_sync_test.cljc index 84397671de..cbfc8f3905 100644 --- a/common/test/common_tests/logic/comp_sync_test.cljc +++ b/common/test/common_tests/logic/comp_sync_test.cljc @@ -18,6 +18,7 @@ [app.common.test-helpers.ids-map :as thi] [app.common.test-helpers.shapes :as ths] [app.common.types.component :as ctk] + [app.common.types.container :as ctn] [app.common.types.shape-tree :as ctst] [clojure.test :as t])) @@ -539,3 +540,44 @@ ;; A position-only change in the main component must not propagate to copies ;; and therefore must produce no redo-changes. (t/is (empty? (:redo-changes sync-changes))))) + +(t/deftest test-update-main-from-a-copy-with-reordered-children + (let [;; ==== Setup + file (-> (thf/sample-file :file1) + (tho/add-component-with-many-children-and-copy :component1 + :main-root + [:main-child1 :main-child2] + :copy-root + :copy-root-params {:children-labels [:copy-child1 :copy-child2]})) + page (thf/current-page file) + + ;; The UI never reorders the children of a copy, so the order is + ;; set directly: the copy holds them in the reverse order of the main. + file (update-in file [:data :pages-index (:id page) :objects (thi/id :copy-root) :shapes] + (comp vec reverse)) + + changes (cls/generate-update-shapes (pcb/empty-changes nil (:id page)) + #{(thi/id :copy-child2)} + #(assoc % :hidden true) + (:objects page) + {}) + file (thf/apply-changes file changes) + + ;; ==== Action + page (thf/current-page file) + container (ctn/make-container page :page) + changes (-> (pcb/empty-changes) + (pcb/with-container container) + (cll/generate-sync-shape-inverse (:data file) + {(:id file) file} + container + (thi/id :copy-root))) + file' (thf/apply-changes file changes) + + ;; ==== Get + main-root' (ths/get-shape file' :main-root) + main-child2' (ths/get-shape file' :main-child2)] + + ;; ==== Check + (t/is (true? (:hidden main-child2'))) + (t/is (= [(thi/id :main-child2) (thi/id :main-child1)] (:shapes main-root')))))