fix(app-router): propagate unstable_rootParams to actions and route handlers (#1452)#1652
Conversation
…andlers (#1452) `setRootParams` was only called after the post-action route match, so server actions, progressive (form) actions, and route handlers all ran before the unified request context's `rootParams` slice was populated. User code calling `unstable_rootParams()` from those contexts therefore saw `undefined` instead of the matched root layout's params. Seed `setRootParams` from a pre-action route match against the current cleanPathname so that: - route handlers observe rootParams during dispatch - server actions observe rootParams while the action runs and during the page rerender that follows a successful action - `"use cache"` functions invoked from any of those contexts read rootParams from the unified request context The legacy post-rewrite `setRootParams` call is preserved so the value is refreshed if `afterFiles`/`fallback` rewrites land on a different route (and consequently a different set of root param names).
|
/bigbonk review |
commit: |
|
There was a problem hiding this comment.
Clean, well-scoped fix. The production change is minimal and the test coverage is thorough.
What the fix does right:
- Seeds
setRootParamsfrom apreActionMatchagainst the post-beforeFilescleanPathname, which is the correct point in the pipeline — after middleware rewrites andbeforeFilesrewrites have been applied, but before action dispatch. - Reuses
preActionMatchas the initial value forlet matchat line 544 (let match = preActionMatch), avoiding a redundant secondmatchRoutecall. This is a nice efficiency gain since the match result was already computed. - The post-rewrite
setRootParamsat line 636 remains as a refresh, correctly using the final resolved route'srootParamNamesifafterFiles/fallbackrewrites land on a different route. - The no-match case (when
preActionMatchis null) is safe becausecreateRequestContextinitializesrootParamstonulland each request runs in its ownAsyncLocalStoragescope — no cross-request leakage.
Tests:
All four test cases target distinct dispatch paths (route handler, server action, progressive action, root-param filtering) and use dynamic import of the root-params shim inside the mock callbacks to read from the live unified request context. This is a solid testing pattern for verifying request-scoped state. The fourth test (only picks declared root params) is a particularly useful regression guard.
One minor observation (not blocking): the comment at line 499 says "The route is matched against the pre-rewrite cleanPathname here" — this is slightly imprecise since cleanPathname has already been mutated by middleware rewrites (line 429) and beforeFiles rewrites (line 454) by this point. It's really the "pre-afterFiles/fallback-rewrite" pathname. The intent is clear from context though.
LGTM.
|
Review posted and PR approved. Here's a summary: Verdict: Approved The fix correctly addresses #1452 by eagerly seeding Key points from the review:
I left one minor non-blocking observation about a slightly imprecise comment ("pre-rewrite" when |
Summary
Fixes #1452.
setRootParamswas only invoked after the post-action route match, so server actions, progressive (form) actions, and route handlers all ran before the unified request context'srootParamsslice was populated. Calls tounstable_rootParams()(thenext/root-paramsgetters) from those contexts sawundefinedinstead of the matched root layout's params.The fix seeds
setRootParamsfrom a pre-action route match against the currentcleanPathname, so:rootParamsduring dispatch.rootParamswhile the action runs and during the page rerender that follows a successful action."use cache"functions invoked from any of those contexts readrootParamsfrom the unified request context.The legacy post-rewrite
setRootParamscall is preserved as a refresh in caseafterFiles/fallbackrewrites land on a different route (which can have a different set of root param names).Tests
Added four targeted tests in
tests/app-rsc-handler.test.tsundercreateAppRscHandler > root params propagation (issue #1452):Confirmed all four tests fail on
mainwithout the production-code change (the route-handler test still passes because the legacy post-matchsetRootParamsruns before route-handler dispatch in the no-rewrite path; the two action tests fail). After the fix all four pass.Test plan
pnpm test tests/app-rsc-handler.test.ts— 28 passedpnpm test tests/root-params.test.ts tests/app-request-context.test.ts— 10 passedpnpm test tests/app-router.test.ts— 331 passedpnpm test tests/features.test.ts— 309 passedvp check packages/vinext/src/— no format/lint/type errorsReferences
.nextjs-ref/packages/next/src/server/request/root-params.ts.nextjs-ref/test/e2e/app-dir/app-root-params-getters/simple.test.ts