Skip to content

Fix stale component boundary bookkeeping after Link navigation - #129

Merged
nakasyou merged 2 commits into
mainfrom
devin/1790212743-fix-stale-component-boundary-nav
Sep 24, 2026
Merged

nakasyou merged 2 commits into
mainfrom
devin/1790212743-fix-stale-component-boundary-nav

Conversation

@nakasyou

Copy link
Copy Markdown
Member

Summary

Link navigation broke content rendered through component-valued props/children inside a shared layout, and also broke signal-driven updates after navigating to a route. Two related runtime bugs:

  1. Stale boundary anchors after DOM removal. On shared-layout navigation (updateSharedLayoutBoundary), replaceProjectionSlotContents drops the old slot DOM but component records kept start/end pointing into the detached subtree. On the next render those components took the wasActive reuse early-return in renderComponentToNodes and emitted only empty ec:c:start/ec:c:end marker pairs — the content (e.g. RenderPropProbe's aa/children slots) never re-rendered. Fixed by clearing component.start/end for boundaries inside removed ranges: clearRemovedComponentBoundaryAnchors runs in replaceProjectionSlotContents (which now takes container) and in replaceBoundaryContents behind a new options.container, plus a pre-render clear (clearComponentBoundaryAnchorsInRange) before each nav slot re-render, since the render happens while the old DOM is still attached.

  2. Stale dirty-flush activation clobbering freshly rendered routes. activateComponent captures activateSymbol, then await loadSymbol(...). If the component was re-rendered by route navigation during the await (same component id, different symbol), the flush resumed with the old route's module and replaceBoundaryContents overwrote the just-rendered boundary — destroying DOM nodes that compiled reactive effects still anchored to, so the /suspense ready signal write ran its effect but the update landed in detached nodes. Fixed by bailing when component.active || component.symbol !== activateSymbol after the module load.

Reproduction / regression test

e2e/test/example.dev.test.ts → keeps component-valued props and children rendered after Link navigation round trip (added in this branch): navigates Home → Counter → Home with Link and asserts the prop/children components are still rendered. It fails without the fix (empty ec:c: boundaries, 0 elements) and passes with it.

Verification

  • bun run test — all unit tests pass.
  • bun run test:e2e — 48/49 pass, including the new regression test and the suspense-after-nav tests. The only failure is shares atom state across components and preserves it across Link navigation, which fails identically on unpatched main (pre-existing).
  • vp check and bun typecheck clean.

Link to Devin session: https://app.devin.ai/sessions/d9018d13ba034be88c394f41f902b2e8
Open in Devin Desktop: https://app.devin.ai/desktop/session/d9018d13ba034be88c394f41f902b2e8?variant=devin
Requested by: @nakasyou

devin-ai-integration Bot and others added 2 commits September 24, 2026 01:19
When a shared layout survives a route change, the components map is kept
without a container reset. Components whose previous DOM boundary was
detached by the navigation slot swap still hit the 'wasActive' reuse
early return in renderComponentToNodes, emitting only ec:c: boundary
markers and relying on preserveReusableContentInRoots to recover the old
DOM. The old DOM is no longer reachable, so the boundary renders empty.

Gate the reuse paths on the previous boundary still being attached to a
parent (previousDomReusable = previousStart/End have a parentNode), so
detached components render fresh nodes instead of empty boundaries.

Adds a permanent E2E regression test covering component-valued props and
children surviving a Link navigation round trip through a shared layout.

Co-authored-by: codex <codex@openai.com>
Co-Authored-By: Shotaro Nakamura <syoutarou1103@outlook.jp>
When a shared layout survives a Link navigation, components whose DOM
was dropped by the projection slot swap kept their start/end anchors
pointing into the detached subtree. A later render then hit the
wasActive reuse early return and emitted empty ec:c: boundary pairs,
so component-valued props and children disappeared after a navigation
round trip.

Clear component start/end anchors when the DOM containing them is
removed (replaceProjectionSlotContents / replaceBoundaryContents) and
before a shared-layout nav renders the replacement slot contents, so
stale records render fresh instead of reusing dead DOM.

Also skip a queued dirty-flush activation when its captured symbol no
longer matches the component after awaiting module load: the flush
previously resumed with the old route's module and replaced the
freshly-rendered boundary contents, destroying DOM nodes that compiled
reactive effects still anchored to (e.g. a signal-driven update after
Link navigation never reached the page).

Co-authored-by: codex <codex@openai.com>
Co-Authored-By: Shotaro Nakamura <syoutarou1103@outlook.jp>
Copilot AI lite review requested due to automatic review settings September 24, 2026 02:04
@nakasyou nakasyou self-assigned this Sep 24, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions

Copy link
Copy Markdown
Contributor

Website preview: https://pr-129-eclipsa.xiarenda61.workers.dev

Commit: f2b1a99

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 7.50000% with 74 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
packages/eclipsa/core/runtime.ts 7.50% 74 Missing ⚠️
Files with missing lines Coverage Δ
packages/eclipsa/core/runtime.ts 21.49% <7.50%> (-0.16%) ⬇️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@nakasyou
nakasyou merged commit 21d9651 into main Sep 24, 2026
5 checks passed
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.

3 participants