mirror of
https://github.com/penpot/penpot.git
synced 2026-10-29 14:00:37 +00:00
🔥 Remove dead and deprecated code from component sync
- The :shapes-group reads. Sync skipped adding, removing and moving the children of a copy whose touched groups held :shapes-group. Nothing has set that group since 2024 (it was removed then, with a script that cleaned it from files), and the component contract makes structure follow the main, so the guards never applied. They go, with the omit-touched? argument they alone used in add-shape-to-instance, remove-shape and move-shape. - :fill-color, :fill-opacity and :stroke-width in ctk/sync-attrs, marked deprecated. Shapes keep fills and strokes in :fills and :strokes, migrations moved the old shape-level attributes there, the shape schema no longer has them, and no test syncs them any more, so sync no longer checks them on every shape. The TODO about concat-changes in generate-sync-file goes too: concat-changes no longer nests lazy sequences. - The inverse sync's swapped callback. An inverse sync pairs no swapped child, and every pair it makes counts as synced, so compare-children never calls it; the inverse direction now has no :swapped and passes nil. - The detach in generate-relocate of a copy shape moved out of its component. It only looked at the copy heads among the moved shapes (get-child-heads), and a head is never a non-head copy shape. No behavior change: the golden masters and the suites are unchanged. Benchmark (lower quantile, ms; baseline / now / ratio): deepest-edit-through-three-levels 35.2 / 38.2 / 1.08 edit-through-a-chain-of-dependents 68.9 / 65.0 / 0.94 library-update-into-copies 83.1 / 77.3 / 0.93 main-edit-into-10-copies 1.8 / 1.5 / 0.86 main-edit-into-100-copies 15.4 / 15.1 / 0.98 main-edit-into-1000-copies 153.1 / 154.8 / 1.01 variant-switch-into-copies 32.1 / 27.5 / 0.86 AI-assisted-by: claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
alonso.torres
parent
d7323cac4a
commit
64c666b336
@ -168,7 +168,7 @@
|
||||
(and early-return? (seq (:redo-changes changes))))
|
||||
changes
|
||||
(recur (next containers)
|
||||
(pcb/concat-changes ;;TODO Remove concat changes
|
||||
(pcb/concat-changes
|
||||
changes
|
||||
(generate-sync-container (pcb/empty-changes nil)
|
||||
asset-type
|
||||
@ -431,7 +431,8 @@
|
||||
`:children-main`):
|
||||
- `:sync-shape` syncs the pair itself;
|
||||
- `:only-inst`, `:only-main`, `:swapped` and `:moved` handle the
|
||||
children `compare-children` does not walk (see its callbacks);
|
||||
children `compare-children` does not walk (see its callbacks); an
|
||||
inverse sync pairs no swapped child, so it has no `:swapped`;
|
||||
- `:after-children` runs once the children are done.
|
||||
`:main-container` (ctx, shape-inst, shape-main -> container) finds the
|
||||
container of the main shape, and `:inverse?` goes to `compare-children`."
|
||||
@ -463,7 +464,7 @@
|
||||
(partial (:only-inst direction) step)
|
||||
(partial (:only-main direction) step)
|
||||
walk
|
||||
(partial (:swapped direction) step)
|
||||
(some-> (:swapped direction) (partial step))
|
||||
(partial (:moved direction) step)
|
||||
(:inverse? direction)
|
||||
(or (:inverse? direction) reset?))
|
||||
@ -515,40 +516,27 @@
|
||||
(change-remote-synced shape-inst container true))))
|
||||
|
||||
:only-inst
|
||||
(fn [{:keys [shape-inst container reset?]} changes child-inst]
|
||||
(fn [{:keys [container]} changes child-inst]
|
||||
(shape-log :trace (:id child-inst) container
|
||||
:msg "Only inst"
|
||||
:child-inst (str (:name child-inst) " " (pretty-uuid (:id child-inst))))
|
||||
(if-not (and (not reset?)
|
||||
(contains? (:touched shape-inst)
|
||||
:shapes-group))
|
||||
(remove-shape changes
|
||||
child-inst
|
||||
container
|
||||
(not reset?))
|
||||
changes))
|
||||
(remove-shape changes child-inst container))
|
||||
|
||||
:only-main
|
||||
(fn [{:keys [shape-inst container main-container children-main root-inst root-main reset?
|
||||
initial-root?]}
|
||||
(fn [{:keys [container main-container children-main root-inst root-main reset? initial-root?]}
|
||||
changes child-main]
|
||||
(shape-log :trace (:id child-main) main-container
|
||||
:msg "Only main"
|
||||
:child-main (str (:name child-main) " " (pretty-uuid (:id child-main))))
|
||||
(if-not (and (not reset?)
|
||||
(contains? (:touched shape-inst)
|
||||
:shapes-group))
|
||||
(add-shape-to-instance changes
|
||||
child-main
|
||||
(d/index-of children-main
|
||||
child-main)
|
||||
main-container
|
||||
container
|
||||
root-inst
|
||||
root-main
|
||||
(not reset?)
|
||||
(and (not initial-root?) reset?))
|
||||
changes))
|
||||
(add-shape-to-instance changes
|
||||
child-main
|
||||
(d/index-of children-main
|
||||
child-main)
|
||||
main-container
|
||||
container
|
||||
root-inst
|
||||
root-main
|
||||
(and (not initial-root?) reset?)))
|
||||
|
||||
:swapped
|
||||
(fn [{:keys [container root-inst root-main]} changes child-inst child-main]
|
||||
@ -566,7 +554,7 @@
|
||||
changes))
|
||||
|
||||
:moved
|
||||
(fn [{:keys [container children-inst children-main reset?]} changes child-inst child-main]
|
||||
(fn [{:keys [container children-inst children-main]} changes child-inst child-main]
|
||||
(shape-log :trace (:id child-inst) container
|
||||
:msg "Move"
|
||||
:child-inst (str (:name child-inst) " " (pretty-uuid (:id child-inst)))
|
||||
@ -576,8 +564,7 @@
|
||||
child-inst
|
||||
(d/index-of children-inst child-inst)
|
||||
(d/index-of children-main child-main)
|
||||
container
|
||||
(not reset?)))
|
||||
container))
|
||||
|
||||
:after-children
|
||||
(fn [changes {:keys [shape-inst shape-main library component reset?]}]
|
||||
@ -697,19 +684,7 @@
|
||||
|
||||
:only-main
|
||||
(fn [{:keys [main-container]} changes child-main]
|
||||
(remove-shape changes
|
||||
child-main
|
||||
main-container
|
||||
false))
|
||||
|
||||
:swapped
|
||||
(fn [{:keys [container]} changes child-inst child-main]
|
||||
(shape-log :trace (:id child-inst) container
|
||||
:msg "Match slot"
|
||||
:child-inst (str (:name child-inst) " " (pretty-uuid (:id child-inst)))
|
||||
:child-main (str (:name child-main) " " (pretty-uuid (:id child-main))))
|
||||
;; For now we don't make any sync here.
|
||||
changes)
|
||||
(remove-shape changes child-main main-container))
|
||||
|
||||
:moved
|
||||
(fn [{:keys [main-container children-inst children-main]} changes child-inst child-main]
|
||||
@ -718,8 +693,7 @@
|
||||
child-main
|
||||
(d/index-of children-main child-main)
|
||||
(d/index-of children-inst child-inst)
|
||||
main-container
|
||||
false))
|
||||
main-container))
|
||||
|
||||
:after-children
|
||||
(fn [changes {:keys [shape-inst synced-ids]}]
|
||||
@ -747,7 +721,9 @@
|
||||
the callback of each case. A child pairs with the main child it refers to;
|
||||
a swapped child pairs that way too, and takes `swapped-cb` in a normal sync,
|
||||
`both-cb` in a reset, and does not pair in an inverse sync, so the main
|
||||
child it replaces is removed and the swapped child is added in its place."
|
||||
child it replaces is removed and the swapped child is added in its place.
|
||||
An inverse sync (which passes `reset?`) never calls `swapped-cb`, so it
|
||||
passes nil."
|
||||
[changes shape-inst children-inst children-main container-inst only-inst-cb only-main-cb both-cb swapped-cb moved-cb inverse? reset?]
|
||||
(shape-log :trace (:id shape-inst) container-inst :msg "Compare children")
|
||||
(let [pair?
|
||||
@ -823,7 +799,7 @@
|
||||
(moved-cb child-inst' child-main)))))))))))))
|
||||
|
||||
(defn- add-shape-to-instance
|
||||
[changes component-shape index component-page container root-instance root-main omit-touched? set-remote-synced?]
|
||||
[changes component-shape index component-page container root-instance root-main set-remote-synced?]
|
||||
(shape-log :info (:id component-shape) component-page
|
||||
:msg (str "ADD [P " (pretty-uuid (:id container)) "] "
|
||||
(:name component-shape)
|
||||
@ -897,10 +873,7 @@
|
||||
:shapes all-parents}))
|
||||
changes' (reduce del-obj-change changes' new-shapes)]
|
||||
|
||||
(if (and (ctk/touched-group? parent-shape :shapes-group) omit-touched?)
|
||||
changes
|
||||
(-> changes'
|
||||
(pcb/apply-changes-local)))))
|
||||
(pcb/apply-changes-local changes')))
|
||||
|
||||
(defn- add-shape-to-main
|
||||
[changes shape index component component-container page root-instance root-main libraries]
|
||||
@ -1034,7 +1007,7 @@
|
||||
(pcb/apply-changes-local))))
|
||||
|
||||
(defn- remove-shape
|
||||
[changes shape container omit-touched?]
|
||||
[changes shape container]
|
||||
(shape-log :info (:id shape) container
|
||||
:msg (str "REMOVE-SHAPE "
|
||||
(if (cfh/page? container) "[P " "[C ")
|
||||
@ -1044,7 +1017,6 @@
|
||||
(pretty-uuid (:id shape))))
|
||||
(let [objects (get container :objects)
|
||||
parents (cfh/get-parent-ids objects (:id shape))
|
||||
parent (first parents)
|
||||
children (cfh/get-children-ids objects (:id shape))
|
||||
ids (-> (into [(:id shape)] children)
|
||||
(reverse)) ;; Remove from bottom to top
|
||||
@ -1082,13 +1054,10 @@
|
||||
changes'
|
||||
ids)]
|
||||
|
||||
(if (and (ctk/touched-group? parent :shapes-group) omit-touched?)
|
||||
changes
|
||||
(-> changes'
|
||||
(pcb/apply-changes-local)))))
|
||||
(pcb/apply-changes-local changes')))
|
||||
|
||||
(defn- move-shape
|
||||
[changes shape index-before index-after container omit-touched?]
|
||||
[changes shape index-before index-after container]
|
||||
(shape-log :info (:id shape) container
|
||||
:msg (str "MOVE "
|
||||
(if (cfh/page? container) "[P " "[C ")
|
||||
@ -1100,30 +1069,24 @@
|
||||
index-before
|
||||
" -> "
|
||||
index-after))
|
||||
(let [parent (ctn/get-shape container (:parent-id shape))
|
||||
|
||||
changes' (-> changes
|
||||
(update :redo-changes conj (make-change
|
||||
container
|
||||
{:type :mov-objects
|
||||
:parent-id (:parent-id shape)
|
||||
:shapes [(:id shape)]
|
||||
:index index-after
|
||||
:ignore-touched true
|
||||
:allow-altering-copies true}))
|
||||
(update :undo-changes conj (make-change
|
||||
container
|
||||
{:type :mov-objects
|
||||
:parent-id (:parent-id shape)
|
||||
:shapes [(:id shape)]
|
||||
:index index-before
|
||||
:ignore-touched true
|
||||
:allow-altering-copies true})))]
|
||||
|
||||
(if (and (ctk/touched-group? parent :shapes-group) omit-touched?)
|
||||
changes
|
||||
(-> changes'
|
||||
(pcb/apply-changes-local)))))
|
||||
(-> changes
|
||||
(update :redo-changes conj (make-change
|
||||
container
|
||||
{:type :mov-objects
|
||||
:parent-id (:parent-id shape)
|
||||
:shapes [(:id shape)]
|
||||
:index index-after
|
||||
:ignore-touched true
|
||||
:allow-altering-copies true}))
|
||||
(update :undo-changes conj (make-change
|
||||
container
|
||||
{:type :mov-objects
|
||||
:parent-id (:parent-id shape)
|
||||
:shapes [(:id shape)]
|
||||
:index index-before
|
||||
:ignore-touched true
|
||||
:allow-altering-copies true}))
|
||||
(pcb/apply-changes-local)))
|
||||
|
||||
(defn change-touched
|
||||
"Update the touched groups of the dest shape: clear them (`reset-touched?`,
|
||||
|
||||
@ -514,33 +514,24 @@
|
||||
;; be more efficient than using reduce and continuous data
|
||||
;; desturcturing.
|
||||
|
||||
;; Sets the correct components metadata for the moved shapes
|
||||
;; `shapes-to-detach` Detach from a component instance a shape that was inside a component and is moved outside
|
||||
;; Sets the correct components metadata for the moved copy heads
|
||||
;; `shapes-to-deroot` Removes the root flag from a component instance moved inside another component
|
||||
;; `shapes-to-reroot` Adds a root flag when a nested component instance is moved outside
|
||||
[shapes-to-detach shapes-to-deroot shapes-to-reroot]
|
||||
(reduce (fn [[shapes-to-detach shapes-to-deroot shapes-to-reroot] id]
|
||||
[shapes-to-deroot shapes-to-reroot]
|
||||
(reduce (fn [[shapes-to-deroot shapes-to-reroot] id]
|
||||
(let [shape (get objects id)
|
||||
parent (get objects parent-id)
|
||||
component-shape (ctn/get-component-shape objects shape)
|
||||
component-shape-parent (ctn/get-component-shape objects parent {:allow-main? true})
|
||||
root-parent (ctn/get-instance-root objects parent)
|
||||
|
||||
detach? (and (ctk/in-component-copy-not-head? shape)
|
||||
(not= (:id component-shape)
|
||||
(:id component-shape-parent)))
|
||||
deroot? (and (ctk/instance-root? shape)
|
||||
root-parent)
|
||||
reroot? (and (ctk/subinstance-head? shape)
|
||||
(not component-shape-parent))
|
||||
(not component-shape-parent))]
|
||||
|
||||
ids-to-detach (when detach?
|
||||
(cons id (cfh/get-children-ids objects id)))]
|
||||
|
||||
[(cond-> shapes-to-detach detach? (into ids-to-detach))
|
||||
(cond-> shapes-to-deroot deroot? (conj id))
|
||||
[(cond-> shapes-to-deroot deroot? (conj id))
|
||||
(cond-> shapes-to-reroot reroot? (conj id))]))
|
||||
[[] [] []]
|
||||
[[] []]
|
||||
(->> ids
|
||||
(mapcat #(ctn/get-child-heads objects %))
|
||||
(map :id)))
|
||||
@ -599,9 +590,6 @@
|
||||
(fn [shape]
|
||||
(assoc shape :masked-group false)))
|
||||
|
||||
;; Detach shapes moved out of their component
|
||||
(pcb/update-shapes shapes-to-detach ctk/detach-shape)
|
||||
|
||||
;; Make non root a component moved inside another one
|
||||
(pcb/update-shapes shapes-to-deroot
|
||||
(fn [shape]
|
||||
|
||||
@ -62,12 +62,6 @@
|
||||
:text-align :text-display-group
|
||||
:strokes :stroke-group
|
||||
|
||||
;; DEPRECATED: FIXME: this attrs are deprecated for a long time but
|
||||
;; we still have tests that uses this attribute for synchronization
|
||||
:stroke-width :stroke-group
|
||||
:fill-color :fill-group
|
||||
:fill-opacity :fill-group
|
||||
|
||||
:r1 :radius-group
|
||||
:r2 :radius-group
|
||||
:r3 :radius-group
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user