Skip to content

fix: a preserved permanent element keeps its descendant scripts - #1279

Draft
vivek7405 wants to merge 1 commit into
mainfrom
fix/permanent-descendant-scripts
Draft

fix: a preserved permanent element keeps its descendant scripts#1279
vivek7405 wants to merge 1 commit into
mainfrom
fix/permanent-descendant-scripts

Conversation

@vivek7405

Copy link
Copy Markdown
Collaborator

Closes #1252

data-webjs-permanent preserves an element across a swap as the same live DOM
node, and the reconciler already reads that as a SUBTREE guarantee
(diffElementInPlace returns early rather than recursing into one).
reactivateScripts disagreed: it re-emitted every script inside the preserved
subtree, 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

  • A module-scope WeakSet (regraftedPermanents) that both regrafts add the
    live node to at the moment they move it, including the detached top-level
    branch of regraftPermanentInSlice that assigns into incomingSlice[idx].
  • reactivateScripts collects preserved roots from the container DOWNWARD
    (contains, never closest(), 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 exemption keys on ACTUAL preservation by identity, never 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.
  • The container-is-a-script case stays unexempt. It has no live node to preserve
    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

  • Browser (headline): two new cases in 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.
  • Counterfactual: reverting the skip line alone reds both new cases on Chromium,
    Firefox and WebKit, with the four must-stay-green cases still passing.
  • Unit: router-client.test.js 203/203. The case that pinned the old behaviour
    is inverted and now drives a real regraft; data-webjs-permanent does NOT exempt a script (#1102) stays green unchanged.
  • Full router browser suite: 138/138 on Chromium, Firefox and WebKit.
  • Bun parity: N/A. router-client.js runs only in the browser and Bun never
    executes the swap path; the require-bun-parity hook's pattern agrees.

Docs

website/app/docs/client-router/page.ts (the re-runnable-scripts bullet, the
paragraph that said the attribute does not help, and the permanent-element
section), .agents/skills/webjs/references/client-router-and-streaming.md, and
the router-client.js row in packages/core/AGENTS.md, which flatly said the
attribute is not an exemption and must not be made one.

Merge order: this lands FIRST, before #1118. Both touch router-client.js; this
is a correctness bug on a shipped feature sitting directly on top of #1236, and
the two regions are disjoint.

`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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix: a permanent element's descendant scripts re-execute on every swap

1 participant