fix: a preserved permanent element keeps its descendant scripts - #1279
Draft
vivek7405 wants to merge 1 commit into
Draft
fix: a preserved permanent element keeps its descendant scripts#1279vivek7405 wants to merge 1 commit into
vivek7405 wants to merge 1 commit into
Conversation
`data-webjs-permanent` preserves an element across a swap as the same live DOM node, and the reconciler already treats that as a SUBTREE guarantee (`diffElementInPlace` returns early rather than recursing into one). The reactivation pass disagreed: it re-emitted every script inside the preserved subtree, and re-emitting a script is what makes it execute. So a widget whose init script sits inside the permanent element got that script re-run on every soft navigation, against an instance the author explicitly asked the router to keep alive. The exemption keys on whether the node was ACTUALLY preserved by identity, not on the attribute. The regrafts have a both-exist guard, so a permanent element arriving for the first time is a freshly imported node whose scripts have never run; an attribute-only filter would leave those never running on any path. Both regrafts now add the live node to a module-scope WeakSet as they move it, and `reactivateScripts` skips a script under any preserved root. The container-is-a-script case stays unexempt, deliberately. That one has no live node to preserve on the swap that first mounts a route, so exempting it is #1102 exactly, and a test pins the asymmetry.
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.
Closes #1252
data-webjs-permanentpreserves an element across a swap as the same live DOMnode, and the reconciler already reads that as a SUBTREE guarantee
(
diffElementInPlacereturns early rather than recursing into one).reactivateScriptsdisagreed: it re-emitted every script inside the preservedsubtree, and re-emitting a script is what makes it execute. A widget whose init
script sits inside the permanent element got that script re-run on every soft
navigation, against an instance the author explicitly asked the router to keep
alive. Pre-existing, not introduced by #1236.
What changed
WeakSet(regraftedPermanents) that both regrafts add thelive node to at the moment they move it, including the detached top-level
branch of
regraftPermanentInSlicethat assigns intoincomingSlice[idx].reactivateScriptscollects preserved roots from the container DOWNWARD(
contains, neverclosest(), so the test cannot escape the swapped range)and skips scripts under them. Empty list means the existing loop runs
unchanged, so the common path costs one
querySelectorAll.The regrafts have a both-exist guard, so a permanent element arriving for the
first time is a freshly imported node whose scripts have never run; an
attribute-only filter would leave those never running on any path.
on the swap that first mounts a route, so exempting it is fix: a top-level script in a swapped range is never reactivated #1102 exactly. A
test pins the asymmetry.
Verification
toplevel-script-reactivate.test.js(morph tier and replace tier) asserting the inner script does not re-run while
an ordinary sibling does, so an empty array cannot pass by accident. Plus a
first-mount case where the inner marker MUST appear.
Firefox and WebKit, with the four must-stay-green cases still passing.
router-client.test.js203/203. The case that pinned the old behaviouris inverted and now drives a real regraft;
data-webjs-permanent does NOT exempt a script (#1102)stays green unchanged.router-client.jsruns only in the browser and Bun neverexecutes the swap path; the require-bun-parity hook's pattern agrees.
Docs
website/app/docs/client-router/page.ts(the re-runnable-scripts bullet, theparagraph that said the attribute does not help, and the permanent-element
section),
.agents/skills/webjs/references/client-router-and-streaming.md, andthe
router-client.jsrow inpackages/core/AGENTS.md, which flatly said theattribute is not an exemption and must not be made one.
Merge order: this lands FIRST, before #1118. Both touch
router-client.js; thisis a correctness bug on a shipped feature sitting directly on top of #1236, and
the two regions are disjoint.