github/penpot
1
0
Fork 0
mirror of https://github.com/penpot/penpot.git synced 2026-10-29 05:50:35 +00:00
Code Issues Packages Projects Releases Wiki Activity

🐛 Skip shape validation when applying transforms

Browse Source 
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
This commit is contained in:
Eva Marco 2026-10-02 13:40:07 +02:00 committed by Andrés Moya
parent 2ba607cd5a
commit 6fca46caab
4 changed files with 50 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
common/src/app/common/logic/shapes.cljc
View File

@ -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

31
common/test/common_tests/files_changes_test.cljc
View File

@ -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 []}

10
frontend/src/app/main/data/workspace/modifiers.cljs
View File

@ -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]

8
frontend/src/app/main/data/workspace/shapes.cljs
View File

@ -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))