From 6fca46caabad77d52cf79e6fbaceff3fa9e8e872 Mon Sep 17 00:00:00 2001 From: Eva Marco Date: Fri, 2 Oct 2026 13:40:07 +0200 Subject: [PATCH] :bug: Skip shape validation when applying transforms generate-update-shapes validates every instance root of the updated shapes, including all their children. A variant switch syncs the copies of the changed main in separate commits, and the layout update it schedules runs 100 ms later through apply-modifiers. When that update lands before the sync ends, the validation finds a nested copy whose component-id still differs from its updated main, and raises. The layout update stops halfway, and the composable test suite fails in VariantSwitchPropagates. Transforms only write transform-attrs (geometry, fills, strokes), which can't break the references between components. Add a :skip-validation? option to generate-update-shapes and update-shapes, and set it in apply-modifiers* and apply-wasm-modifiers. AI-assisted-by: claude-opus-5-5 --- common/src/app/common/logic/shapes.cljc | 8 +++-- .../test/common_tests/files_changes_test.cljc | 31 +++++++++++++++++++ .../app/main/data/workspace/modifiers.cljs | 10 ++++-- .../src/app/main/data/workspace/shapes.cljs | 8 +++-- 4 files changed, 50 insertions(+), 7 deletions(-) diff --git a/common/src/app/common/logic/shapes.cljc b/common/src/app/common/logic/shapes.cljc index 4ca070f672..26bb1194ce 100644 --- a/common/src/app/common/logic/shapes.cljc +++ b/common/src/app/common/logic/shapes.cljc @@ -84,7 +84,7 @@ (reduce check-shape changes mod-obj-changes))) (defn generate-update-shapes - [changes ids update-fn objects {:keys [attrs changed-sub-attr changed-item-index ignore-tree ignore-touched with-objects? translation? skip-grid-reassignment? extra-context]}] + [changes ids update-fn objects {:keys [attrs changed-sub-attr changed-item-index ignore-tree ignore-touched with-objects? translation? skip-grid-reassignment? skip-validation? extra-context]}] (let [changes (reduce (fn [changes id] (let [opts {:attrs attrs @@ -112,7 +112,11 @@ (pcb/get-page-id changes)) modified-components (ctn/get-all-instance-roots objects ids) - changes (if (and page-id (seq modified-components)) + ;; skip-validation? is for updates that can't break the references + ;; between components (e.g. geometry only). The validation covers + ;; the whole instance, so it could stop on copies that other + ;; changes have not synced yet. + changes (if (and page-id (seq modified-components) (not skip-validation?)) (pcb/validate-shapes changes page-id modified-components diff --git a/common/test/common_tests/files_changes_test.cljc b/common/test/common_tests/files_changes_test.cljc index da98d1b1ef..8ed2c59d19 100644 --- a/common/test/common_tests/files_changes_test.cljc +++ b/common/test/common_tests/files_changes_test.cljc @@ -1211,6 +1211,37 @@ (t/is (seq (:redo-changes changes))) (t/is (not-any? #(= :validate-shapes (:type %)) (:redo-changes changes))))) +(t/deftest update-shapes-on-page-validates-the-instance + (let [file (-> (thf/sample-file :file1) + (tho/add-simple-component :c01 :m01 :r01)) + page (thf/current-page file) + main (ths/get-shape file :m01) + + changes (cls/generate-update-shapes (pcb/empty-changes nil (:id page)) + [(:id main)] + #(assoc % :x 10) + (:objects page) + {})] + + (t/is (some #(= :validate-shapes (:type %)) (:redo-changes changes))))) + +(t/deftest update-shapes-with-skip-validation-does-not-validate + ;; Transforms only change geometry, so they skip the validation of the + ;; whole instance, which could stop on copies not synced yet. + (let [file (-> (thf/sample-file :file1) + (tho/add-simple-component :c01 :m01 :r01)) + page (thf/current-page file) + main (ths/get-shape file :m01) + + changes (cls/generate-update-shapes (pcb/empty-changes nil (:id page)) + [(:id main)] + #(assoc % :x 10) + (:objects page) + {:skip-validation? true})] + + (t/is (seq (:redo-changes changes))) + (t/is (not-any? #(= :validate-shapes (:type %)) (:redo-changes changes))))) + (t/deftest skip-validate-changes (let [changes [{:type :mod-obj :id (uuid/next)} {:type :validate-shapes :page-id (uuid/next) :shape-ids []} diff --git a/frontend/src/app/main/data/workspace/modifiers.cljs b/frontend/src/app/main/data/workspace/modifiers.cljs index 0e4bb0a5ce..006a512d61 100644 --- a/frontend/src/app/main/data/workspace/modifiers.cljs +++ b/frontend/src/app/main/data/workspace/modifiers.cljs @@ -922,7 +922,10 @@ (assoc :translation? translation?) ;; Attributes that can change in the transform. This ;; way we don't have to check all the attributes - (assoc :attrs transform-attrs)) + (assoc :attrs transform-attrs) + ;; Transforms only change geometry, which can't break the + ;; references between components + (assoc :skip-validation? true)) modif-tree (propagate-structure-modifiers modif-tree (dsh/lookup-page-objects state)) @@ -1072,7 +1075,10 @@ (assoc :ignore-tree ignore-tree) ;; Attributes that can change in the transform. This ;; way we don't have to check all the attributes - (assoc :attrs transform-attrs)) + (assoc :attrs transform-attrs) + ;; Transforms only change geometry, which can't break the + ;; references between components + (assoc :skip-validation? true)) update-shape (fn [shape] diff --git a/frontend/src/app/main/data/workspace/shapes.cljs b/frontend/src/app/main/data/workspace/shapes.cljs index b11adf55cc..a9c24a5cdb 100644 --- a/frontend/src/app/main/data/workspace/shapes.cljs +++ b/frontend/src/app/main/data/workspace/shapes.cljs @@ -110,7 +110,7 @@ ([ids update-fn {:keys [reg-objects? save-undo? stack-undo? attrs ignore-tree page-id ignore-touched undo-group with-objects? changed-sub-attr changed-item-index - translation? skip-grid-reassignment? skip-component-sync? resize-ids] + translation? skip-grid-reassignment? skip-component-sync? resize-ids skip-validation?] :or {reg-objects? false save-undo? true stack-undo? false @@ -152,7 +152,8 @@ :ignore-tree ignore-tree :ignore-touched ignore-touched :with-objects? with-objects? - :skip-grid-reassignment? skip-grid-reassignment?}) + :skip-grid-reassignment? skip-grid-reassignment? + :skip-validation? skip-validation?}) (cond-> reg-objects? (pcb/resize-parents (or resize-ids ids))) (pcb/set-translation? translation?) (pcb/set-skip-component-sync? skip-component-sync?))))] @@ -211,7 +212,7 @@ {:as props :keys [reg-objects? save-undo? stack-undo? attrs ignore-tree page-id ignore-touched undo-group with-objects? changed-sub-attr changed-item-index translation? - skip-grid-reassignment? skip-component-sync? resize-ids extra-context] + skip-grid-reassignment? skip-component-sync? resize-ids skip-validation? extra-context] :or {reg-objects? false save-undo? true stack-undo? false @@ -252,6 +253,7 @@ :with-objects? with-objects? :translation? translation? :skip-grid-reassignment? skip-grid-reassignment? + :skip-validation? skip-validation? :extra-context extra-context}) (cond-> undo-group (pcb/set-undo-group undo-group))