[PR #1016] [MERGED] refactor(app-router): extract app fallback renderer factory #1020

New issue

Closed

opened 2026-05-06 13:11:35 +02:00 by BreizhHardware · 0 comments

BreizhHardware commented

2026-05-06 13:11:35 +02:00

Owner

📋 Pull Request Information

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

Base: main ← Head: nathan/extract-fallback-renderer

📝 Commits (1)

99bc7b6 refactor(app-router): extract app fallback renderer factory to eliminate per-request closure reconstruction

📊 Changes

6 files changed (+763 additions, -652 deletions)

View changed files

📝 packages/vinext/src/entries/app-rsc-entry.ts (+31 -93)
➕ packages/vinext/src/server/app-fallback-renderer.ts (+212 -0)
📝 packages/vinext/src/server/app-page-boundary-render.ts (+1 -1)
📝 tests/__snapshots__/entry-templates.test.ts.snap (+174 -546)
➕ tests/app-fallback-renderer.test.ts (+321 -0)
📝 tests/app-router.test.ts (+24 -12)

📄 Description

[Kimi 2.6]

What this changes

Replaces three generated wrapper functions (renderHTTPAccessFallbackPage, renderNotFoundPage, renderErrorBoundaryPage) in the App Router RSC entry with a single factory createAppFallbackRenderer() that returns { renderHttpAccessFallback, renderNotFound, renderErrorBoundary }.

The generated entry constructs the factory once at module level and calls the returned methods many times. App-level dependencies (root boundaries, font providers, metadata routes, SSR loader, thenable-params helper, sanitizer, RSC renderer) are pre-bound at construction time. The per-request Request object is passed at the call site and is no longer captured in any factory closure.

Why

Previously each of the three wrappers reconstructed a large options object on every invocation, passing the same app-level constants over and over. The request parameter was also captured in each wrapper's closure, which inflated the dependency surface and made it hard to distinguish per-app from per-request state at a glance.

This follows the codebase principle: codegen should describe the app shape; normal modules should implement behavior.

Approach

New typed module: packages/vinext/src/server/app-fallback-renderer.ts
- Exports createAppFallbackRenderer(options) with a strongly-typed options bag
- Returns three render methods that delegate to the existing renderAppPageHttpAccessFallback and renderAppPageErrorBoundary helpers in app-page-boundary-render.ts
- The empty-middleware fallback ({ headers: null, status: null }) moves into the factory module so generated code no longer needs to define it
packages/vinext/src/entries/app-rsc-entry.ts
- Removes ~90 lines of generated wrapper code
- Adds a single const __fallbackRenderer = __createAppFallbackRenderer({...}) construction
- Call sites in __dispatchAppPage and the not-found fallback path now call __fallbackRenderer.renderErrorBoundary(...), __fallbackRenderer.renderHttpAccessFallback(...), and __fallbackRenderer.renderNotFound(...)
packages/vinext/src/server/app-page-boundary-render.ts
- Exports AppPageBoundaryRoute type so the factory can reference it without redeclaring

Validation

Added tests/app-fallback-renderer.test.ts with focused unit tests for:
- Factory returns the three expected methods
- renderNotFound delegates to renderHttpAccessFallback with status 404
- Request is passed at call site (verified by calling with two different requests)
- createRscOnErrorHandler receives the request at call time
- Empty middleware context fallback works when none is provided
Updated tests/app-router.test.ts generated-code assertions to verify:
- createAppFallbackRenderer as __createAppFallbackRenderer is imported
- const __fallbackRenderer = __createAppFallbackRenderer({...}) appears in generated code
- The old inline return __renderAppPageHttpAccessFallback({...}) and return __renderAppPageErrorBoundary({...}) are gone
- Delegation closures inside __dispatchAppPage call __fallbackRenderer.renderErrorBoundary and __fallbackRenderer.renderHttpAccessFallback
All existing tests pass:
- tests/app-router.test.ts (316 tests)
- tests/app-page-boundary-render.test.ts (12 tests)
- tests/app-page-dispatch.test.ts (4 tests)
- tests/app-fallback-renderer.test.ts (6 tests)

Relevant Next.js source

This refactoring aligns vinext's App Router fallback/error-boundary rendering with the same architectural separation Next.js uses between its app-render pipeline and the underlying boundary components:

Next.js app-render error/HTTP-fallback rendering logic: packages/next/src/server/app-render/app-render.tsx (see findPrerenderHTTPErrorBoundaryTree, getErrorRSCPayload, and the errorRSCPayload / errorServerStream paths around L8400-L8480)
Next.js HTTP access fallback boundary component: packages/next/src/client/components/http-access-fallback/error-boundary.tsx
Next.js metadata boundary handling for not-found: packages/next/src/server/app-render/app-render.tsx (see createMetadataComponents with errorType: 'not-found' around L2110-L2125)

Risks / follow-ups

None. This is a pure refactor: no public API changes, no behavioral changes. The factory delegates to the exact same renderAppPageHttpAccessFallback and renderAppPageErrorBoundary helpers that were already in use.

_{🔄 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/1016 **Author:** [@NathanDrake2406](https://github.com/NathanDrake2406) **Created:** 5/2/2026 **Status:** ✅ Merged **Merged:** 5/2/2026 **Merged by:** [@james-elicx](https://github.com/james-elicx) **Base:** `main` ← **Head:** `nathan/extract-fallback-renderer` --- ### 📝 Commits (1) - [`99bc7b6`](https://github.com/cloudflare/vinext/commit/99bc7b61f2d9573b437bfff20bef44c93f2bb41d) refactor(app-router): extract app fallback renderer factory to eliminate per-request closure reconstruction ### 📊 Changes **6 files changed** (+763 additions, -652 deletions) <details> <summary>View changed files</summary> 📝 `packages/vinext/src/entries/app-rsc-entry.ts` (+31 -93) ➕ `packages/vinext/src/server/app-fallback-renderer.ts` (+212 -0) 📝 `packages/vinext/src/server/app-page-boundary-render.ts` (+1 -1) 📝 `tests/__snapshots__/entry-templates.test.ts.snap` (+174 -546) ➕ `tests/app-fallback-renderer.test.ts` (+321 -0) 📝 `tests/app-router.test.ts` (+24 -12) </details> ### 📄 Description [Kimi 2.6] ## What this changes Replaces three generated wrapper functions (`renderHTTPAccessFallbackPage`, `renderNotFoundPage`, `renderErrorBoundaryPage`) in the App Router RSC entry with a single factory `createAppFallbackRenderer()` that returns `{ renderHttpAccessFallback, renderNotFound, renderErrorBoundary }`. The generated entry constructs the factory **once** at module level and calls the returned methods many times. App-level dependencies (root boundaries, font providers, metadata routes, SSR loader, thenable-params helper, sanitizer, RSC renderer) are pre-bound at construction time. The per-request `Request` object is passed at the call site and is no longer captured in any factory closure. ## Why Previously each of the three wrappers reconstructed a large options object on every invocation, passing the same app-level constants over and over. The `request` parameter was also captured in each wrapper's closure, which inflated the dependency surface and made it hard to distinguish per-app from per-request state at a glance. This follows the codebase principle: **codegen should describe the app shape; normal modules should implement behavior.** ## Approach - New typed module: `packages/vinext/src/server/app-fallback-renderer.ts` - Exports `createAppFallbackRenderer(options)` with a strongly-typed options bag - Returns three render methods that delegate to the existing `renderAppPageHttpAccessFallback` and `renderAppPageErrorBoundary` helpers in `app-page-boundary-render.ts` - The empty-middleware fallback (`{ headers: null, status: null }`) moves into the factory module so generated code no longer needs to define it - `packages/vinext/src/entries/app-rsc-entry.ts` - Removes ~90 lines of generated wrapper code - Adds a single `const __fallbackRenderer = __createAppFallbackRenderer({...})` construction - Call sites in `__dispatchAppPage` and the not-found fallback path now call `__fallbackRenderer.renderErrorBoundary(...)`, `__fallbackRenderer.renderHttpAccessFallback(...)`, and `__fallbackRenderer.renderNotFound(...)` - `packages/vinext/src/server/app-page-boundary-render.ts` - Exports `AppPageBoundaryRoute` type so the factory can reference it without redeclaring ## Validation - Added `tests/app-fallback-renderer.test.ts` with focused unit tests for: - Factory returns the three expected methods - `renderNotFound` delegates to `renderHttpAccessFallback` with status 404 - Request is passed at call site (verified by calling with two different requests) - `createRscOnErrorHandler` receives the request at call time - Empty middleware context fallback works when none is provided - Updated `tests/app-router.test.ts` generated-code assertions to verify: - `createAppFallbackRenderer as __createAppFallbackRenderer` is imported - `const __fallbackRenderer = __createAppFallbackRenderer({...})` appears in generated code - The old inline `return __renderAppPageHttpAccessFallback({...})` and `return __renderAppPageErrorBoundary({...})` are gone - Delegation closures inside `__dispatchAppPage` call `__fallbackRenderer.renderErrorBoundary` and `__fallbackRenderer.renderHttpAccessFallback` - All existing tests pass: - `tests/app-router.test.ts` (316 tests) - `tests/app-page-boundary-render.test.ts` (12 tests) - `tests/app-page-dispatch.test.ts` (4 tests) - `tests/app-fallback-renderer.test.ts` (6 tests) ## Relevant Next.js source This refactoring aligns vinext's App Router fallback/error-boundary rendering with the same architectural separation Next.js uses between its app-render pipeline and the underlying boundary components: - Next.js app-render error/HTTP-fallback rendering logic: [`packages/next/src/server/app-render/app-render.tsx`](https://github.com/vercel/next.js/blob/canary/packages/next/src/server/app-render/app-render.tsx) (see `findPrerenderHTTPErrorBoundaryTree`, `getErrorRSCPayload`, and the `errorRSCPayload` / `errorServerStream` paths around L8400-L8480) - Next.js HTTP access fallback boundary component: [`packages/next/src/client/components/http-access-fallback/error-boundary.tsx`](https://github.com/vercel/next.js/blob/canary/packages/next/src/client/components/http-access-fallback/error-boundary.tsx) - Next.js metadata boundary handling for not-found: [`packages/next/src/server/app-render/app-render.tsx`](https://github.com/vercel/next.js/blob/canary/packages/next/src/server/app-render/app-render.tsx) (see `createMetadataComponents` with `errorType: 'not-found'` around L2110-L2125) ## Risks / follow-ups - None. This is a pure refactor: no public API changes, no behavioral changes. The factory delegates to the exact same `renderAppPageHttpAccessFallback` and `renderAppPageErrorBoundary` helpers that were already in use. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>