fix(app-router): match Pages navigation params in hybrid builds

[NathanDrake2406]

Summary

• emit a vinext-owned client entry manifest from client bundle metadata so hashed Pages client entries are discoverable in mixed app/ + pages/ production builds
• expose that Pages Router client entry to the Node production server and Cloudflare worker injection path so Pages fallback routes hydrate instead of staying as server-only HTML
• model Pages Router readiness for next/navigation compat hooks: pre-ready Pages snapshots expose empty search params and nullable path params, then publish the ready params once the Pages router is ready
• keep the initial Pages useParams() hydration snapshot stable by deriving route params from __NEXT_DATA__ until the Pages router readiness transition provides the real Pages context

Root Cause

The upstream navigation deploy suite exercises a Pages Router dynamic page inside a hybrid App Router project. That route was server-rendered, but vinext did not reliably discover the Pages Router client entry after the client bundle was emitted with generic hashed filenames, so production HTML for the fallback page could omit every module script needed for hydration.

Once hydration was restored, the remaining failure was identity-level: vinext treated the Pages router as immediately ready, while Next.js's Pages adapter returns null path params until router.isReady, then publishes path params through PathParamsContext. The upstream test observes exactly two params effect runs: initial hydration and the ready params object; unrelated rerenders must not create another object.

Next.js References

• Upstream failing test: test/e2e/app-dir/navigation/navigation.test.ts
• Pages fixture used by that test: pages/search-params-pages/[foo]/index.js
• Pages adapter semantics for SearchParamsContext and PathParamsContext: packages/next/src/shared/lib/router/adapters.tsx
• Client provider wiring for Pages Router navigation contexts: packages/next/src/client/index.tsx
• Pages router readiness initialization: packages/next/src/shared/lib/router/router.ts
• Pages router readiness transition during client changes: packages/next/src/shared/lib/router/router.ts
• Server-side readiness rule: packages/next/src/server/render.tsx
• Public useParams docs: nextjs.org/docs/app/api-reference/functions/use-params

Verification

• vp test run tests/client-build-manifest.test.ts tests/app-router.test.ts -t "client build manifest helpers|emits the Pages Router client entry"
• PLAYWRIGHT_PROJECT=app-router pnpm run test:e2e tests/e2e/app-router/pages-router-use-params.spec.ts
• vp run vinext#build
• vp check

References #1555

[pkg-pr-new]

Open in StackBlitz

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

npm i https://pkg.pr.new/vinext@1741

commit: f949202

[james-elicx]

/bigbonk review for issues

[ask-bonk]

Review: Pages navigation params readiness in hybrid builds

Solid, well-targeted PR. I verified the core readiness model against Next.js v16.2.6 source and it is a faithful port:

• getPagesNavigationIsReadyFromSerializedState mirrors the Pages Router constructor isReady predicate exactly (gssp || gip || isExperimentalCompile || (appGip && !gsp) || (!autoExportDynamic && !search && !__NEXT_HAS_REWRITES)). The autoExport && isDynamicRoute gating and the hasRewrites term match upstream.
• The two-phase snapshot (pre-ready null params / empty search params, then ready values published via the queued notification) correctly produces the "exactly 2 params effect runs" behavior the upstream test observes, and the isReady-keyed client context cache keeps the post-ready params object referentially stable so unrelated rerenders don't add a third run.
• isPagesRouterDocumentActive() correctly prevents Pages compat state from shadowing App Router navigation snapshots in hybrid documents (App browser entry sets window.next.appDir = true via installWindowNext), and the __VINEXT_PAGE_LOADERS__-first ordering gives Pages fallback documents priority.
• The client-entry manifest approach (emitting vinext-client-entry-manifest.json from bundle metadata) is a clean fix for discovering hashed Pages entries in mixed builds, with good unit coverage for the marker-priority lookup and on-disk fallback.

Test coverage is strong (unit + e2e + ported Next.js fixtures). A few non-blocking observations:

1. useParams<T>() return type vs. nullable pre-ready snapshots. getServerParamsSnapshot()/getClientParamsSnapshot() can now return null (pre-ready Pages state), but useParams<T>() is typed T and casts the snapshots with as () => T. This matches Next.js runtime behavior (Pages useParams returns null until ready), but the type lies to TS consumers — const p = useParams(); p.foo typechecks yet p is null during the pre-ready window. Next.js's public useParams type is Params | null. Consider widening the return type or documenting the nullable window so consumers handle it rather than hitting a runtime Cannot read properties of null.

2. Readiness flip depends on PagesRouterProvider mounting. The transition is driven by setTimeout(0) inside PagesRouterProvider's effect (router.ts ~L1862), and _pagesRouterReady + the ready notification only fire if that provider mounts. Next.js flips isReady inside Router.change() (the hydration query-update), independent of any component. If a deferred (pre-ready) Pages route renders useParams/useSearchParams from next/navigation but never instantiates a PagesRouterProvider (a page that doesn't use next/router's useRouter), the readiness transition won't fire and params stay null indefinitely. Worth confirming PagesRouterProvider always wraps Pages content in the hydration entry rather than only when useRouter() is called — or moving the ready-flip to the runtime install path so it's component-independent.

3. App Router startup now unconditionally calls installPagesClientAssetGlobals (prod-server.ts ~L1173). For pure App Router projects (no pages/) this sets __VINEXT_SSR_MANIFEST__/__VINEXT_LAZY_CHUNKS__/__VINEXT_CLIENT_ENTRY__ globals that were previously left unset. These globals are only consumed by pages-server-entry.ts, and for App-only builds they resolve to undefined/{}, so the change is benign — but consider gating on a hasPagesDir-equivalent signal or adding a comment so it's clear it's intentionally inert for App-only deployments and to avoid reading/parsing manifests that won't be used.

4. Minor dead branch in getClientParamsSnapshot (navigation.ts ~L1432). The if (state && Object.keys(state.clientParams).length > 0) return state.clientParams; block is redundant with the following return state?.clientParams ?? _EMPTY_PARAMS; — both return state.clientParams when state is truthy. The block (and the getClientNavigationState() call) can be dropped.

[ask-bonk]

Review posted successfully: #1741 (review)

Summary

I reviewed PR #1741 (fix: match Pages navigation params in hybrid builds) against the Next.js v16.2.6 source and verified the core logic is correct.

Strengths confirmed:

• getPagesNavigationIsReadyFromSerializedState is a faithful port of the Next.js Pages Router constructor isReady predicate (verified line-by-line against upstream router.ts).
• The two-phase pre-ready/ready snapshot model correctly produces the "exactly 2 params effect runs" behavior the upstream test asserts, with referentially stable post-ready params (via the isReady-keyed cache).
• isPagesRouterDocumentActive() correctly prevents Pages compat state from shadowing App Router navigation in hybrid documents.
• The vinext-owned client-entry manifest cleanly solves hashed entry discovery in mixed app+pages builds, with strong unit coverage.
• Test coverage is thorough (unit + e2e + ported Next.js fixtures).

Non-blocking observations raised (4):

1. useParams<T>() is typed T but the underlying snapshots can now return null during the pre-ready window — the as () => T cast hides a real runtime nullability; Next.js's public type is Params | null.
2. The readiness flip depends on PagesRouterProvider actually mounting (via setTimeout(0)), whereas Next.js flips isReady in Router.change() independent of any component — worth confirming the provider always wraps Pages content.
3. App Router startup now unconditionally calls installPagesClientAssetGlobals, setting globals for pure-App projects that were previously unset (benign, but broadens side-effect surface).
4. A minor dead/redundant branch in getClientParamsSnapshot.

No correctness blockers found. I posted these as a single grouped review (line-anchored comments were rejected with 422 because the target lines weren't part of the diff hunks, so I consolidated them into the summary body with file/line references).

github run

[NathanDrake2406]

@codex review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 06bc621d43

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[james-elicx]

/bigbonk review for issues

[ask-bonk]

@james-elicx Bonk workflow was cancelled.

View workflow run · To retry, trigger Bonk again.

[james-elicx]

/bigbonk review for issues

[james-elicx]

/bigbonk review for issues

[james-elicx]

/bigbonk review for issues

[ask-bonk]

Review: Pages navigation readiness in hybrid builds

Follow-up review focused on correctness and dev/prod parity. The readiness model (getPagesNavigationIsReadyFromSerializedState), the two-phase pre-ready/ready snapshots, the hashed-entry manifest, and the isPagesRouterDocumentActive() guard all look sound and well-tested. I verified SSR/client readiness parity holds because __NEXT_DATA__ now serializes the same gssp/gsp/gip/appGip/autoExport/isExperimentalCompile/__vinext.hasRewrites fields the SSR predicate consumes (pages-page-response.ts:156-162, pages-server-entry.ts:1048), so the client's shouldDeferInitialPagesRouterReady() recomputes the identical result.

I did find one genuine correctness gap plus a couple of minor cleanups.

1. (likely bug) useRouter().isReady is always true during SSR, diverging from the deferred next/navigation hooks

This PR correctly defers useParams/useSearchParams/usePathname server snapshots using ssrCtx.navigationIsReady (router.ts:560-570). But useRouter().isReady is fed by PagesRouterProvider → getRouterSnapshot() → isPagesRouterReady(), and isPagesRouterReady() returns true unconditionally on the server (typeof window === "undefined"). It never consults ssrCtx.navigationIsReady.

Consequences:

• Next.js parity gap. Next's server-side readiness rule (the same render.tsx#L641-L654 predicate this PR cites) returns false for auto-export dynamic / query-string / rewrite-capable routes, so router.isReady should be false in that initial render, not true.
• Hydration-mismatch hazard. For a pre-ready route the server now renders router.isReady === true while the client's initial _pagesRouterReady is deliberately false. Any component that reads router.isReady directly in JSX (rather than in a post-hydration useEffect, as the fixtures do) will mismatch on first paint. The existing fixtures only read pagesRouter.isReady inside useEffect, so this case is currently untested.

Suggested direction: have the server branch of the readiness check read the SSR context, e.g. make isPagesRouterReady() (or getRouterSnapshot()) return _getSSRContext()?.navigationIsReady ?? true on the server instead of always true, and add an SSR assertion that useRouter().isReady is false for an auto-export dynamic / query-string render.

2. (nit) Dead branch in getClientParamsSnapshot

The early-return is redundant with the fall-through on the next line — both yield state.clientParams when state is truthy.

3. (type-safety, carried over) useParams<T>(): T can now return null

getClientParamsSnapshot/getServerParamsSnapshot now legitimately return null during the pre-ready window, but useParams<T>() is typed T and casts via as () => T. This matches runtime behavior but the type hides the nullable window (Next's public type is Params | null). Worth widening the return type or documenting it so consumers guard before dereferencing.

4. (nit) App-only startup now reads Pages manifests

startAppRouterServer unconditionally calls installPagesClientAssetGlobals (prod-server.ts:1173), which reads the SSR manifest + client build manifest from disk even for pure App-only deployments. Benign (results resolve to undefined), but a hasPagesDir-style guard or a clarifying comment would avoid the unnecessary disk reads and make the intent clear.

[ask-bonk]

getRouterSnapshot() feeds useRouter().isReady via PagesRouterProvider. On the server isPagesRouterReady() returns true unconditionally and never reads _getSSRContext()?.navigationIsReady, so for pre-ready routes (auto-export dynamic / query string / rewrites) SSR renders isReady: true while the client hydrates with isReady: false. That diverges from Next's server readiness rule and risks a hydration mismatch for any component reading router.isReady directly in JSX.

Consider deriving readiness from the SSR context on the server:

Suggested change

	return { ...getPathnameAndQuery(), isReady: isPagesRouterReady() };
	function getRouterSnapshot(): ReturnType<typeof getPathnameAndQuery> & { isReady: boolean } {
	const isReady =
	typeof window === "undefined"
	? (_getSSRContext()?.navigationIsReady ?? true)
	: isPagesRouterReady();
	return { ...getPathnameAndQuery(), isReady };
	}

[ask-bonk]

The preceding if (state && Object.keys(state.clientParams).length > 0) return state.clientParams; is redundant with this line — both return state.clientParams whenever state is truthy. The guard block can be dropped:

Suggested change

	return state?.clientParams ?? _EMPTY_PARAMS;
	return state?.clientParams ?? _EMPTY_PARAMS;

[ask-bonk]

Review posted successfully: #1741 (review)

Summary

I reviewed PR #1741 against the Next.js v16.2.6 source. The core design is sound and well-tested — I verified SSR/client readiness parity holds because __NEXT_DATA__ serializes the same fields the readiness predicate consumes, so the client recomputes the identical isReady result.

I found one likely correctness bug plus three minor items:

1. (bug) useRouter().isReady is always true during SSR. This PR correctly defers the next/navigation hooks (useParams/useSearchParams/usePathname) using ssrCtx.navigationIsReady, but useRouter().isReady flows through getRouterSnapshot() → isPagesRouterReady(), which returns true unconditionally on the server and never reads the SSR context. For pre-ready routes (auto-export dynamic / query string / rewrites) the server renders isReady: true while the client hydrates isReady: false — a Next.js parity gap and a hydration-mismatch hazard for any component reading router.isReady directly in JSX. The fixtures only read it inside useEffect, so this case is untested. I posted a suggestion to derive server readiness from _getSSRContext()?.navigationIsReady.

2. (nit) Dead branch in getClientParamsSnapshot — redundant early-return; posted a suggestion.

3. (type-safety) useParams<T>(): T can now return null during the pre-ready window but the type hides it via as () => T.

4. (nit) App-only startup now unconditionally reads Pages manifests via installPagesClientAssetGlobals.

Codex's earlier getInitialProps concern is already addressed (both dev and prod check pageModule.default?.getInitialProps), and review #2's PagesRouterProvider-mounting concern is mitigated since the Pages client entry always wraps content via wrapWithRouterContext. The two line-anchored suggestions were accepted by the API this time.

github run

[james-elicx]

/bigbonk review for issues

[ask-bonk]

Review: Pages navigation params readiness in hybrid builds

Reviewed for correctness and dev/prod parity. The PR is well-architected and the previous review rounds (the useRouter().isReady SSR bug, the Page.getInitialProps detection, the dead branch) have all been addressed in the current diff. I verified the following against Next.js v16.2.6 source:

• Readiness predicate (getPagesNavigationIsReadyFromSerializedState, router.ts:728) is a faithful port of the Pages Router constructor isReady rule (gssp || gip || isExperimentalCompile || (appGip && !gsp) || (!autoExportDynamic && !search && !hasRewrites)).
• SSR/client parity for __NEXT_DATA__: prod (pages-page-handler.ts:395-404 + serializedPagesNextData at :465) and dev (dev-server.ts:577-586 + serializedPagesNextData at :1252) serialize the identical gssp/gsp/gip/appGip/autoExport/__vinext.hasRewrites fields the client shouldDeferInitialPagesRouterReady() reads, so the client recomputes the same isReady. The dev hasPageGip check now reads pageModule.default?.getInitialProps matching prod.
• useRouter().isReady SSR fix (router.ts:789-801): now derives from _getSSRContext()?.navigationIsReady on the server instead of unconditional true, with a regression test in shims.test.ts. Removes the hydration-mismatch hazard.
• isReady accessor on the Router object (router.ts:2325) correctly switched from a static true to a getter delegating to isPagesRouterReady().
• Readiness flip depends on PagesRouterProvider mounting via setTimeout(0); confirmed wrapWithRouterContext always wraps Pages content in both SSR (pages-page-handler.ts:502,629) and the Pages client entry (pages-client-entry.ts:186), and App documents never mount it but are gated out by isPagesRouterDocumentActive().
• Root-cause fix (the hashed Pages client entry discovery): the new vinext-client-entry-manifest.json (index.ts:2730) plus installPagesClientAssetGlobals (prod-server.ts:1086) restore __VINEXT_CLIENT_ENTRY__ for Pages fallback routes in mixed app+pages App Router prod/Cloudflare builds, which collectAssetTags (pages-asset-tags.ts:133) injects as the hydration script.
• nextConfig?.rewrites.beforeFiles.length (index.ts:3776, pages-server-entry.ts:367) follows the existing normalized-config pattern (rewrites is always { beforeFiles, afterFiles, fallback }) — not a new throw hazard.

Ran locally: client-build-manifest.test.ts (12 passed), the new shims readiness tests, and the pages-router.test.ts nav-compat suite — all green.

No correctness blockers. Two minor, non-blocking observations below.

[ask-bonk]

(nit) SSR vs. client param-resolution ordering differs. Here the SSR path tries getRouteParamsFromQuery(ssrCtx.pathname, ssrCtx.query) before extractRouteParamsFromPath, while the client path in _buildClientPagesNavigationContext (router.ts:496) does the reverse — extractRouteParamsFromPath first, then getRouteParamsFromQuery. For ordinary dynamic and catch-all routes the two helpers resolve to the same params, so this is benign in practice. But if a query key ever collides with a route-param name (or a serialized query array diverges from path extraction), SSR and client could publish different params on the ready snapshot, risking a subtle post-hydration mismatch. Worth aligning the two orderings to keep them provably identical.

[ask-bonk]

(nit, carried over) App-only startup now reads Pages manifests. startAppRouterServer unconditionally calls installPagesClientAssetGlobals, which reads ssr-manifest.json + manifest.json from disk and sets __VINEXT_SSR_MANIFEST__/__VINEXT_CLIENT_ENTRY__/__VINEXT_LAZY_CHUNKS__ even for pure App-only deployments (no pages/). It's benign — those globals are only consumed by the Pages renderer (pages-asset-tags.ts) and resolve to undefined/{} for App-only — but a hasPagesDir-style guard or a one-line comment noting it's intentionally inert for App-only builds would avoid the unnecessary disk reads and clarify intent.

[ask-bonk]

Review posted successfully: #1741 (review)

Summary

I reviewed PR #1741 for correctness and dev/prod parity against Next.js v16.2.6 source. The PR is well-architected and faithful to Next.js semantics. All three concerns from prior review rounds (the useRouter().isReady SSR bug, Page.getInitialProps detection, and the dead branch) are already resolved in the current diff.

Verified correct:

• The readiness predicate is a faithful port of the Pages Router constructor isReady rule.
• SSR/client __NEXT_DATA__ parity holds — dev and prod serialize the same flags the client readiness check recomputes; the dev hasPageGip check now matches prod (pageModule.default?.getInitialProps).
• The useRouter().isReady server fix derives from navigationIsReady instead of unconditional true, with a regression test.
• The readiness flip via PagesRouterProvider always mounts in Pages renders (wrapWithRouterContext), and App documents are gated out by isPagesRouterDocumentActive().
• The root-cause fix (hashed Pages client entry discovery via the new vinext-client-entry-manifest.json + installPagesClientAssetGlobals) correctly restores hydration for Pages fallback routes in mixed app+pages builds.
• The nextConfig?.rewrites.beforeFiles.length access follows the existing normalized-config pattern — not a new throw hazard.

I ran the client-build-manifest, shims readiness, and pages-router nav-compat tests locally — all green.

Posted 2 non-blocking nits:

1. SSR vs. client param-resolution helper ordering differs (getRouteParamsFromQuery first on server, extractRouteParamsFromPath first on client) — benign for normal routes but worth aligning for provable parity.
2. App-only startup now unconditionally reads Pages manifests via installPagesClientAssetGlobals (benign; a hasPagesDir guard or comment would clarify intent).

No correctness blockers found.

github run

[james-elicx]

/bigbonk review for issues

[james-elicx]

/bigbonk review for issues

[ask-bonk]

Review: Pages navigation params readiness in hybrid builds

Reviewed for correctness and dev/prod parity against Next.js semantics. The PR is sound and the substantive concerns from prior review rounds are all resolved in the current diff. I independently verified:

• Readiness predicate (getPagesNavigationIsReadyFromSerializedState, router.ts:734) faithfully mirrors the Pages Router constructor isReady rule (gssp || gip || isExperimentalCompile || (appGip && !gsp) || (!autoExportDynamic && !search && !hasRewrites)), gating the dynamic-route branch on autoExport as Next.js does.
• useRouter().isReady SSR fix (router.ts:803-806) now derives from _getSSRContext()?.navigationIsReady instead of unconditional true, removing the hydration-mismatch hazard, with a regression test in shims.test.ts.
• Dev/prod __NEXT_DATA__ parity: dev (dev-server.ts:580-604) and prod (pages-page-handler.ts:395-439) serialize the identical gssp/gsp/gip/appGip/autoExport/__vinext.hasRewrites fields the client shouldDeferInitialPagesRouterReady() recomputes, and both now detect gip via pageModule.default?.getInitialProps (Codex's earlier concern). The prod main render path sets nextData on the SSR context (pages-page-handler.ts:439); the ISR-revalidation re-render at pages-page-data.ts:589 omits it, but that branch only runs for already-cached GSP routes (autoExport: false), so the pathname-null logic is unaffected.
• Readiness flip depends on PagesRouterProvider mounting via setTimeout(0); confirmed wrapWithRouterContext always wraps Pages content in SSR (pages-page-handler.ts:502) and the Pages client entry (pages-client-entry.ts:186), and App documents are gated out by isPagesRouterDocumentActive(). The notify wiring (PAGES_NAVIGATION_NOTIFY_KEY) correctly re-reads the navigation snapshot through useSyncExternalStore after the flip.
• Root-cause fix: the new vinext-client-entry-manifest.json (index.ts:2730, facade matching handles the \0 prefix at index.ts:556) plus installPagesClientAssetGlobals (prod-server.ts:1178) restore __VINEXT_CLIENT_ENTRY__ for hashed Pages entries in mixed app+pages builds. The App-only-startup unconditional call now carries an explanatory comment (prod-server.ts:1173-1177).
• The dead branch in getClientParamsSnapshot is gone; the function is clean (navigation.ts:1433).

No correctness blockers. Two non-blocking observations:

1. (type-safety, now newly reachable) useParams<T>(): T can return null

The hook casts the snapshot via getClientParamsSnapshot as () => T / getServerParamsSnapshot as () => T (navigation.ts:1533/1538-1539). On main, getServerParamsSnapshot() returned Record<...> and defaulted to _EMPTY_PARAMS — it could never be null. This PR changes both snapshot helpers to return Record<...> | null and now legitimately returns null during the pre-ready Pages window (navigation.ts:1436/1440/1452). So the as () => T cast hides a null that previously could not occur: const p = useParams(); p.foo typechecks but throws at runtime during the pre-ready window. Next.js's public type is Params | null. Consider widening the return type to T | null (or documenting the nullable window) so consumers guard before dereferencing. This was raised in earlier rounds and remains open.

2. (nit) Confirm SSR/client ready-snapshot param ordering stays aligned

The SSR ready path resolves params query-first — getRouteParamsFromQuery(ssrCtx.pathname, ssrCtx.query) then extractRouteParamsFromPath (router.ts:570-572) — and _buildClientPagesNavigationContext uses the same order (router.ts:501-504), with the comment at router.ts:495 noting query-first is intentional for byte-for-byte SSR/client parity. They look aligned. Just flagging to keep them provably identical: if a query key ever collided with a route-param name, divergent ordering would publish different params on the ready snapshot and risk a post-hydration mismatch. No change needed if they stay in lockstep.

[ask-bonk]

Review posted successfully: #1741 (review)

Summary

I reviewed PR #1741 for correctness and dev/prod parity against Next.js semantics. The PR is well-architected and all substantive concerns from prior review rounds are resolved in the current diff. I independently verified each fix against the source rather than relying on the prior bot reviews.

Verified correct:

• The readiness predicate (getPagesNavigationIsReadyFromSerializedState) is a faithful port of the Pages Router constructor isReady rule.
• The useRouter().isReady SSR fix now derives from _getSSRContext()?.navigationIsReady instead of unconditional true, with a regression test.
• Dev/prod __NEXT_DATA__ serialization parity holds — both paths emit the identical readiness flags and both detect gip via pageModule.default?.getInitialProps. I also traced the ISR-revalidation re-render path (pages-page-data.ts:589) that omits nextData and confirmed it's benign (only runs for cached GSP/autoExport:false routes).
• The readiness flip wiring (PagesRouterProvider mount → markPagesRouterReady → notify → useSyncExternalStore re-read) is sound, and wrapWithRouterContext always wraps Pages content.
• The root-cause manifest fix correctly handles the \0 facade prefix and restores hashed Pages client entries in hybrid builds.
• The dead branch is removed; the App-only-startup comment is added.

Two non-blocking items raised:

1. (type-safety, newly reachable) useParams<T>(): T now hides a real null — the snapshot helpers changed from never-null on main to nullable during the pre-ready window, so the as () => T cast can crash consumers. Next.js's public type is Params | null.
2. (nit) Flagged keeping SSR/client ready-snapshot param-resolution ordering provably aligned (they currently are).

No correctness blockers. Inline anchors were rejected with 422 (target lines outside diff hunks), so I consolidated into the review body with file:line references.

github run

[james-elicx]

/bigbonk review for issues