Skip to content

perf(router): lazy-load App Router page and route-handler modules#1781

Merged
james-elicx merged 5 commits into
mainfrom
feat/lazy-route-modules
Jun 6, 2026
Merged

perf(router): lazy-load App Router page and route-handler modules#1781
james-elicx merged 5 commits into
mainfrom
feat/lazy-route-modules

Conversation

@james-elicx
Copy link
Copy Markdown
Member

Summary

The App Router RSC entry statically imported every route module, so they were all evaluated at Worker startup. For typical pages this is cheap, but routes with expensive module-level initialization make startup scale linearly and can exceed Cloudflare's ~400ms startup CPU budget.

This makes lazy route-module loading the default: static-route page modules and all route-handler modules are emitted as () => import() thunks instead of import * as, so they're code-split out of the entry's top-level evaluation and loaded on demand for the matched route only.

Measured impact (real wrangler deploy → "Worker Startup Time", vinext account)

Typical pages — startup is flat regardless of route count, ~0.013 ms/route (noise):

Routes RSC index.js Startup
5 888 KB 16–20 ms
50 996 KB 17–18 ms
150 1238 KB 20–21 ms
300 1604 KB 18–26 ms

Heavy module-level init (each page builds a large lookup table at import) — this is the failure mode lazy loading fixes:

Variant (150 routes) Startup
eager (before) 414–467 ms
lazy (this PR) 16–19 ms

Typical apps pay nothing; heavy apps stop blowing the startup budget.

How it works

  • Codegen (app-rsc-manifest.ts): new getLazyLoaderVar emits const load_N = () => import(path). Static-route pages and all route handlers use it; route table fields become page/__loadPage and routeHandler/__loadRouteHandler.
  • Helper (app-route-module-loader.ts): ensureAppRouteModulesLoaded(route) hydrates the lazy modules onto the synchronous page/routeHandler fields. Idempotent, and dedups concurrent loads into a single import.
  • Hydration sites: invoked at the central match point (app-rsc-handler.ts) and at every mid-flight route lookup that reads a module before buildPageElement — server-action redirect/rerender targets (app-server-action-execution.ts), interception + ISR revalidation source routes (app-page-dispatch.ts), buildPageElements, and handleServerActionRequest.

Scope / follow-up

  • Dynamic-route pages stay eager because their generateStaticParams is referenced in the module-level generateStaticParamsMap. Making those lazy needs a prerender static-params resolver rework — tracked as a follow-up.
  • Pages Router server entry is untouched (separate change if wanted).

Testing

  • End-to-end on real Cloudflare: static (lazy), dynamic [slug] (eager), and /api/hello route handler (lazy) all render 200; heavy-150 startup 19 ms.
  • Green suites: features (311), prod/dev server + interception + prerender + production build (316), route-handler + ISR + worker-entry + codegen (142), app-page consumers (162), action/dispatch units (81), entry-templates (25, updated), new app-route-module-loader unit tests (7). vp check clean.

🤖 Generated with opencode

Statically-imported route modules were all evaluated at Worker startup.
For typical pages this is cheap, but routes with expensive module-level
initialization make startup scale linearly and can exceed Cloudflare's
~400ms startup CPU budget. Measured on real Workers: 150 pages with heavy
module-init went from ~414-467ms startup (eager) to ~17ms (lazy); typical
pages are unaffected (~0.01ms/route either way).

The RSC manifest now emits page modules of static routes and all
route-handler modules as `() => import()` thunks instead of eager
`import * as`, so they are code-split out of the entry's top-level
evaluation and loaded on demand for the matched route only.

ensureAppRouteModulesLoaded() hydrates a route's lazy modules onto the
synchronous page/routeHandler fields before any consumer reads them; it is
idempotent and dedups concurrent loads. Hydration is invoked at the central
match point and at every mid-flight route lookup that reads modules before
buildPageElement (server-action redirect/rerender targets, interception and
ISR revalidation source routes).

Dynamic-route pages stay eager because their generateStaticParams is
referenced in the module-level generateStaticParamsMap; making those lazy
needs a prerender-resolver rework and is tracked as follow-up.
@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented Jun 6, 2026

Open in StackBlitz

npm i https://pkg.pr.new/@vinext/cloudflare@1781
npm i https://pkg.pr.new/vinext@1781

commit: 7516530

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 6, 2026

Example Preview Production Original
app-router-cloudflare preview production
pages-router-cloudflare preview production
app-router-playground preview production original
realworld-api-rest preview production
nextra-docs-template preview production
benchmarks preview production
hackernews preview production original
workers-cache preview production
web preview production


const varName = `load_${lazyIdx++}`;
const absPath = normalizePathSeparators(filePath);
imports.push(`const ${varName} = () => import(${JSON.stringify(absPath)});`);
@james-elicx james-elicx changed the title feat(router): lazy-load App Router page and route-handler modules perf(router): lazy-load App Router page and route-handler modules Jun 6, 2026
@james-elicx
Copy link
Copy Markdown
Member Author

/bigbonk review for issues

Copy link
Copy Markdown
Contributor

@ask-bonk ask-bonk Bot left a comment

Choose a reason for hiding this comment

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

/bigbonk review — lazy-load App Router page/route-handler modules

I traced every synchronous read of route.page / route.routeHandler against the new hydration sites. The wiring is solid: the central match point (app-rsc-handler.ts:678), buildPageElements (app-rsc-entry.ts:493), the action redirect/rerender targets, intercept + ISR revalidation source routes, and handleServerActionRequest are all hydrated before any read. Intercept page fields and dynamic-route pages stay eager, so the lazy paths are consistent. No correctness blocker found.

A few things worth considering:

1. Failed lazy import is cached permanently (no retry / no reset)

ensureAppRouteModulesLoaded (app-route-module-loader.ts:60-72) only clears __loading and sets __loaded inside the .then success continuation. If the dynamic import() rejects:

  • __loading keeps holding the rejected promise,
  • __loaded stays false,
  • every subsequent call returns that same rejected promise (line 51).

So a single transient import failure for a route wedges that route into a permanent 500 for the life of the Worker isolate, and the stored rejected promise can surface as an unhandled rejection if a later caller doesn't attach a handler in time.

This is a behavior change from the eager model: with import * as a module-eval failure happens once at startup and is effectively deterministic, whereas now it's per-request and sticky. Import failures are rare and usually deterministic, so severity is low — but a .catch that resets __loading = null (leaving __loaded false so the next request retries) would match the "load on demand" intent better and avoid the sticky-rejection footgun. At minimum, add a test for the rejection path.

2. CodeQL alert on app-rsc-manifest.ts:124 is a false positive — worth a suppression/comment

The new getLazyLoaderVar emits const load_N = () => import(${JSON.stringify(absPath)}). CodeQL flags it as "improper code sanitization" because the path flows into generated code. The input is a filesystem scan result (route.pagePath / route.routePath), the same trust level as the pre-existing eager import * as ${var} from ${JSON.stringify(absPath)} two lines up (line 114), which CodeQL does not flag. It's a false positive driven by import() being treated as dynamic. Recommend dismissing the alert with a note (or a short codegen comment) so it doesn't recur — the surrounding getImportVar is identical in trust model.

3. Minor: double hydration on the intercept path

app-page-dispatch.ts:719 awaits ensureRouteLoaded(interceptRoute) and then calls buildPageElement, which (app-rsc-entry.ts:493) awaits __ensureRouteLoaded(route) again. Idempotent so harmless, but the inner await makes the dispatch-site hydration redundant for that callback. Not a problem, just noting it's belt-and-suspenders.

Verified

  • vp test run tests/app-route-module-loader.test.ts tests/entry-templates.test.ts — 32 passing.
  • renderNotFound (which reads route.page at app-rsc-entry.ts:902) is only invoked with route: null (app-rsc-handler.ts:660), so the un-hydrated path is unreachable there.
  • Prerender endpoint reads only staticParamsMap (eager dynamic-route modules); static-route pages have no generateStaticParams, so leaving them lazy is correct.
  • Pages Router prod server delegates App Router to the built RSC entry, inheriting hydration — no separate gap.

Overall this is a well-scoped, correct change. Item 1 is the only one I'd want addressed (or consciously waived) before merge.

@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk Bot commented Jun 6, 2026

Review posted to PR #1781.

Summary

I traced every synchronous read of route.page/route.routeHandler against the new lazy-hydration sites. The wiring is correct — all match points, dispatch callbacks, action redirect/rerender targets, intercept paths, and route-handler dispatch hydrate before reading. No correctness blocker.

Findings posted:

  1. Failed lazy import is cached permanently (low severity, the only thing worth fixing before merge)ensureAppRouteModulesLoaded only resets __loading/sets __loaded in the success continuation. A rejected import() leaves the rejected promise stored and re-returned forever, wedging the route into a permanent 500 for the isolate's life and risking an unhandled rejection. This is a behavior shift from the eager model. Suggested a .catch that resets __loading = null so the next request retries, plus a rejection-path test.

  2. CodeQL alert at app-rsc-manifest.ts:124 is a false positive — the import() path comes from a filesystem scan (same trust as the unflagged eager import * as two lines above). Recommend dismissing with a note.

  3. Minor: double hydration on the intercept path (idempotent, harmless).

Also verified: renderNotFound's route.page read is only reached with route: null; the prerender endpoint only touches the eager staticParamsMap; and the Pages Router prod server inherits hydration via the built RSC entry. Ran the loader + entry-template tests (32 passing).

github run

…g the rejection

Addresses /bigbonk review feedback on the lazy route-module loader:

- ensureAppRouteModulesLoaded no longer caches a rejected dynamic import().
  On failure it clears __loading (leaving __loaded false) and re-throws, so
  the current request still sees the error but the next request retries —
  matching the eager model's per-isolate retry semantics instead of wedging
  the route into a permanent 500 and risking an unhandled rejection.
- Document why the getLazyLoaderVar import() codegen is a CodeQL false
  positive (trusted filesystem-scan path, same trust model as the eager
  getImportVar import * as).
- Add a rejection-path unit test covering retry-after-failure.
@james-elicx
Copy link
Copy Markdown
Member Author

Thanks @ask-bonk — addressed in 61a7eb2.

1. Sticky failed import (fixed). ensureAppRouteModulesLoaded now chains a .catch that clears __loading (leaving __loaded false) and re-throws. The current request still observes the error, but the route is no longer wedged — the next request retries, matching the eager model's per-isolate retry semantics and avoiding the stored-rejection unhandled-rejection footgun. Added a rejection-path unit test (does not cache a failed import: re-throws and retries on the next call).

2. CodeQL false positive (documented). Added a comment at getLazyLoaderVar explaining the path is a trusted filesystem-scan result (route.pagePath/route.routePath), identical trust model to the eager import * as in getImportVar two lines up. Will dismiss the alert as a false positive in the UI.

3. Double hydration on the intercept path (intentional, keeping). The dispatch-site await ensureRouteLoaded(interceptRoute) in app-page-dispatch.ts is required, not redundant: resolveRouteFetchCacheMode(interceptRoute) reads interceptRoute.page before buildPageElement runs, so the route must be hydrated at that point. The inner buildPageElements await is then a cheap idempotent no-op. Leaving both for correctness.

Loader + entry-template + action/dispatch suites green (103 passing).

@james-elicx
Copy link
Copy Markdown
Member Author

/bigbonk review for issues

@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk Bot commented Jun 6, 2026

APIError: Function tools with reasoning_effort are not supported for gpt-5.5 in /v1/chat/completions. Please use /v1/responses instead.

github run

@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk Bot commented Jun 6, 2026

@james-elicx Bonk workflow failed. Check the logs for details.

View workflow run · To retry, trigger Bonk again.

@james-elicx
Copy link
Copy Markdown
Member Author

/bigbonk re-review for issues

Copy link
Copy Markdown
Contributor

@ask-bonk ask-bonk Bot left a comment

Choose a reason for hiding this comment

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

/bigbonk review — lazy-load App Router page/route-handler modules

Re-reviewed at 61a7eb2 (the failed-import fix). I re-traced every synchronous read of route.page / route.routeHandler against the hydration sites and ran the loader, entry-template, features (311), and production-server + worker-entry (42) suites locally — all green. The wiring is correct and the prior blocker is properly resolved.

Verified correct

  • Sticky-failure fix is sound. ensureAppRouteModulesLoaded's .catch clears __loading (leaving __loaded false) and re-throws, so the current request observes the error and the next request retries — matching the eager per-isolate model. Concurrent callers share the one rejected promise and all await it, so there's no unhandled-rejection footgun. The new rejection-path test (does not cache a failed import…) covers it.
  • Every hydration site awaits. All 8 call sites (app-rsc-handler central match, buildPageElements, intercept + ISR-revalidation getSourceRoute targets in app-page-dispatch, action redirect/current/rerender targets in app-server-action-execution, and handleServerActionRequest) await before any .page/.routeHandler read.
  • handleServerActionRequest ordering is correct: await __ensureRouteLoaded(__actionMatch.route) (entry:795) precedes the __resolveAppPageSegmentConfig({ page: route.page }) runtime read (entry:797).
  • Intercept pages stay eager (getImportVar, manifest:234), so intercept page reads need no hydration; the intercept source route is hydrated before its layouts/page render via the buildPageElements wrapper (entry:493), and __buildPageElements is never called bypassing that wrapper.
  • renderNotFound's route.page read (entry:902) is only reached with route: null (handler:664) and is guarded.
  • Background ISR regeneration re-enters renderFreshPageForCache, which hydrates revalidationTarget.route (dispatch:568) before reading — covered.
  • Prerender endpoint touches only the eager staticParamsMap; App Router prod-server delegates to the built RSC entry so it inherits hydration.
  • CodeQL alert at app-rsc-manifest.ts getLazyLoaderVar is a false positive (JSON-encoded filesystem-scan path, same trust model as the eager import * as in getImportVar); now documented inline. Fine to dismiss.

Minor (non-blocking)

  1. Inaccurate comment, not a bug. registerRouteModules (app-rsc-manifest.ts:157) says route handlers "have no generateStaticParams." Next.js route handlers do support generateStaticParams for dynamic segments (app/api/[slug]/route.ts). The behavior here is still safe because buildGenerateStaticParamsEntries only ever sources from route.layouts and route.pagePath — never route.routePath — so dynamic route-handler static params were already absent from generateStaticParamsMap before this PR. Making route handlers lazy introduces no regression, but the comment overstates the reason. Suggest rewording to: "route handlers are never referenced by generateStaticParamsMap (it sources only layouts + page), so they can always be lazy" — the accurate justification, and a reminder that prerendering dynamic route handlers remains a separate gap.

  2. Double hydration on the intercept path (app-page-dispatch.ts:719 + the inner buildPageElements await) is intentional and idempotent, as the author noted — resolveRouteFetchCacheMode(interceptRoute) reads .page before buildPageElement runs, so the dispatch-site await is required. No change needed.

No correctness blocker. LGTM once item 1's comment is reworded (optional).

@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk Bot commented Jun 6, 2026

Review posted to PR #1781.

Summary

I re-reviewed the lazy-route-module change at 61a7eb2, traced every synchronous read of route.page / route.routeHandler against the hydration sites, and ran the loader, entry-template, features (311), and production-server + worker-entry (42) suites locally — all green.

Conclusion: no correctness blocker. The implementation is sound and the prior review's only blocker (sticky failed import) is properly resolved.

Findings posted:

  1. Sticky-failure fix verified — the .catch clears __loading/re-throws, concurrent callers all await the shared rejected promise (no unhandled-rejection), and the new rejection-path test covers it.

  2. All 8 hydration sites await before any module read — central match, buildPageElements, intercept/ISR getSourceRoute targets, action redirect/rerender targets, and handleServerActionRequest. Also confirmed renderNotFound is only reached with route: null, background ISR regen hydrates its target, and __buildPageElements is never called bypassing the hydrating wrapper.

  3. Minor (non-blocking): inaccurate comment. app-rsc-manifest.ts:157 claims route handlers "have no generateStaticParams" — they actually can in Next.js. It's safe because buildGenerateStaticParamsEntries never sources from route.routePath (pre-existing), so no regression — but suggested rewording the justification.

  4. CodeQL false positive (now documented) is fine to dismiss; double-hydration on the intercept path is intentional and idempotent.

github run

Reword the getLazyLoaderVar comment per /bigbonk re-review: route handlers
are lazy because they are never sourced into generateStaticParamsMap (which
only reads layouts + page), not because they 'have no generateStaticParams'
(Next.js route handlers can export it for prerendering — a separate gap).
@james-elicx
Copy link
Copy Markdown
Member Author

Thanks @ask-bonk — addressed the non-blocking item in 8220e7f.

Item 1 (comment reworded). Updated the getLazyLoaderVar justification to the accurate reason: route handlers are lazy because they're never sourced into generateStaticParamsMap (which reads only layouts + page, never route.routePath), not because they "have no generateStaticParams." Also noted that wiring route-handler generateStaticParams into prerendering remains a separate, pre-existing gap unaffected by this PR.

Item 2 (double hydration) is intentional/idempotent as discussed — no change. CodeQL alert will be dismissed as a false positive in the UI.

@james-elicx
Copy link
Copy Markdown
Member Author

/bigbonk re-review for issues

@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk Bot commented Jun 6, 2026

APIError: Function tools with reasoning_effort are not supported for gpt-5.5 in /v1/chat/completions. Please use /v1/responses instead.

github run

Copy link
Copy Markdown
Contributor

@ask-bonk ask-bonk Bot left a comment

Choose a reason for hiding this comment

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

/bigbonk re-review — lazy-load App Router page/route-handler modules

Re-reviewed at 8220e7f (latest, includes the failed-import fix and the reworded codegen comment). I re-traced every synchronous read of route.page / route.routeHandler against the hydration sites and ran the loader + entry-template + production-server + worker-entry suites locally — all green.

Conclusion: no correctness blocker. Both prior findings (sticky failed import, inaccurate comment) are resolved, and I could not find a new issue.

Verified correct at this commit

  • Sticky-failure fix is sound. ensureAppRouteModulesLoaded's .catch (loader:71-80) clears __loading and re-throws, leaving __loaded false so the next request retries — matching the eager per-isolate model. The .then/.catch callbacks run as a single microtask chain off the same Promise.all, so a concurrent caller can never observe a half-reset state: while pending, all callers share the one __loading promise (loader:51); once settled, late callers either see __loaded (success) or a freshly-null __loading and retry (failure). The new rejection-path test (does not cache a failed import…) covers it.

  • All hydration sites await before any module read. I confirmed each one:

    • Central match: app-rsc-handler.ts:678 hydrates before the route-handler-vs-page branch (route.routeHandler read at :696) and before dispatchMatchedPage's synchronous route.page?.default read (entry:593).
    • buildPageElements wrapper: entry:493 awaits __ensureRouteLoaded(route) before __buildPageElements. __buildPageElements is never called bypassing this wrapper.
    • Intercept + ISR-revalidation getSourceRoute targets: app-page-dispatch.ts:568 (before resolveRouteFetchCacheMode(revalidationTarget.route) at :573) and :719 (before resolveRouteFetchCacheMode(interceptRoute) at :720).
    • Action redirect target: app-server-action-execution.ts:1037 before canRenderActionRedirectTarget (reads .routeHandler/.page at :646-649) and resolveRouteFetchCacheMode(targetMatch.route) at :1063.
    • Action current route: :1047 before resolveRouteRuntime(currentMatch.route) at :1089.
    • Action re-render target: :1174 before resolveRouteFetchCacheMode(actionRerenderTarget.route) at :1176.
    • handleServerActionRequest: entry:795 awaits __ensureRouteLoaded(__actionMatch.route) before the __resolveAppPageSegmentConfig({ ..., page: __actionMatch.route.page }) read at :797.
  • getSourceRoute-resolved targets only read route.params pre-hydration. resolveAppPageInterceptionRerenderTarget / resolveAppPageActionRerenderTarget call getRouteParamNames(sourceRoute)sourceRoute.params, which is a static field always present in the route table (manifest:282). The .page read on the source route is deferred to the hydrated buildPageElement callback. Correct.

  • renderNotFound's route.page read (entry:902) is only reached with route: null (handler:664), guarded by the route ? ... : false ternary.

  • Prerender endpoint reads only staticParamsMap (app-prerender-endpoints.ts:57), which sources exclusively from eager dynamic-route page modules (mod_N?.generateStaticParams) — never lazy static-route route.page. Leaving static pages lazy introduces no prerender gap.

  • Shared mutable route objects are intentional, not a per-request leak. __loaded/__loading/page/routeHandler are mutated on the module-level singleton routes array; once loaded they stay loaded for the isolate's life. That is the cache, and it carries no per-request state.

  • CodeQL alert at getLazyLoaderVar (manifest:118-133) is a false positive — a JSON-encoded filesystem-scan path, identical trust model to the eager import * as in getImportVar two functions up. Now documented inline; fine to dismiss.

  • Comment rewording (item 1 from last round) landed correctly. manifest:157-162 now justifies route-handler laziness by "never referenced by generateStaticParamsMap (sources only layouts + page)" and notes that wiring route-handler generateStaticParams into prerendering is a pre-existing gap unaffected by this PR. Accurate.

Local verification

  • vp test run tests/app-route-module-loader.test.ts — 8 passing (incl. rejection-path retry test).
  • vp test run tests/entry-templates.test.ts — 25 passing.
  • vp test run tests/app-router-production-server.test.ts tests/app-router-worker-entry.test.ts — 42 passing, including route-handler ISR STALE/background-regen (exercises lazy route-handler hydration end-to-end), public-file serving, and hybrid production.

LGTM.

@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk Bot commented Jun 6, 2026

Review posted to PR #1781.

Summary

Re-reviewed the lazy-route-module change at 8220e7f (latest commit, with the failed-import fix and reworded comment). I re-traced every synchronous route.page / route.routeHandler read against the hydration sites and ran the relevant suites locally.

Conclusion: no correctness blocker. Both prior findings are properly resolved and I found no new issue.

What I verified:

  1. Sticky-failure fix is sound — the .catch resets __loading/re-throws; the .then/.catch run on one microtask chain so concurrent callers never see a half-reset state. New rejection-path test covers it.

  2. All 8 hydration sites await before any module read — central match, buildPageElements wrapper, intercept/ISR getSourceRoute targets, action redirect/current/re-render targets, and handleServerActionRequest (entry:795 before the route.page read at :797).

  3. No new gapsgetSourceRoute targets only read the static route.params pre-hydration; renderNotFound is guarded to route: null; the prerender endpoint reads only the eager staticParamsMap; shared route-object mutation is the intended per-isolate cache, not a per-request leak.

  4. CodeQL false positive documented inline (fine to dismiss); the comment rewording landed accurately.

Local runs: loader (8), entry-templates (25), and prod-server + worker-entry (42, including route-handler ISR which exercises lazy hydration end-to-end) — all green.

LGTM.

github run

The previous cut kept dynamic-route pages (and anything nested under a
dynamic segment) eager, because their generateStaticParams was referenced
synchronously in the module-level generateStaticParamsMap. That gutted the
benefit for a large class of apps: anything with a dynamic root segment
(i18n `[locale]`, multi-tenant `[org]`) made every route dynamic, so
nothing was lazy — and the heavy `[slug]` page (the prime heavy-module-init
candidate) stayed eager.

Now ALL page modules are lazy. generateStaticParamsMap embeds lazy `{ load }`
page sources instead of `mod.generateStaticParams`, and
createAppPrerenderStaticParamsResolver imports them on demand at prerender
time (already async). It returns the same null "no static params" sentinel
the prerender driver relies on (skip / output:export error), so SSG
behavior is unchanged. Layout generateStaticParams sources stay eager.

The eager set is now just genuinely-shared modules: layouts, templates,
boundaries, intercepts, and global-error.

Verified on real Cloudflare: 150 heavy pages under a dynamic `[locale]`
root went from eager (~450ms startup) to ~22ms, and SSG via the `[locale]`
layout's generateStaticParams still prerenders correctly.

Tests: prerender + prerender-route-params + prerender-endpoints + features +
prod/dev server + interception + actions all green; new
app-prerender-static-params unit tests cover the lazy-source path and null
sentinel; entry-templates updated for all-lazy pages and `{ load }` sources.
@james-elicx
Copy link
Copy Markdown
Member Author

Pushed ad2d9159all page modules are now lazy, including dynamic routes.

The prior cut had a real gap (thanks for the push on this): dynamic-route pages stayed eager because their generateStaticParams was referenced synchronously in the module-level generateStaticParamsMap. Since isDynamic is inherited from ancestors, any app with a dynamic root segment (app/[locale]/... i18n, app/[tenant]/...) made every route dynamic → nothing was lazy, and the heavy [slug] page (the prime heavy-module-init candidate) stayed eager too.

Now:

  • Every page module is lazy (isDynamic no longer factors into the decision).
  • generateStaticParamsMap embeds lazy { load } page sources; createAppPrerenderStaticParamsResolver imports them on demand at prerender time (already async) and returns the same null "no static params" sentinel the driver relies on — so SSG / output: export behavior is unchanged.
  • Eager set is now only genuinely-shared modules: layouts, templates, boundaries, intercepts, global-error.

Verified on real Cloudflare: 150 heavy pages under a dynamic [locale] root went from eager (~450 ms startup) to 22 ms, and SSG via the [locale] layout's generateStaticParams still prerenders correctly.

Tests: prerender + prerender-route-params + prerender-endpoints + features + prod/dev server + interception + actions all green (699 in the broad batch); added app-prerender-static-params unit tests for the lazy-source path + null sentinel; entry-templates updated for all-lazy pages and { load } sources.

@james-elicx
Copy link
Copy Markdown
Member Author

/bigbonk re-review for issues

Copy link
Copy Markdown
Contributor

@ask-bonk ask-bonk Bot left a comment

Choose a reason for hiding this comment

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

/bigbonk re-review — lazy-load all page modules, including dynamic routes

Re-reviewed at ad2d915, which is a substantive new change since my last LGTM: dynamic-route pages (and everything nested under a dynamic segment) are now lazy too, and generateStaticParamsMap was reworked to embed lazy { load } page sources resolved at prerender time. I focused this pass on the new prerender static-params resolver path and the SSG-parity claim, since that's where the risk moved.

Conclusion: no correctness blocker. The SSG/output: export sentinel is preserved and the prerender suites confirm it end-to-end.

Verified correct at this commit

  • Null-sentinel parity holds across the consumer. The behavioral shift is that createAppPrerenderStaticParamsResolver now returns null (no resolver) only when there are zero sources, whereas before it returned null whenever no source was a function. For a dynamic route with a lazy page source but no generateStaticParams, the factory now returns a resolver that yields null at call time (app-prerender-static-params.ts:85). I traced handleStaticParamsEndpoint (app-prerender-endpoints.ts:57-70): both staticParamsMap[pattern] == null (→ jsonNullResponse(), :59) and callAppPrerenderStaticParams(...) === null (→ jsonResponse(null), :70) emit an identical "null" 200 body. So the prerender driver's skip / output: export error path is unchanged. The only new cost is importing a dynamic page module at prerender time to discover it has no generateStaticParams — build-time only, acceptable.

  • Only dynamic routes get a resolver entry. buildGenerateStaticParamsEntries short-circuits on if (!route.isDynamic) continue; (app-rsc-manifest.ts:367), so purely-static routes never get a generateStaticParamsMap entry → staticParamsMap[pattern] is undefinedapp-prerender-endpoints.ts:58 returns null without importing anything. No extra eager work for static apps.

  • Source composition order is preserved. The resolver now partitions sources into [...eagerFns, ...lazyFns] (app-prerender-static-params.ts:72) rather than the original order-preserving sources.filter. This is safe for the current emission because, per route, layout sources are appended (to their prefix patterns) before the page source (to route.pattern) in buildGenerateStaticParamsEntries:369-387, and for any pattern that collects both, layout-before-page ordering is the only ordering produced — so eager-then-lazy reconstructs the same hierarchical order. (Minor robustness note below.)

  • Lazy { load } and route-table __loadPage share one loader. getLazyLoaderVar is deduped via lazyMap (app-rsc-manifest.ts:118-132), so the page module's loader is reused by both the route table and the static-params source — one import() per module, not two.

  • Central match hydration still covers the page read. ensureRouteLoaded(route) is awaited at app-rsc-handler.ts:678 before the route.routeHandler branch (:696) and before dispatchMatchedPage reads route.page?.default (app-rsc-entry.ts:593). The all-lazy change didn't move any read ahead of this gate.

  • Parallel-slot pages stay eager (app-rsc-manifest.ts:185 uses getImportVar), so the synchronous slot.page reads in segment-config resolution (app-rsc-entry.ts:597) remain valid without hydration.

  • Failed-import retry fix (prior round) is intact at app-route-module-loader.ts:71-80.

Local verification

  • tests/app-prerender-static-params.test.ts (5), tests/app-route-module-loader.test.ts (8), tests/entry-templates.test.ts (25) — all green.
  • tests/prerender.test.ts (79), tests/app-prerender-endpoints.test.ts + tests/prerender-route-params.test.ts (107 total) — all green, including output: export SSR-route error wiring and the Cloudflare Workers hybrid build (cf-app-basic, prerendered 9 routes / 2 skipped), which exercises the in-process lazy generateStaticParamsMap resolver end-to-end. This is the direct evidence for the SSG-parity claim.

Minor (non-blocking)

  1. Partition-and-concat is more fragile than the old order-preserving filter. [...eagerFns, ...lazyFns] happens to match source order only because the codegen always emits layout sources before the page source for a coinciding pattern. If a future change ever appended a lazy source before an eager one for the same pattern, composition order would silently differ from sources order. Consider resolving in original sources order (e.g. map each source to either its eager fn or its awaited lazy fn, then filter) so correctness doesn't depend on the emitter's append order. Not a bug today.

  2. Prerender-time lazy-load failure is sticky (mirror of the runtime issue, but acceptable here). resolveFns memoizes resolvePromise; if a source.load() rejects, the rejected promise is re-returned on subsequent calls and resolvedFns never sets. Unlike the runtime loader (which now resets on failure), this is build/prerender-time only, where an import failure is deterministic and fatal anyway, so caching it is fine. Noting for symmetry, no change needed.

  3. CodeQL false positive at getLazyLoaderVar is documented inline and fine to dismiss; double-hydration on the intercept path remains intentional/idempotent.

LGTM.

@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk Bot commented Jun 6, 2026

Review posted to PR #1781.

Summary

This pass focused on the new change since my last LGTM: commit ad2d915 made dynamic-route pages lazy too and reworked generateStaticParamsMap to embed lazy { load } page sources resolved at prerender time. That's where the risk moved, so I concentrated on SSG/output: export parity.

Conclusion: no correctness blocker.

What I verified:

  1. Null-sentinel parity holds end-to-end. The factory now returns a resolver (not null) for a dynamic route with a lazy page source but no generateStaticParams; that resolver yields null at call time. Both that and the old factory-returns-null case emit an identical "null" 200 from handleStaticParamsEndpoint, so the prerender driver's skip / output: export error path is unchanged. Only new cost: a build-time module import to discover "no params."

  2. Static apps pay nothingbuildGenerateStaticParamsEntries only emits resolvers for isDynamic routes; the lazy page loader is deduped/shared with the route table's __loadPage.

  3. Central match hydration still gates the route.page / route.routeHandler reads; parallel-slot pages stay eager so their synchronous reads remain valid; the prior failed-import retry fix is intact.

  4. Tests green locally: loader (8), static-params (5), entry-templates (25), and crucially the full prerender suites (107) including the Cloudflare Workers hybrid build that exercises the lazy generateStaticParamsMap resolver end-to-end — direct evidence for the SSG-parity claim.

Two minor non-blocking notes: the new [...eagerFns, ...lazyFns] partition is correctness-by-emitter-convention (more fragile than the old order-preserving filter), and prerender-time lazy-load failure is sticky (acceptable since it's build-time and deterministic).

LGTM.

github run

…solver

Per /bigbonk re-review: the resolver partitioned sources into
[...eagerFns, ...lazyFns], which only matched the intended composition order
because the codegen happens to emit layout (eager) sources before the page
(lazy) source. Resolve sources in their original declared order instead —
each source maps to its eager function or its awaited lazy function, then
non-functions are dropped — so composition order no longer depends on the
emitter's append order. Added a unit test with a lazy source ordered before
an eager one.
@james-elicx
Copy link
Copy Markdown
Member Author

Thanks @ask-bonk — addressed the non-blocking robustness note in 75165307.

Note 1 (source-order robustness — fixed). The resolver no longer partitions into [...eagerFns, ...lazyFns]. It now resolves sources in their original declared order: each source maps to its eager function or its awaited lazy function, then non-functions are dropped. Composition order no longer depends on the codegen emitting layout (eager) sources before the page (lazy) source. Added a unit test with a lazy source ordered before an eager one to lock it in.

Note 2 (sticky prerender-time load failure). Leaving as-is per your call — it's build-time and deterministic, where an import failure is fatal anyway, so memoizing it is fine. (The runtime loader still resets-and-retries.)

Notes 3 (CodeQL false positive documented inline; double-hydration intentional/idempotent) — no change.

Prerender suites (112) + resolver unit tests (6) green.

@james-elicx
Copy link
Copy Markdown
Member Author

/bigbonk re-review for issues

@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk Bot commented Jun 6, 2026

APIError: Function tools with reasoning_effort are not supported for gpt-5.5 in /v1/chat/completions. Please use /v1/responses instead.

github run

Copy link
Copy Markdown
Contributor

@ask-bonk ask-bonk Bot left a comment

Choose a reason for hiding this comment

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

/bigbonk re-review — lazy-load all App Router page modules + source-order resolver fix

Re-reviewed at 7516530 (latest), which adds the only new change since my prior LGTM: the prerender static-params resolver no longer partitions sources into [...eagerFns, ...lazyFns] — it now resolves each source in its original declared order. I re-traced the hydration sites against the all-lazy-pages change and ran the relevant suites locally.

Conclusion: no correctness blocker. LGTM.

Source-order fix verified (the one new change)

createAppPrerenderStaticParamsResolver (app-prerender-static-params.ts:53-87) now filters sources to usableSources (preserving order), then in resolveFns maps each source to its eager fn or awaited lazy fn and drops non-functions (.filter(fn !== null)). Composition no longer depends on the codegen emitting layout (eager) sources before the page (lazy) source for a coinciding pattern. The new unit test composes sources in declared order regardless of eager/lazy kind (lazy ordered before eager) locks it in. The prior [...eagerFns, ...lazyFns] fragility note is resolved.

Still-correct at this commit

  • Null-sentinel parity preserved. Zero usable sources → null (no resolver); a lazy page source with no generateStaticParams → non-null resolver yielding null at call time. handleStaticParamsEndpoint emits identical "null" for both staticParamsMap[pattern] == null and a resolver returning null, so the output: export / skip path is unchanged. Confirmed end-to-end by the Cloudflare Workers hybrid build in prerender.test.ts (9 prerendered / 2 skipped).
  • All-lazy hydration sites all await before any route.page/route.routeHandler read. Re-traced every one for the now-lazy dynamic routes:
    • Central match: app-rsc-handler.ts:678 before the route-handler branch (:696) and dispatchMatchedPage's route.page?.default (entry:593).
    • buildPageElements wrapper: app-rsc-entry.ts:493.
    • Redirect path (app-server-action-execution.ts): targetMatch.route hydrated at :1037 covers canRenderActionRedirectTarget (:1038, reads .routeHandler/.page), resolveRouteFetchCacheMode (:1063), buildServerActionPageTags/buildPageElement (:1064-65), and resolveRouteRuntime(targetRoute) (:1089); currentMatch.route hydrated at :1047 covers resolveRouteRuntime(currentRoute) (:1088).
    • Re-render target: :1174 before resolveRouteFetchCacheMode (:1176); resolveAppPageActionRerenderTarget (:1157) reads only static route.params pre-hydration.
    • Intercept + ISR getSourceRoute targets: app-page-dispatch.ts:566 and :719 before their resolveRouteFetchCacheMode reads.
    • handleServerActionRequest: entry:795 before the route.page read at :797.
  • Failed-import retry fix intact (app-route-module-loader.ts:71-80): .catch clears __loading, leaves __loaded false, re-throws; rejection-path test present.
  • Static apps pay nothing: buildGenerateStaticParamsEntries short-circuits non-dynamic routes; lazy { load } page source dedups with the route table's __loadPage via lazyMap (one import() per module).
  • Parallel-slot pages stay eager (getImportVar), so synchronous slot.page reads (entry:597) remain valid; metadata routes register their own eager getImportVar and read from importMap, unaffected by lazy page loading.
  • CodeQL alert at getLazyLoaderVar is a documented false positive (JSON-encoded filesystem-scan path; same trust model as the eager import * as). Fine to dismiss.

Local verification

  • app-route-module-loader (8), app-prerender-static-params (6, incl. the new declared-order test), entry-templates (25) — 39 passing.
  • prerender + app-prerender-endpoints + prerender-route-params (107) — green, incl. the CF hybrid build exercising the lazy resolver end-to-end.
  • app-router-production-server + app-router-worker-entry + app-page-dispatch (54) — green, incl. route-handler ISR STALE/background-regen (lazy route-handler hydration end-to-end).
  • features (311) and app-server-action-execution (58) — green.
  • vp check on all 5 changed source files — clean.

LGTM.

@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk Bot commented Jun 6, 2026

Review posted to PR #1781.

Summary

Re-reviewed at 7516530 (latest commit), which adds the only new change since the prior LGTM: the prerender static-params resolver now resolves sources in their original declared order instead of partitioning into [...eagerFns, ...lazyFns]. I verified that fix and re-traced the all-lazy-pages hydration against every synchronous route.page / route.routeHandler read.

Conclusion: no correctness blocker. LGTM.

What I verified:

  1. Source-order fix is soundcreateAppPrerenderStaticParamsResolver filters to usableSources preserving order, then maps each to its eager fn or awaited lazy fn and drops non-functions. The prior fragility note (composition depending on emitter append order) is resolved, and the new composes sources in declared order test (lazy-before-eager) locks it in.

  2. All hydration sites await before any module read — re-traced the now-lazy dynamic routes through the central match, buildPageElements, redirect target (targetMatch/currentMatch), re-render target, intercept + ISR getSourceRoute targets, and handleServerActionRequest. All read route.page/route.routeHandler only after hydration.

  3. Null-sentinel SSG parity preserved, static apps pay nothing, parallel-slot pages stay eager, failed-import retry fix intact, CodeQL alert is a documented false positive.

Local runs all green: loader (8), static-params (6), entry-templates (25), prerender suites (107, incl. CF hybrid build), prod-server + worker-entry + dispatch (54), features (311), action execution (58); vp check clean on all 5 changed source files.

github run

@james-elicx james-elicx marked this pull request as ready for review June 6, 2026 23:18
@james-elicx james-elicx merged commit eca6d3f into main Jun 6, 2026
80 of 82 checks passed
@james-elicx james-elicx deleted the feat/lazy-route-modules branch June 6, 2026 23:18
@github-actions github-actions Bot mentioned this pull request Jun 6, 2026
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.

2 participants