refactor: extract app route handler execution helpers#624
refactor: extract app route handler execution helpers#624southpolesteve merged 1 commit intomainfrom
Conversation
8c59175 to
09f3378
Compare
|
/bigbonk |
commit: |
|
There was a problem hiding this comment.
Review: Extract app route handler execution helpers
Clean refactoring that moves the inline route-handler execution logic from the generated app-rsc-entry.ts template into a typed, testable app-route-handler-execution.ts runtime module. The behavioral equivalence is well-preserved.
What looks good
-
Faithful extraction. The
executeAppRouteHandler()function reproduces the original inline logic from the generated entry template:setHeadersAccessPhase("route-handler")/ restore infinallyblockconsumeDynamicUsage()→ run handler →consumeDynamicUsage()(reset-before, check-after pattern preserved inrunAppRouteHandler)shouldApplyAppRouteHandlerRevalidateHeader→applyRouteHandlerRevalidateHeadershouldWriteAppRouteHandlerCache→markRouteHandlerCacheMiss+ clone + async cache write viawaitUntil- Cookie collection, draft mode,
clearRequestContext, andfinalizeRouteHandlerResponse→applyRouteHandlerMiddlewareContext— all in the correct order - Error path:
getAndClearPendingCookies()on catch,resolveAppRouteHandlerSpecialError, redirect/status discrimination,reportRequestErrorfor generic failures,clearRequestContextbefore returning error responses
-
Background regeneration reuse. The ISR stale-while-revalidate background regen path now uses
runAppRouteHandler()instead of duplicating thecreateTrackedAppRouteRequest+consumeDynamicUsagedance inline. This is a nice DRY improvement. -
Clean interface. The
ExecuteAppRouteHandlerOptionsinterface is well-typed — each dependency is an explicit function/value rather than relying on module-level state from the generated entry. This makes the unit tests possible and makes the contract between the generated code and the runtime explicit. -
Good test coverage. The test file covers the happy path (static response with cache write), dynamic usage detection (skips cache, marks route), and error handling (redirect via digest, generic error with
reportRequestError). ThecreateDynamicUsageState()helper is a clean way to simulate the global reset/check pattern.
Minor observations (non-blocking)
-
paramstype annotation.RunAppRouteHandlerOptions.paramsis typed asRecord<string, string | string[]>, but the generated entry passes the match result directly (which could have additional prototype quirks frommatchPattern). This works fine in practice because the params are spread into a plain object bymakeThenableParamsbefore reaching the handler. Just noting for awareness. -
Snapshot reduction is dramatic (~762 lines removed). This is a direct consequence of the refactoring — the inline route handler code was repeated in multiple snapshot variants. The smaller snapshots are a maintenance win.
-
clearRequestContextcall in the error path. In the original code, the error path calledsetHeadersContext(null); setNavigationContext(null)before theresolveAppRouteHandlerSpecialErrorcheck (for special errors like redirect). The new code callsclearRequestContext()after callinggetAndClearPendingCookies()and before checkingspecialError. This is the same effective ordering — good.
Verdict
Straightforward mechanical extraction with correct behavioral equivalence. Tests cover the key paths. No regressions expected.
|
I've reviewed and approved PR #624. Here's a summary of my findings: Verdict: Approved The PR cleanly extracts ~115 lines of inline route-handler execution logic from the generated
|
Summary
app-route-handler-execution.tsruntime moduleapp-rsc-entry.tsVerification
vp check packages/vinext/src/server/app-route-handler-execution.ts packages/vinext/src/entries/app-rsc-entry.ts tests/app-route-handler-execution.test.ts tests/app-router.test.ts tests/__snapshots__/entry-templates.test.ts.snapvp test run tests/app-route-handler-execution.test.ts tests/app-route-handler-runtime.test.ts tests/app-route-handler-response.test.ts tests/app-route-handler-policy.test.tsvp test run tests/entry-templates.test.tsvp test run tests/app-router.test.ts -t 'route handler'vp test run tests/nextjs-compat/app-routes.test.tsvp run vinext#buildStacked on #622.
Written by Codex.