fix: drop stale canvas element when ids collide with a different tag#15
Merged
fix: drop stale canvas element when ids collide with a different tag#15
Conversation
Loading a project file while another doc was open could leave visible residue from the previous document — kinetic-typography animations, particle-button effects, gaussian-blur layers etc. — inside what should be a fresh tree. Repro: open the kinetic template, then File → Open another project; effects from the kinetic template bleed into the new view. Root cause: every Bareforge document starts its `:next-id` counter at 0, so two unrelated documents share the same id space (`n_0`, `n_1`, …). When `apply-loaded-project!` swaps `:document`, `patch!` walks the new doc and `upsert-node!` finds an existing element in `node-index` for each colliding id. The fast path just diffs attrs / props / text — it never checked whether the existing element's TAG matches the new node's tag. If kinetic's `n_5` was an `x-particle-button` and the new doc's `n_5` is a plain `x-button`, the existing custom element keeps its tag (and all the side-effect state attached to it: animation loops, canvas pixels, internal listeners, portaled scrims). Only its attrs get diffed. upsert-node! now checks `(.-localName prior)` against the new node's `:tag`. On mismatch it removes the old element and deletes the index entry, so the create-path runs and a fresh custom element takes over. The full subtree rebuilds because the children's parent pointer changes. Manual repro confirmed the bug; the existing 584-test gate is unchanged because the shared-id-different-tag scenario has no node-test coverage (canvas tests live in browser-test land, which the project hasn't wired up yet). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Loading a project file while another doc was open could leave
visible residue from the previous document — kinetic-typography
animations, particle-button effects, gaussian-blur layers, etc. —
inside what should be a fresh tree.
Repro: open the Kinetic Showcase template, then File → Open
another project. Effects from the kinetic template bleed into the
new view.
Root cause
Every Bareforge document starts its `:next-id` counter at 0,
so two unrelated documents share the same id space (`n_0`,
`n_1`, …). When `apply-loaded-project!` swaps `:document`,
`patch!` walks the new doc and `upsert-node!` finds an
existing element in `node-index` for each colliding id. The
fast path just diffs attrs / props / text — it never checked
whether the existing element's TAG matches the new node's tag.
If kinetic's `n_5` was an `x-particle-button` and the new
doc's `n_5` is a plain `x-button`, the existing custom element
keeps its tag (and all the side-effect state attached to it:
animation loops, canvas pixels, internal listeners, portaled
scrims). Only its attrs get diffed.
Fix
`upsert-node!` now checks `(.-localName prior)` against the
new node's `:tag`. On mismatch it calls `rec/remove-el!`,
deletes the index entry, and falls through to the create path so
a fresh custom element takes over. The full subtree rebuilds
because the children's parent pointer changes.
Test plan
The shared-id-different-tag scenario has no node-test coverage
because canvas-reconciler tests need a real DOM (browser-test build)
and the project hasn't wired up that test target yet. Manual repro
🤖 Generated with Claude Code