[PR #748] [MERGED] refactor: extract route wiring from generated entry into typed runtime module #815

New issue

Closed

opened 2026-05-06 13:10:14 +02:00 by BreizhHardware · 0 comments

BreizhHardware commented

2026-05-06 13:10:14 +02:00

Owner

📋 Pull Request Information

Original PR: https://github.com/cloudflare/vinext/pull/748
Author: @NathanDrake2406
Created: 4/2/2026
Status: ✅ Merged
Merged: 4/3/2026
Merged by: @james-elicx

Base: main ← Head: feat/layout-persistence-pr-2b

📝 Commits (7)

7f4f8eb Extract app page route wiring helpers
be33773 Fix app page error boundary serialization
ca40d05 Fix client error boundary pathname reset
bddda39 Document Next.js error boundary verification
f7b35a0 Address bonk review: edge case tests and readability nit
175ae93 Address second-round review: type honesty and test cleanup
98174ca Add comment explaining template vs layout param asymmetry

📊 Changes

6 files changed (+910 additions, -1606 deletions)

View changed files

📝 packages/vinext/src/entries/app-rsc-entry.ts (+34 -233)
➕ packages/vinext/src/server/app-page-route-wiring.tsx (+337 -0)
📝 packages/vinext/src/shims/error-boundary.tsx (+31 -4)
📝 tests/__snapshots__/entry-templates.test.ts.snap (+180 -1334)
➕ tests/app-page-route-wiring.test.ts (+175 -0)
📝 tests/error-boundary.test.ts (+153 -35)

📄 Description

Summary

Part of #726 (PR 2b). Moves boundary/layout/segment wiring logic out of the generated entry template (app-rsc-entry.ts) into a typed runtime module (server/app-page-route-wiring.tsx). This follows the project's architectural guideline that generated entries should stay thin: codegen glue, not runtime logic.

What moved

The generated buildPageElement() function in app-rsc-entry.ts previously contained ~250 lines of inline logic for:

Resolving dynamic route segments to param values (catch-all, optional catch-all, dynamic)
Building the layout -> boundary -> template -> page nesting chain
Wiring LayoutSegmentProvider with computed segmentMap at each layout level
Attaching ErrorBoundary, NotFoundBoundary, and Suspense (loading) at correct tree positions
Mounting parallel slot content with per-slot layouts, loading, and error boundaries

All of this is now in packages/vinext/src/server/app-page-route-wiring.tsx as typed, unit-testable functions:

Export	Purpose
`buildAppPageRouteElement()`	Main orchestrator that takes route config + page element and returns the full wired React tree
`createAppPageLayoutEntries()`	Computes layout entry metadata (tree path, ID, associated error/notFound modules)
`resolveAppPageChildSegments()`	Resolves route segments against matched params (dynamic `[id]`, catch-all `[...slug]`, optional `[[...slug]]`)
`createAppPageTreePath()`	Builds tree path string from route segments (includes route groups for collision avoidance)

The generated entry now imports and calls __buildAppPageRouteElement() instead of inlining the logic. Template snapshots updated accordingly, and the generated code is much shorter.

ErrorBoundary pathname-based reset

ErrorBoundary now resets error state on pathname changes, matching the existing pattern in NotFoundBoundary. This was identified as a required fix in #726 because the boundary instance persists at the same tree position across client navigations.

Implementation details:

ErrorBoundary is now a function wrapper that reads usePathname() and passes it to ErrorBoundaryInner.
getDerivedStateFromProps() clears the caught error when the pathname changes.
getDerivedStateFromError() preserves the existing previousPathname instead of overwriting it, which avoids immediately clearing freshly caught client errors before error.tsx can render.

Important scope note: this reset is pathname-based only. It does not reset on search-param-only navigations because usePathname() does not include the query string.

Why this matters for layout persistence

PR 2c ("The Switch") needs to restructure how the route wiring is built from a single nested tree to flat map entries with Slot references. Having the wiring logic in a typed module instead of embedded in a template string means PR 2c can modify buildAppPageRouteElement() directly with type safety, IDE support, and focused unit tests.

Test plan

tests/app-page-route-wiring.test.ts — unit coverage for the extracted helpers (segment resolution, layout entries, tree paths)
tests/error-boundary.test.ts — pathname reset coverage, including a regression test that caught the immediate-clear bug in the client error boundary state machine
tests/entry-templates.test.ts — snapshot updated to verify generated entry delegates to the extracted helper
Targeted app-router E2E coverage for the browser error flows that regressed during refactor

vp test run tests/error-boundary.test.ts tests/app-page-route-wiring.test.ts tests/entry-templates.test.ts
vp check packages/vinext/src/entries/app-rsc-entry.ts packages/vinext/src/server/app-page-route-wiring.tsx packages/vinext/src/shims/error-boundary.tsx tests/error-boundary.test.ts
vp run test:e2e -- tests/e2e/app-router/error-handling.spec.ts tests/e2e/app-router/error-interactive.spec.ts tests/e2e/app-router/nextjs-compat/error-nav.spec.ts tests/e2e/app-router/navigation-flows.spec.ts --project app-router

Refs #726

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/cloudflare/vinext/pull/748 **Author:** [@NathanDrake2406](https://github.com/NathanDrake2406) **Created:** 4/2/2026 **Status:** ✅ Merged **Merged:** 4/3/2026 **Merged by:** [@james-elicx](https://github.com/james-elicx) **Base:** `main` ← **Head:** `feat/layout-persistence-pr-2b` --- ### 📝 Commits (7) - [`7f4f8eb`](https://github.com/cloudflare/vinext/commit/7f4f8ebb019ab08597e0eba0a978bf98f5e7745d) Extract app page route wiring helpers - [`be33773`](https://github.com/cloudflare/vinext/commit/be33773470b44b3767e6f3b2ed4aecceefa075b4) Fix app page error boundary serialization - [`ca40d05`](https://github.com/cloudflare/vinext/commit/ca40d05f70e9fad6b5e557d5233cbf020573a081) Fix client error boundary pathname reset - [`bddda39`](https://github.com/cloudflare/vinext/commit/bddda39ac3ee2504bd99df2b71248024b5d2efe8) Document Next.js error boundary verification - [`f7b35a0`](https://github.com/cloudflare/vinext/commit/f7b35a0897481901988b3999f8661e520fc9294a) Address bonk review: edge case tests and readability nit - [`175ae93`](https://github.com/cloudflare/vinext/commit/175ae936575e1e5ad3cf8a510ccedc3d4b43ee44) Address second-round review: type honesty and test cleanup - [`98174ca`](https://github.com/cloudflare/vinext/commit/98174cabee846780f94ab3e9e0df80f14f147bd5) Add comment explaining template vs layout param asymmetry ### 📊 Changes **6 files changed** (+910 additions, -1606 deletions) <details> <summary>View changed files</summary> 📝 `packages/vinext/src/entries/app-rsc-entry.ts` (+34 -233) ➕ `packages/vinext/src/server/app-page-route-wiring.tsx` (+337 -0) 📝 `packages/vinext/src/shims/error-boundary.tsx` (+31 -4) 📝 `tests/__snapshots__/entry-templates.test.ts.snap` (+180 -1334) ➕ `tests/app-page-route-wiring.test.ts` (+175 -0) 📝 `tests/error-boundary.test.ts` (+153 -35) </details> ### 📄 Description ## Summary Part of #726 (PR 2b). Moves boundary/layout/segment wiring logic out of the generated entry template (`app-rsc-entry.ts`) into a typed runtime module (`server/app-page-route-wiring.tsx`). This follows the project's architectural guideline that [generated entries should stay thin](https://github.com/cloudflare/vinext/blob/main/CLAUDE.md#generated-entry-modules-should-stay-thin): codegen glue, not runtime logic. ### What moved The generated `buildPageElement()` function in `app-rsc-entry.ts` previously contained ~250 lines of inline logic for: - Resolving dynamic route segments to param values (catch-all, optional catch-all, dynamic) - Building the layout -> boundary -> template -> page nesting chain - Wiring `LayoutSegmentProvider` with computed `segmentMap` at each layout level - Attaching `ErrorBoundary`, `NotFoundBoundary`, and `Suspense` (loading) at correct tree positions - Mounting parallel slot content with per-slot layouts, loading, and error boundaries All of this is now in `packages/vinext/src/server/app-page-route-wiring.tsx` as typed, unit-testable functions: | Export | Purpose | |--------|---------| | `buildAppPageRouteElement()` | Main orchestrator that takes route config + page element and returns the full wired React tree | | `createAppPageLayoutEntries()` | Computes layout entry metadata (tree path, ID, associated error/notFound modules) | | `resolveAppPageChildSegments()` | Resolves route segments against matched params (dynamic `[id]`, catch-all `[...slug]`, optional `[[...slug]]`) | | `createAppPageTreePath()` | Builds tree path string from route segments (includes route groups for collision avoidance) | The generated entry now imports and calls `__buildAppPageRouteElement()` instead of inlining the logic. Template snapshots updated accordingly, and the generated code is much shorter. ### ErrorBoundary pathname-based reset `ErrorBoundary` now resets error state on pathname changes, matching the existing pattern in `NotFoundBoundary`. This was identified as a required fix in #726 because the boundary instance persists at the same tree position across client navigations. Implementation details: - `ErrorBoundary` is now a function wrapper that reads `usePathname()` and passes it to `ErrorBoundaryInner`. - `getDerivedStateFromProps()` clears the caught error when the pathname changes. - `getDerivedStateFromError()` preserves the existing `previousPathname` instead of overwriting it, which avoids immediately clearing freshly caught client errors before `error.tsx` can render. Important scope note: this reset is pathname-based only. It does **not** reset on search-param-only navigations because `usePathname()` does not include the query string. ### Why this matters for layout persistence PR 2c ("The Switch") needs to restructure how the route wiring is built from a single nested tree to flat map entries with `Slot` references. Having the wiring logic in a typed module instead of embedded in a template string means PR 2c can modify `buildAppPageRouteElement()` directly with type safety, IDE support, and focused unit tests. ## Test plan - [x] `tests/app-page-route-wiring.test.ts` — unit coverage for the extracted helpers (segment resolution, layout entries, tree paths) - [x] `tests/error-boundary.test.ts` — pathname reset coverage, including a regression test that caught the immediate-clear bug in the client error boundary state machine - [x] `tests/entry-templates.test.ts` — snapshot updated to verify generated entry delegates to the extracted helper - [x] Targeted app-router E2E coverage for the browser error flows that regressed during refactor ```bash vp test run tests/error-boundary.test.ts tests/app-page-route-wiring.test.ts tests/entry-templates.test.ts vp check packages/vinext/src/entries/app-rsc-entry.ts packages/vinext/src/server/app-page-route-wiring.tsx packages/vinext/src/shims/error-boundary.tsx tests/error-boundary.test.ts vp run test:e2e -- tests/e2e/app-router/error-handling.spec.ts tests/e2e/app-router/error-interactive.spec.ts tests/e2e/app-router/nextjs-compat/error-nav.spec.ts tests/e2e/app-router/navigation-flows.spec.ts --project app-router ``` Refs #726 --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>