mirror of
https://github.com/cloudflare/vinext.git
synced 2026-09-14 19:04:59 +08:00
vinext@0.1.1
241 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Nathan Nguyen
|
3a1e56748f |
fix(app-router): match Pages navigation params in hybrid builds (#1741)
* fix(app-router): emit pages client entry for hybrid builds * fix pages router navigation params readiness * fix(pages-router): track hashed fallback client entry * fix(pages-router): honor navigation readiness in hooks * fix(pages-router): derive server router.isReady from SSR readiness Address review feedback on #1741: - getRouterSnapshot() now reads navigationIsReady from the SSR context on the server instead of unconditionally returning true. For pre-ready Pages routes (auto-export dynamic / query-string / rewrite-capable builds) this keeps useRouter().isReady consistent between SSR and client hydration, avoiding a hydration mismatch for components that read it in JSX. Mirrors Next.js render.tsx's server readiness rule. - Drop the redundant guard branch in getClientParamsSnapshot. - Add a regression test asserting server useRouter().isReady reflects the SSR navigationIsReady context. * fix(pages-router): align param resolution and dev/prod readiness parity Address review feedback (ask-bonk + self-review) on #1741: - Align client Pages navigation params to query-first, matching the server snapshot and Next.js's adaptForPathParams (derives path params from router.query). __NEXT_DATA__.query is kept current on client navigation, so the two snapshots are now provably identical and avoid a latent post- hydration mismatch. - Populate window.__VINEXT_PAGE_PATTERNS__ in the dev server (own script tag) so the next/navigation compat hooks can resolve a dynamic pattern from a resolved path in dev exactly as in production. Closes a dev/prod parity gap (prod set this via the client entry; dev never did). - Drop the redundant pre-readiness setSSRContext in dev; the single post- module-load call now publishes navigationIsReady before rendering, matching the prod handler. - Document that installPagesClientAssetGlobals is intentionally inert for App-only builds, and that getReadonlyPagesSearchParams' string-keyed cache is safe to share across concurrent SSR requests (immutable wrappers). - Add a dev regression test asserting __VINEXT_PAGE_PATTERNS__ exposure. * fix(pages-router): prefer URL params for client snapshots * fix(app-router): gate pages asset globals * test(app-router): add production test for hybrid basePath + assetPrefix * fix(navigation): widen usePathname return type to string | null * refactor(pages-router): dedupe navigation readiness modeling and cache pattern scan Addresses code-review cleanup findings on the Pages Router navigation readiness work: - Extract buildPagesReadinessNextData() so the dev SSR handler and the production Pages page handler derive the gssp/gsp/gip/appGip/autoExport __NEXT_DATA__ readiness flags from one place, instead of two hand-kept copies that could drift and produce a dev/prod hydration mismatch. - Extract resolvePagesNavigationPathname() shared by the client and SSR branches of getPagesNavigationContext, so the pathname null-ness rule (fallback / pre-ready auto-export dynamic) cannot diverge between them. - Memoize resolvePagesRoutePatternForPath()'s pattern scan, which ran an uncached O(routes) loop on every useSyncExternalStore snapshot read. - In getServerSearchParamsSnapshot, serialize the source only when the object-identity fast path misses, avoiding a toString() on every read. * refactor(pages-router): consistent ISR readiness data + robustness cleanups Addresses further code-review findings: - Thread the readiness flags (gssp/gsp/gip/appGip/autoExport) through the ISR background-regeneration path: renderPagesIsrHtml now serializes the same __NEXT_DATA__ readiness state the initial render emits, so a regenerated, re-cached page hydrates with the identical initial router.isReady the server computed. Previously these flags were dropped on that path — benign today (ISR-eligible getStaticProps routes never trigger the flag-gated early-ready branches) but a latent dev/prod hydration-readiness drift. Adds a shared PagesNextDataExtras type so every render path stays in lockstep. - readSsrManifest: degrade a malformed/unparseable SSR manifest to "no SSR manifest" with a warning instead of throwing and aborting server startup, restoring the previous tolerant behavior. - pages-readiness: project PagesReadinessNextData from the canonical VinextNextData and accept PagesPageModule, instead of bespoke types that could drift from the __NEXT_DATA__ shape. - getReadonlyPagesSearchParams: drop the redundant string-keyed cache slot; the per-object WeakMap is authoritative and concurrency-safe on its own. - isPagesRouterReady: drop the redundant server-side window check. * fix(pages-router): restore string-keyed readonly search-params cache Reverts the cache simplification from the previous commit. The string-keyed slot is load-bearing, not redundant: across the Pages Router pre-ready → ready transition the navigation context swaps in a new URLSearchParams instance even when the query string is unchanged, so the per-object WeakMap alone returns a fresh ReadonlyURLSearchParams wrapper and useSearchParams() loses Object.is stability — re-firing a [searchParams] effect. Caught by the rewrite-driven pre-ready static page e2e (search-params-snapshots saw ['',''] not ['']). Keeps the improved comment explaining why both cache levels are needed. --------- Co-authored-by: James <james@eli.cx> |
||
|
James Anderson
|
9596b72495 |
fix(pages-router): pass revalidateReason "on-demand" to gsp/gssp (#1462) (#1856)
* fix(pages-router): pass revalidateReason "on-demand" to getStaticProps (#1462) Implements res.revalidate() for Pages Router API routes and wires the on-demand revalidation trigger through to getStaticProps/getServerSideProps context as revalidateReason: "on-demand". res.revalidate(urlPath) issues an internal HEAD request to the target path carrying the x-prerender-revalidate header (mirroring Next.js's api-resolver). The Pages render path (dev-server and prod pages-page-data) detects that header, bypasses the fresh/stale cache-hit short-circuits, and regenerates the entry synchronously with reason "on-demand". The build and stale reasons already worked; this finishes the on-demand case. Closes #1462 * fix(pages-router): authenticate on-demand revalidation with a secret (#1462) The on-demand `revalidateReason` detection treated `x-prerender-revalidate` as an unauthenticated presence check and sent the literal value "1". Any external client could send `x-prerender-revalidate: 1` to force synchronous regeneration of any ISR page, bypassing the fresh/stale cache short-circuits — a cache-stampede/DoS vector. Mirror Next.js's `checkIsOnDemandRevalidate`: a request is only treated as on-demand revalidation when the `x-prerender-revalidate` header value EQUALS a secret (the vinext analog of `previewModeId`). Introduce a per-process revalidate secret (`getRevalidateSecret`, 256-bit random hex via Web Crypto) that `res.revalidate()` attaches to its internal loopback request, and a constant-time `isOnDemandRevalidateRequest` equality check on the receiving dev-server and Pages page handler. Header presence alone is never honored. Also fix the test's HTML-reason extraction to avoid CodeQL's incomplete-sanitization / bad-HTML-regex findings, and add a negative test asserting forged `x-prerender-revalidate` values ("1", "", a guess) are rejected while the real secret is accepted. * refactor(pages-router): share res.revalidate() impl; support unstable_onlyGenerated (#1462) Address non-blocking bonk review findings: - Extract the two near-identical `res.revalidate()` implementations (`api-handler.ts`, `pages-node-compat.ts`) into a shared `performOnDemandRevalidate` helper so the secret wiring and success detection cannot drift between dev and prod. - Align success detection with Next.js's api-resolver: accept `x-nextjs-cache: REVALIDATED`, status 200, or a 404 when the caller passed `unstable_onlyGenerated`. Add `unstable_onlyGenerated` support (`x-prerender-revalidate-if-generated` header). * docs(pages-router): note x-nextjs-cache REVALIDATED branch is parity-only (#1462) * fix(pages-router): make on-demand revalidate secret build-time (shared across isolates) The on-demand ISR revalidation secret was a per-process random value. On Cloudflare Workers, res.revalidate()'s loopback fetch() can land on a different isolate than the sender, so a per-process secret mismatches across isolates and false-rejects legitimate revalidations. Mirror Next.js's previewModeId: generate one 256-bit secret at build time (in the vinext build CLI, once per build, shared across plugin instances like __VINEXT_SHARED_BUILD_ID) and bake it SERVER-ONLY into every server bundle via a Vite define (__VINEXT_REVALIDATE_SECRET). It is byte-for-byte identical in every isolate and never reaches the client bundle. getRevalidateSecret() reads the baked constant at runtime and falls back to a process-shared random secret in dev (single-process, no regression). Also persist revalidateSecret in vinext-server.json for diagnostics/tooling parity with prerenderSecret. Constant-time safeEqual validation and reject-arrays/empty/absent behavior are unchanged. Refs #1462 * test(pages-router): cover server-only revalidate secret; drop dead manifest reader Address ask-bonk review of the build-time revalidate secret: - Add tests pinning the security-critical invariant that the __VINEXT_REVALIDATE_SECRET define is baked into server environments (rsc/ssr) but NEVER into the client bundle (the client env returns null outright). A leak would ship the secret to every browser and re-open the cache-stampede vector the equality check prevents. Also harden the existing "no-ops when defineServer unset" test to explicitly clear the env var instead of relying on it happening to be unset. - Drop the unused readRevalidateSecret export and the orphaned revalidateSecret manifest field — the runtime authenticates against the baked define (both Workers isolates and the Node prod-server run the bundle), so the on-disk copy had no consumer (knip-flagged dead code). - Hoist the client early-return in vinext:compiler-define-server so the server-only guarantee is structural (removes the redundant name check), and reword the dev-fallback comments ("absent unless set during vinext build"). Refs #1462 |
||
|
James Anderson
|
c669930f56 |
fix(build): correct CSS ordering for global-not-found (#1549) (#1858)
* fix(build): correct CSS ordering for global-not-found (#1549) * test(build): assert global-not-found CSS ordering in production (#1549) Adds a production-build regression test for #1549: builds the global-not-found-css-order fixture, serves it via the Vite preview server, and asserts the route-miss 404 document serves global-not-found's own CSS (red wins) without leaking the root layout's stylesheet (green), while matched routes still serve the layout's CSS. Verified the test fails without the framework-chunk split in client-build-config.ts (the 404 leaks the layout's green) and passes with it. * chore: update lockfile for global-not-found-css-order test fixture (#1549) * refactor(build): migrate RSC framework chunk to codeSplitting; add unit tests Address bonk review on #1858 (issue #1549): - Replace the deprecated rolldown `advancedChunks` output config with the non-deprecated `codeSplitting` form for Vite 8+, mirroring the client build. Stops the per-prod-build deprecation warning. - Derive both RSC_FRAMEWORK_CHUNK_TEST (regex) and isRscFrameworkModule (predicate) from a single FRAMEWORK_PACKAGES list so they can't drift. - Add focused unit tests for createRscFrameworkChunkOutputConfig (both the Vite 7 manualChunks and Vite 8+ codeSplitting branches) and for the framework package matching (matches react/react-dom/scheduler/ react-server-dom-webpack, excludes react-icons / @react-aria/*). |
||
|
James Anderson
|
29a040dd21 |
fix(app-router): include route pattern in repeated-slash Link warning (#1554) (#1855)
* fix(app-router): include route pattern in repeated-slash Link warning (#1554) The repeated forward-slashes Link warning added in #1563 fired with the wrong "in page" segment: it used `window.location.pathname` (or `"/"` during SSR) instead of Next.js's route pattern (`router.pathname`, e.g. `/my/path/[name]`). The Next.js compat test `repeated-forward-slashes-error` asserts the *full* message including that route pattern, so it kept failing in the deploy suite despite #1563 — the `/Invalid href/` regex matched but the `toContain` of the exact message did not. Source the page segment from the current render's route pattern via a new `getCurrentRoutePathnameForWarning()` helper: the Pages Router SSR context's route pattern on the server, `window.location.pathname` on the client, falling back to `"/"`. Normal Link behaviour is unchanged. Add an SSR-level compat test (app + pages router) that renders a Link with a repeated-slash href and asserts the exact warning, plus the upstream dynamic-route fixture `pages/my/path/[name]`. Finishes #1554 (the warning wasn't firing with the right page name despite #1563). Closes #1554 * fix(app-router): read route pattern via side-effect-free accessor The previous commit imported `router.ts` into `link.tsx` to source the route pattern for the repeated-slash warning. But `router.ts` has a browser-only top-level side effect (`installWindowNext({ router })`), so pulling it into the Link shim — which is in the App Router client bundle too — clobbered `window.next.router` with the Pages Router instance, breaking App Router client navigation (useParams/prefetch/interception E2E timeouts). This is exactly why `navigation.ts` deliberately avoids importing `router.ts` and reads the Pages nav context through a `Symbol.for` handle. Mirror that pattern: add a side-effect-free `route-pattern-for-warning` module that reads the SSR route pattern through a `Symbol.for` accessor published by the server-only `router-state.ts`. The Link shim imports only this tiny module, so no Pages Router side effect leaks into the App Router bundle. Behaviour is unchanged: pages-router SSR reports `in page: '/my/path/[name]'`, client/App Router fall back to `window.location.pathname` then `"/"`. |
||
|
James Anderson
|
dd3fb05977 |
test(pages-router): cover javascript: URL blocking (#1576) (#1854)
* fix(pages-router): emit console.error for javascript: URL navigation (#1576) Finishes the Pages Router half of #1576 (the App Router warnings already land). Adds the four Pages Router scenarios from the upstream test/e2e/app-dir/javascript-urls suite — Link `href`, Link `as`, router.push, and router.replace — as e2e fixtures + a Playwright spec, plus unit coverage for the shared Link shim's dangerous-click handler. No production change is required: the Link shim already emits the canonical "Next.js has blocked a javascript: URL as a security precaution." console.error on click for both `href` and `as`, and the Pages Router router.push/replace synchronous guard from #1575 already does the same. This PR adds the missing Pages Router coverage that proves all four scenarios. Closes #1576 * test(pages-router): suppress dev error overlay before safe navigation The router.push/replace javascript: scenarios throw synchronously (the #1575 behaviour that surfaces the console.error), which trips vinext's dev error overlay. Its backdrop intercepted the follow-up safe-link click, timing out the test even though the security block already passed. Suppress the overlay via the existing disableDevErrorOverlay helper after asserting the block, matching how other pages-router/app-router specs handle intentional dev-time errors. * test(pages-router): cover javascript: URL blocking in production build The Pages Router javascript: URL guard (Link dangerous-click handler in shims/link.tsx + router.push/replace synchronous guard in shims/router.ts, both via shims/url-safety.ts) is already correct on main. The existing e2e spec, however, only runs against the dev server (vp dev), while the Next.js deploy suite exercises these scenarios against a production build where the dev error overlay is absent and the client bundle is minified. Add a pages-router-prod Playwright spec that re-runs all four upstream scenarios (Link href, Link as, router.push, router.replace) against vinext build + vinext start, so a prod-only regression is caught here. Refs #1576 |
||
|
James Anderson
|
ca690db954 |
test(actions): cover server action POST under node-runtime middleware (#1480) (#1847)
* test(actions): cover server action POST under node-runtime middleware (#1480) Adds regression coverage for cloudflare/vinext#1480: a server-action POST must still be intercepted and executed when a node-runtime middleware matches the action path and consumes the request body before falling through with NextResponse.next(). The app-basic fixture's middleware now matches `/nextjs-compat/action-node-mw` and reads the POST body (mirroring Next.js' `middleware-node.js`), with a bound form server action on that page. Two tests exercise the scenario: - tests/nextjs-compat/action-node-middleware.test.ts (dev inline-middleware path) - tests/app-router-production-server.test.ts (production RSC entry path, the layer the upstream deploy suite exercises) vinext already isolates the middleware's body branch from the downstream request (applyAppMiddleware / createNextRequest clone the body) and treats node-runtime middleware identically to edge middleware, so the action POST is dispatched correctly. These tests lock that behavior in across the dev and production request paths. Closes #1480 * style(test): apply formatter to action-node-mw prod test * test(actions): declare runtime:nodejs on fixture middleware for #1480 Address review: make the app-basic fixture middleware faithfully mirror Next.js' middleware-node.js by exporting config.runtime = 'nodejs'. vinext runs all middleware on Node regardless, so this is a behavioural no-op for the other matcher entries, but it makes the action-node-mw regression a true guard for the named node-runtime scenario from the issue title. * docs(test): point #1480 fixture comment at middleware-runtime config logic Address review nit: the config-reading logic (matcher-only, no runtime branch) lives in server/middleware-runtime.ts, so reference that file where a future runtime branch would land. |
||
|
James Anderson
|
49d753217b |
fix(app-router): propagate "use cache" tags to route-handler ISR entries (#1453) (#1848)
* fix(app-router): propagate "use cache" tags to route-handler ISR entries (#1453) * fix(app-router): bubble nested use-cache tags to parent on data cache HIT (#1453) |
||
|
James Anderson
|
600ba69b4d |
fix(routing): propagate middleware draft cookie to pages/api fallback (#1520) (#1845)
* fix(routing): propagate middleware draft cookie to pages/api fallback (#1520)
In a hybrid app (app/ + pages/api/*), requests with no matching App Router
route fall through to the Pages Router via renderPagesFallback. The bridge
forwarded middleware-mutated request headers and response headers, but dropped
the __prerender_bypass Set-Cookie emitted by draftMode().enable()/disable()
inside middleware — that cookie lives in a request-scoped side channel that
App Router route handlers and page renders read via getDraftModeCookieHeader(),
but the Pages fallback path never did.
Thread getDraftModeCookieHeader through the bridge and append the draft cookie
to the Pages Router fallback response, matching how App Router responses surface
it. Closes the draft-mode flow called out in #1520.
Closes #1520
* test(routing): cover draft-cookie fallback in app-pages-bridge unit test
Add getDraftModeCookieHeader to the renderPagesFallback dependency stub and
assert the draft cookie is appended (API + renderPage branches) and omitted
when middleware did not enable draft mode. Keeps the unit test in sync with
the new required dependency.
* docs(routing): document hybrid-dev draft-mode limitation in pages bridge
Address review feedback on #1845: clarify in app-pages-bridge that the
draft-cookie fallback covers production (inline RSC handler), while hybrid dev
runs middleware in a separate Vite Pages SSR runner where draftMode() inside
middleware is not yet permitted — a pre-existing limitation tracked separately
from #1520. The unit-test regression flagged by review was already fixed in
|
||
|
James Anderson
|
6958dfd0b4 |
fix(build): expand require.context into import.meta.glob map (#1501) (#1836)
* fix(build): expand require.context into import.meta.glob map (#1501) * style: format require-context fixture page * fix(build): strip g/y regex flags and default require.context recursive to true Address ask-bonk review on #1836: - Strip global/sticky flags when reconstructing the filter regexp; a stateful g/y RegExp.test() drops every other matching module. - Default the omitted recursive arg to true to match Webpack's useSubdirectories default (was silently false). - Add a g-flagged fixture/test case locking in the flag-stripping fix. - Clarify the misleading descent comment in collectRequireContextCalls. * test(require-context): cover ctx(key) retrieval and MODULE_NOT_FOUND Address ask-bonk follow-up: lock in the runtime contract beyond .keys(). - Assert ctx('./parent/file1.js').default resolves the module namespace. - Assert an unknown key throws with code 'MODULE_NOT_FOUND'. * docs(require-context): document extensionless no-regex parity caveat Address ask-bonk note: with no filter regexp, import.meta.glob only surfaces resolvable modules, so extensionless keys Webpack would include can be dropped. Documented as a low-risk divergence (upstream Next.js's extensionless test is disabled/Turbopack-pending). |
||
|
James Anderson
|
3f6c174c20 |
fix(app-router): resolve query-only Link href against current path (#1540) (#1844)
A <Link href={{ query: {...} }} /> with no `pathname` was resolved by
defaulting the base to "/", producing "/?query" instead of a query-only
href. The shallow navigation then recorded the wrong history entry (the
site root) so back/forward traversal landed on the wrong URL/page.
Mirror Next.js's formatUrl() (`pathname = urlObj.pathname || ''`): when
`pathname` is omitted, emit a relative query-only href (`?query`) so the
existing relative-href resolution applies it against the current path.
Adds a ported e2e regression (middleware-rewritten shallow link history
traversal) plus a Link unit test.
Closes #1540
|
||
|
James Anderson
|
def033614b |
fix(app-router): render built-in fallback when global-error.tsx throws (#1548) (#1837)
* fix(app-router): render built-in fallback when global-error.tsx throws (#1548) When a user's app/global-error.tsx itself throws while rendering, vinext previously crashed the request with a raw 500. Next.js instead nests the user's global-error inside an outer boundary whose fallback is the built-in default global-error, so a broken boundary degrades to a usable 'This page couldn't load' document. This change mirrors that structure: - Client-side: wrap the user's global-error in a GlobalErrorBoundary whose fallback is the built-in DefaultGlobalError (matches Next.js's outer RootErrorBoundary errorComponent={DefaultGlobalError}). - Server-side: React's renderToReadableStream rejects on a shell-level throw even with an error boundary present, so when the global-error boundary throws during SSR we re-render with the built-in default global error. Scoped to the global-error boundary (which owns the whole document); other boundaries propagate as before. The original error message still reaches the boundary in dev and a digest in prod via sanitizeErrorForClient (unchanged). Adds nextjs-compat coverage (dev + production preview) ported from test/e2e/app-dir/global-error/error-in-global-error. Closes #1548 * fix(app-router): format TRACKING.md table and address review notes (#1548) - Reflow TRACKING.md global-error table (vp check formatting). - Document GlobalErrorBoundary's intentional lack of a resetKey. - Add a port note explaining the SSR-via-pathname throw deviates from upstream's client useEffect throw (HTTP/SSR suite has no browser). * test(app-router): unit-cover global-error server-side fallback retry (#1548) Add a focused renderAppPageErrorBoundary unit test that asserts a throwing global-error boundary re-renders with the built-in default global-error (HTTP 200), locking in the try/catch retry directly rather than only via the fixture-coupled integration test. * fix(app-router): re-throw navigation signals from throwing global-error (#1548) Narrow the server-side global-error fallback so it only degrades genuine render failures to the built-in default. Navigation/HTTP-access signals (redirect/notFound/forbidden/unauthorized) thrown from within global-error are re-thrown so they reach their dedicated handlers instead of being swallowed into a built-in 200. Addresses review feedback that the catch was broader than Next.js's React error boundary. Adds a unit test asserting a notFound() signal from global-error propagates. * docs(app-router): clarify global-error signal re-throw comments (#1548) Soften wording: re-thrown navigation signals propagate to the top-level handler (same as before this change), rather than reaching dedicated handlers from this position. No behavior change. |
||
|
James Anderson
|
de2ad5278f |
fix(app-router): respect reactMaxHeadersLength for preload Link header (#1552) (#1841)
* fix(app-router): respect reactMaxHeadersLength for preload Link header (#1552) * test(app-router): cover reactMaxHeadersLength config resolution * refactor(app-router): prioritize React preloads in Link header; cover buildAppPageLinkHeader Address review feedback on #1841: - Order React preload hints before font preloads so render-critical entries survive a tight reactMaxHeadersLength cap. - Rename the buildAppPageHtmlResponse option to linkHeader (it now carries the combined header, not just fonts). - Add unit coverage for buildAppPageLinkHeader (cap, 0-disables, ordering, whole-entry truncation). |
||
|
James Anderson
|
97947ce775 |
fix(app-router): apply trailingSlash to route handler request URL (#1827) (#1835)
* fix(app-router): apply trailingSlash to route handler request URL (#1827) App Router route handlers received a NextRequest whose `url` was normalized with the default `trailingSlash: false` policy, so a request to `/runtime/edge/` (with `trailingSlash: true`) surfaced `req.url` without the trailing slash even though `req.nextUrl.pathname` kept it. Thread the configured `trailingSlash` through the route-handler dispatch chain (dispatch -> execution/cache -> tracked request -> NextRequest) so `NextURL` applies the correct policy when computing `request.url`, matching Next.js (test/e2e/app-dir/app-routes-trailing-slash). Closes #1827 * test(app-router): cover node runtime for trailingSlash:false route handler (#1827) Address ask-bonk review note on PR #1835: the trailingSlash:false suite only exercised the edge runtime. Parameterize it over edge + node to match the trailingSlash:true suite's coverage. |
||
|
Divanshu Chauhan (divkix)
|
b4c829d6a2 |
feat(interception): sibling-style interception routes (#1364 Part C) (#1804)
* test(interception): add failing sibling-interception graph tests * feat(routing): discover sibling-style interception markers * feat(routing): emit sibling interception manifest facts * feat(app-router): serialize sibling intercepts in route manifest * feat(app-router): match sibling intercepts in route matcher * feat(app-router): render sibling intercept as full page response * test: add siblingIntercepts field to AppRoute test fixtures * test(interception): add sibling interception fixtures * fix(app-router): emit null proof for sibling intercepts to bypass slot-validation * test(interception): add sibling interception e2e specs * fix(interception): correct (...)→(..)(..) marker in interception-from-root fixture * fix(interception): resolve sourceRouteIndex to slot-owner route in intercept lookup When a route inherits a @slot from an ancestor via discoverInheritedParallelSlots, the inherited slot's interceptingRoutes are copied from the ancestor. Previously, createInterceptLookup used routeIndex (the inheriting descendant's index) as sourceRouteIndex, causing resolveAppPageInterceptState to see sourceRoute === currentRoute → kind='current-route' instead of kind='source-route'. Fix: build a patternToIndex map and resolve sourceRouteIndex via the intercept's sourceMatchPattern (which names the actual slot-owner route), falling back to routeIndex only when no match is found. * fix(interception): use locale-prefixed href so middleware interception fires correctly * fix(interception): address codex review issues in sibling intercept implementation - Pass effectiveParams to resolveAppPageHead so generateMetadata/generateViewport sees the intercepted route's params instead of the source route's params - Rename SIBLING_INTERCEPT_SLOT_NAME from __page to __vinext_sibling_intercept to prevent collision with a user @__page parallel route directory - Restore middleware-rewrite fixture link without locale prefix so the interception-dynamic-segment-middleware spec exercises the intended middleware rewrite path * fix(test): update slotId assertions and restore locale-prefixed fixture link - Update test assertions from slot:__page to slot:__vinext_sibling_intercept following the rename in commit |
||
|
Nathan Nguyen
|
6005541c19 |
fix(app-router): restore scroll snapshots on back (#1743)
* fix(app-router): restore scroll snapshots on back * fix(app-router): invalidate scroll snapshots with bfcache epoch * fix(app-router): guard synchronous popstate scroll restore * fix(app-router): strip basePath for snapshot restore targets * fix(app-router): canonicalize snapshot restore target search * docs: document sync-over-async invariant in popstate snapshot restore * docs: document cross-module ordering contracts per review Adds comments for three load-bearing invariants identified in review: - rememberHistoryStateSnapshot layout effect ordering - synchronousPopstateScrollRestoreNavigationId single-slot latch - restoredBfcacheIds/reuseCurrentBfcacheIds snapshot timing |
||
|
Nathan Nguyen
|
5dd7a34eb6 |
fix(app-router): isolate page CSS chunks in production (#1738)
* fix(app-router): isolate production page CSS chunks Production App Router builds statically imported every page module into the generated RSC manifest. That let Vite/Rolldown concatenate page-level global CSS from sibling routes into shared CSS assets, so a sibling hash page with scroll-padding-top changed native hash scroll offsets on unrelated pages. The manifest now emits cached dynamic loaders for page modules and loads the matched page module at the route dispatch boundary. Page generateStaticParams sources use the same loader with an explicit missing-export sentinel so prerender still distinguishes absent exports from malformed results. * test(app-router): add focused unit test for route module loader Address the non-blocking review note from PR #1738: per AGENTS.md, behavior moved out of a template string should get a focused helper test. Cover createRouteModuleLoader (promise dedup, rejection propagation), loadRouteModules (page assignment, concurrent dedup, no-op cases, missing `page` key) and loadRouteMatch (null short-circuit, match load). Also cover createLazyGenerateStaticParamsSource (delegation, missing sentinel for null/empty/non-function exports). Document the rejected-load cache intent and the per-request intercept copy in the WeakMap so future readers don't try to "fix" them as bugs. * test: stabilize dev overlay HMR recovery * chore: retrigger ci * chore: fix knip check config * test(app-router): guard intercepted modal CSS + load lazy page before probe Adds the positive-direction guard the bonk review asked for: navigating /feed -> intercepted /photos/[id] must load the lazy modal page and emit its scroll-padding-top CSS chunk. The existing test only proved the negative (CSS absent on a direct /feed visit), so a regression where the lazy modal page failed to load its CSS would have passed silently. Also fixes the probePage() intercept branch, which read intercept.page without resolving the now-lazy __pageLoader. The render path awaits the loader (resolveAppPageInterceptState), but the probe path did not, so the dynamic-bailout probe silently inspected an undefined component and never observed the intercept page's searchParams/headers access. Resolve the lazy page before probing to match the render path. --------- Co-authored-by: James <james@eli.cx> |
||
|
Nathan Nguyen
|
cda19e1c58 |
fix(app-router): preserve recent segment state with Activity BFCache (#1739)
* fix(app-router): preserve recent segment state with Activity BFCache Fresh App Router navigations were reusing the active React tree when moving between dynamic segment values. That leaked client state into a newly entered segment and made the upstream back-forward-cache suite fail before it even reached browser traversal assertions. The root cause was that vinext had history-entry bfcacheId plumbing but no separate segment state-key cache. Next.js treats those identities separately: render state is keyed by segment identity/pathname, while useRouter().bfcacheId identifies a concrete history entry. Add state-key maps for app router slots, render retained entries through React.Activity when cacheComponents is enabled, keep the active one-entry keyed reset path otherwise, and port focused Next.js regression coverage for the three-entry BFCache behavior. * fix(bfcache): store stateKeyMap in retained slot entries and re-provide on render Review feedback requested two changes: 1. Add stateKeyMap to BfcacheSlotEntry so each retained BFCache entry carries the state-key map that belonged to it when active, rather than borrowing the current navigation's map. This prevents nested hidden entries from being rendered with old elements but new pathname-derived state keys — an identity corruption bug that upstream Next.js avoids by rendering each retained entry from its own state. 2. Add a nested dynamic regression test: /nested/[section]/item/[id] with client counters in both the [section] layout and [id] page. The existing flat /page/[n] tests do not exercise nested retained-entry identity, which is exactly where the missing state-key-map bug lives. Changes: - BfcacheSlotEntry gains stateKeyMap?: Readonly<Record<string, string>> - updateBfcacheSlotEntries accepts and stores an optional stateKeyMap argument - BfcacheSlotBoundary stores the current stateKeyMap alongside elements when caching the active entry - Both the Activity visible/hidden path and the non-cacheComponents single-entry path now wrap rendered entries in BfcacheStateKeyMapContext.Provider using the entry's own stateKeyMap (falling back to current map for forward compatibility) - New e2e test verifies nested layout + page counter preservation across segment navigations * refactor(bfcache): extract shared provider stack into BfcacheEntryProviders BfcacheSlotBoundary rendered the BfcacheStateKeyMapContext / ElementsContext / SegmentContext provider stack in two places: the one-entry keyed reset path and the per-entry Activity retention list. The two stacks were identical except for where the React key sat, so the retained-entry bookkeeping and its render shape were tangled together and harder to audit than needed. Extract BfcacheEntryProviders as a pure presentational helper and render it from both paths. The keyed-reset path keeps its remount semantics: the key now sits on the BfcacheEntryProviders element, so a fresh active stateKey still remounts the whole provider subtree. Hook ordering is unchanged. All hooks stay above the early returns in BfcacheSlotBoundary, so the boundary still satisfies the Rules of Hooks across the activeStateKey undefined-to-defined transition. Covered by the slot BFCache entry-state suite in tests/shims.test.ts. * test(bfcache): isolate cacheComponents BFCache coverage in a dedicated fixture The back/forward-cache e2e suite ran against the shared app-basic fixture with cacheComponents: true. app-basic backs the single app-router Playwright server that every app-router spec uses, so enabling cacheComponents globally changed a core DOM invariant for unrelated tests: inactive route trees stay mounted as hidden Activity DOM. The contamination already leaked a :visible workaround into the interception-modal bfcacheId spec, which had nothing to do with Activity. cacheComponents is an opt-in semantic mode, not default app-router behaviour, so it does not belong on the shared baseline fixture. Move the BFCache routes into a dedicated tests/fixtures/app-bfcache fixture with cacheComponents: true, served by a new app-router-bfcache Playwright project on port 4183, and run the back-forward-cache spec there against the project baseURL. The :visible locators stay in that spec, where duplicate hidden DOM is the expected contract. Remove cacheComponents and the BFCache routes from app-basic so the shared fixture models default behaviour again, and revert the :visible workaround in use-router-bfcache-id.spec.ts now that the modal route no longer retains hidden DOM. Fix the nested item page import while moving it: nested/[section]/item/[id]/ page.tsx imported ../counter, but the counter lives two levels up at nested/[section]/counter.tsx, so it must be ../../counter. Validation: PLAYWRIGHT_PROJECT=app-router-bfcache (5 passed) and the full use-router-bfcache-id spec under app-router (11 passed), plus the slot BFCache unit suite (1172 passed) and vp check (formatting + lint + types clean). * ci: run the app-router-bfcache e2e project in the matrix The dedicated app-router-bfcache Playwright project was added to playwright.config.ts but not to the CI E2E matrix, so the back/forward-cache specs never ran in CI. Add it to the matrix project list so the BFCache suite runs on every push. It uses the default chromium browser, so the existing setup and browser-cache steps apply unchanged. * fix(bfcache): keep persisted slots mounted when cacheComponents is off Without cacheComponents there is no Activity retention, yet BfcacheSlotBoundary still keyed the active entry by its segment stateKey. That stateKey tracks the pathname (createBfcacheSegmentIdentity), so any slot whose identity moves with the URL (shared layouts, interception source slots) was remounted on navigation, discarding client state that survives a normal route change. The baseline router reconciles these slots in place. This regressed seven existing app-router e2e tests once cacheComponents was no longer masking it on the shared fixture: interception source-context and modal preservation in advanced.spec.ts, and layout-persistence.spec.ts dynamic-segment layout counter survival. Fresh-entry reset is driven by userland bfcacheId keying in the fixtures, not by remounting the slot subtree, so the keying was pure overreach in the non-cacheComponents path. Render the single active entry unkeyed when cacheComponents is off so the boundary reconciles in place like the baseline. The keyed Activity retention path is unchanged for cacheComponents. Validation: full app-router project 436 passed / 0 failed (the seven regressions now pass); use-router-bfcache-id 11 passed (fresh-push reset intact); app-bfcache 5 passed (Activity retention intact); shims unit suite 1024 passed. * docs(test): point bfcacheId spec comment at the app-router-bfcache project The header noted that Activity-backed preservation is covered in back-forward-cache.spec.ts, but that spec moved to the dedicated app-bfcache fixture under the app-router-bfcache project. Name the project and path so the cross-reference stays accurate. * docs: align cacheComponents comment with unkeyed active-segment behavior * test(bfcache): assert fresh bfcacheId after Activity-restored return * style: collapse locator chains per oxfmt * refactor(bfcache): split slot entry order from snapshots * fix(bfcache): keep non-cacheComponents slot path baseline * fix(bfcache): use string cacheComponents define * test: remove external fetch from revalidate tag fixture * fix(bfcache): harden Activity state key retention * docs(bfcache): document cacheComponents Activity caveats * fix(bfcache): preserve navigation context for fallback SSR Fallback boundary rendering can outlive the original request-scoped navigation context. That made app SSR fail fast with a missing navigation context when loading-boundary digest routes rendered their HTTP fallback shell. The SSR boundary contract still requires a navigation context for real request renders, but boundary renderers now synthesize the same typed shape from the request URL and matched params when request scope has already been cleared. * refactor(bfcache): normalize pathname in state-key map, document invariant callers Moves hydration-pathname normalization into createBfcacheSegmentStateKeyMap so SSR and client always produce byte-identical Activity keys. Documents the requireNavigationContext guaranteeing callers per review feedback. * fix(bfcache): tolerate malformed state-key pathnames * docs(bfcache): document known Next.js divergence and dead-branch contract * chore(deps): update lockfile for vite 0.1.24 * fix(app-router): preserve encoded delimiters in bfcache keys * docs(bfcache): track router-owned retention follow-up * chore: retrigger ci * fix(app-router): preserve redirect bridge fallback |
||
|
Nathan Nguyen
|
aab9240e10 |
fix(app-router): hard navigate streamed redirects (#1742)
* fix(app-router): hard navigate streamed redirects * fix(navigation): guard page redirect marker checks * test(app-router): count redirect document loads |
||
|
Nathan Nguyen
|
bc31644649 |
fix(app-router): match streaming metadata error responses (#1794)
* fix(app-router): match streaming metadata error responses Vinext currently serializes metadata-originated redirects into a Flight payload for document requests. That response is not an HTML page, so direct browser visits cannot follow the streamed redirect and html-limited bots receive 200 instead of the blocking redirect. The dispatcher also renders HTTP access fallback head metadata from layouts only, so not-found boundary metadata is omitted when generateMetadata() throws notFound(). Use Next.js's split response model: RSC requests keep the Flight redirect digest, streaming-capable document requests receive the refresh meta tag HTML, and html-limited bots fall through to the HTTP redirect. Thread the selected HTTP access boundary module into fallback rendering so boundary metadata is resolved exactly once. * test(app-router): cover metadata RSC redirects The unit CI failure came from a strict dispatch test receiving optional fallback fields with undefined or null values. Those fields are only meaningful when a boundary module is actually selected. Build fallback option objects without absent boundary fields, preserving the existing call shape for ordinary page-level HTTP access fallbacks. Add focused coverage for fromMetadata redirects on RSC requests so metadata document refresh handling cannot accidentally consume the client navigation Flight path. * docs(app-router): update metadata redirect transport docs and drop dead boundary fallback Address review feedback on the metadata streaming error split: - Update the stale APP_PAGE_METADATA_ERROR_MARKER docblock to describe the new three-way transport split (flight digest / HTML refresh meta tag / 307). - Note the intentional minimal-stub HTML divergence from Next.js's full-document insertion in buildMetadataRedirectHtmlResponse. - Drop the redundant resolveAppPageHttpAccessBoundaryComponent fallback in renderAppPageHttpAccessFallback (boundaryModule already covers both paths). * docs(app-router): fix stale metadata redirect docs in head resolver and test reference Address remaining bonk re-review nits: - Update resolveModuleMetadata docblock in app-page-head.ts to describe the three-way transport split (it still claimed redirects ride in the flight payload even for document SSR). - Point the APP_PAGE_METADATA_ERROR_MARKER Next.js test reference at metadata-streaming, which is where the behavioural contracts are ported from. --------- Co-authored-by: James <james@eli.cx> |
||
|
Divanshu Chauhan (divkix)
|
330de786c5 |
test(interception): fixtures and E2E specs for 4 dynamic interception suites (#1364 Part B) (#1785)
* test(interception): add fixtures and E2E specs for 4 dynamic interception suites Adds fixture pages and Playwright E2E specs for the 4 Next.js interception route test suites tracked in issue #1364 (Part B): - interception-dynamic-segment: @modal/(.)[username]/[id] and @modal/(.)test-nested/@sidebar — 6 tests (all pass) - interception-dynamic-single-segment: @modal/(.)groups/[id]/new from a multi-segment source route — 5 tests - interception-dynamic-segment-middleware: locale-rewrite middleware + (.)[username]/p/[id] interception — 4 tests - parallel-routes-and-interception-from-root: (...)[locale]/intercepted from the app root — 3 tests Suite 3 middleware rewrite is tightly scoped to /interception-mw/* in the shared app-basic middleware.ts to avoid interfering with other tests. Tests for suites 2/3/4 that depend on the computeInterceptTarget fix (fix/interception-compute-target, Part A) will fail until that branch is merged, which is expected. * test(interception): mark 7 known-failing tests with test.fail() pending Part C The 7 tests across suites 2-4 exercise behavior that Part C (#1364) will fix. Marking them test.fail() keeps CI green while preserving the specs as executable documentation of the expected behavior. |
||
|
Divanshu Chauhan (divkix)
|
cf04ca63e3 |
fix(form): DISALLOWED_FORM_PROPS stripping, file input warning, viewport prefetch, pages-router E2E (#1752)
* fix(form): implement DISALLOWED_FORM_PROPS, file input warning, viewport prefetch, and pages-router E2E - Strip method/encType/target props with dev console.error (Gap 1) - Warn in dev when FormData contains a File value (Gap 2) - Add viewport IntersectionObserver prefetch for App Router (Gap 3) - Add prefetch prop (false | null) to FormProps - Add pages-dir fixture and E2E spec for Pages Router form tests (Gap 4) - Fix Rules of Hooks: move useRef/useCallback/useEffect before conditionals - Update unit tests: stub React 19 dispatcher for hook-safe direct rendering Closes #1355 * test(form): fix 'renders a form with method prop' to assert stripping behavior * fix(form): address code-review findings — JSDoc, parity warnings, prefetch dedup - Fix misleading JSDoc: pages-dir prefetch prop "has no effect" (not "emits a warning") - Add dev warning for invalid prefetch value (not null/false) - Add dev warning for replace/scroll/prefetch passed with a function action - Gate viewport-prefetch IntersectionObserver with getPrefetchedUrls() / hasPrefetchCacheEntryForNavigation() dedup check to match link.tsx - Add 5 new test cases covering the three new warning paths * style(form): fix formatting in form.test.ts * fix(form): match Next.js warning text verbatim, reuse useMergedRef, skip prefetch for unsafe actions Addresses bonk review feedback: - Invalid prefetch warning now matches Next.js verbatim: "The `prefetch` prop of <Form> must be `false` or `null`" - Function-action warnings split into the two distinct Next.js messages (one for `prefetch`, one for `replace`/`scroll` with doc links), gated separately to match app-dir/form.tsx - Reuse the shared useMergedRef helper instead of hand-rolling ref merge - Viewport prefetch now early-returns on unsafe actions, matching the submit-path isSafeAction gate - Update form.test.ts assertions to the new verbatim text * fix(form): run dev-mode prop validations before DISALLOWED_FORM_PROPS strip to match Next.js order * fix(form): match Next.js validation ordering and bail for server/client action submitters Addresses bonk review feedback: - Run prefetch/replace/scroll dev validations before the DISALLOWED_FORM_PROPS strip loop, matching upstream app-dir/form.tsx ordering - Bail out of GET interception for React server-action submitters (`name` starting with $ACTION_ID_/$ACTION_REF_, dev-only silent bail) and client-action submitters (`formAction="javascript:..."`), ported from Next.js form.tsx / form-shared.tsx - Add unit tests covering both submitter bail paths * fix(form): match Next.js dev-validation order and gate submitter warnings Addresses bonk re-review parity findings: - Gate the three console.error calls in hasUnsupportedSubmitterAttributes behind the dev check, matching Next.js form-shared.tsx (the bail itself still runs in all environments). Previously they fired in production. - Hoist checkFormActionUrl(action, "action") into the top dev-validation block so it runs first, matching upstream app-dir/form.tsx ordering. * refactor(form): drop dead-code GET method check for upstream parity Addresses bonk re-review finding: the form-level `method` prop is stripped via DISALLOWED_FORM_PROPS and any non-GET submitter `formmethod` override is already caught by hasUnsupportedSubmitterAttributes, so the explicit getEffectiveMethod GET check was dead code. Upstream app-dir/form.tsx has no equivalent check. Remove it (and the now-unused getEffectiveMethod helper) and document why the method is always GET at that point. * fix(form): align viewport prefetch fetch with link.tsx (priority + purpose) Addresses bonk re-review finding: the Form viewport prefetch fetch() omitted the `priority: "low"` and `purpose: "prefetch"` options that link.tsx passes (link.tsx:445-451). Without them, RSC prefetches competed with higher-priority requests and weren't tagged as prefetches in devtools / Sec-Purpose. Align for consistency with the Link prefetch path. * fix(form): thread interception context + mounted slots into viewport prefetch Addresses bonk re-review substantive finding: the viewport prefetch built headers with createRscRequestHeaders() (no interceptionContext, no mounted-slots header) and keyed the prefetched set on the bare rscUrl. The actual navigation (and link.tsx / the router) compute getPrefetchInterceptionContext(href) + getMountedSlotsHeader() and key on encodeCacheKey(rscUrl, interceptionContext). For intercepted target routes or pages with mounted parallel slots, the Form's prefetched cache key wouldn't match the navigation lookup, so the prefetch was a silent cache miss (wasted work, no dedup against a nearby <Link>). Mirror the Link/router computation: build context-aware headers, key the prefetched set on encodeCacheKey, and thread interceptionContext/mountedSlotsHeader through hasPrefetchCacheEntryForNavigation and prefetchRscResponse. * docs(form): note prefetch effect dep-array redundancy is lint-mandated Bonk flagged actionHref as redundant alongside action in the prefetch effect's dep array. It is derived purely from action + the module constant, but the exhaustive-deps lint requires it to be listed since the effect references it. Document this so it isn't mistaken for an oversight. --------- Co-authored-by: James <james@eli.cx> |
||
|
Stephen Zhou
|
06f37e9c1b |
fix: client HMR dev overlay recovery (#1758)
* Fix client HMR dev overlay recovery * Stabilize dev overlay build error recovery test * Harden dev overlay recovery handling |
||
|
Nathan Nguyen
|
0ecd05dfe1 |
test(app-router): remove live network from ISR revalidation fixtures (#1756)
* test(app-router): remove live network from ISR revalidation fixtures The App Router ISR revalidation fixtures fetch httpbin.org only to collect next.tags before rendering request ids. That makes CI depend on external network latency even though the tests never assert the remote response body. Use a deterministic data URL for the tagged fetch in the parent and nested fixtures so tag collection still exercises the ISR invalidation path without waiting on an external service. * test: remove incidental external fetches A layout observation unit test and a dynamic-error fixture used external URLs even though their contracts only need fetch observation or no-store policy detection. Those URLs can start unnecessary network work when the test behavior does not depend on a remote response. Use deterministic data URLs so the same fetch branches and cache options are exercised without relying on external network availability. |
||
|
Stephen Zhou
|
fb233b0a34 |
feat: improve dev error overlay source frames (#1746)
* Improve dev overlay stack source mapping * Polish dev error overlay stack display * Add dev overlay code frames * Align dev overlay message spacing * Add dev overlay error copy action * Fix dev overlay knip exports * Polish dev overlay layout * Polish dev error overlay spacing * Fix dev overlay coverage and source frame metadata * Fix dev overlay recovery for RSC HMR errors * Handle Vite HMR build errors in dev overlay * Fix knip * Use Shadow DOM for dev error overlay |
||
|
Stephen Zhou
|
e3090b08fa |
fix: dev overlay browser sourcemap stacks (#1731)
* Add browser runtime sourcemap regression test * Update pnpm lockfile * Fix dev overlay browser sourcemap stacks * Move dev stack sourcemap resolution to server * Fix knip unused stack endpoint export * Fix dev stack sourcemap proxy host handling * test: cover dev stack sourcemap helpers * fix: remove unused sourcemap type export * refactor: share dev stack sourcemap endpoint * chore: trigger ci rerun * fix: cap dev stack sourcemap request body |
||
|
James Anderson
|
f8481ac04d |
fix(css): preserve distinct media filenames for CSS url() assets (#1725)
* fix(css): preserve distinct media filenames for CSS url() assets
Match Next.js `asset/resource` behaviour for CSS `url()` dependencies:
emit them as files under `/_next/static/media/` instead of inlining, and
keep byte-identical sources (e.g. `dark.svg` / `dark2.svg`) as separate
output files instead of letting Vite/Rolldown's content-based asset
dedupe collapse them onto one filename.
There is no config switch to disable that dedupe (vitejs/vite#8632). The
native escape hatch is `this.emitFile({ fileName })`, which is never
deduped. A build-only, client-scoped pair of plugins:
- mark: during the client CSS transform, tag each relative asset
`url()` with a private `?vinext_css_url_asset=<source-basename>`
query — the only durable carrier of per-reference provenance, since
once Rolldown dedupes, the bundle can't tell which reference came
from which source.
- restore: at `generateBundle`, read the marker back, emit a sibling
file under the wanted source name via the `fileName` escape hatch,
rewrite the reference, and strip the marker.
Each reference resolves from its own marker, so split CSS chunks stay
correct without a shared cursor or bundle-iteration-order dependence.
This is a slimmer reimplementation of the approach explored in #1703:
same mark/restore mechanism and public surface, but the restore side
drops the AssetIndexes machinery, filename-reservation loop, and
multi-fallback lookups in favour of a single basename map + emitted-set,
since the sibling-filename construction (source-stem + content-hash +
ext) makes those guarded cases unreachable.
Tests port the Next.js scss/url-global e2e into the committed
`pages-basic` and `app-basic` fixtures (built in isolation), covering
the single-stylesheet two-identical-urls case and cross-chunk provenance.
Co-authored-by: Nathan Nguyen <146415969+NathanDrake2406@users.noreply.github.com>
* fix(css): merge duplicate vite import; drop ReDoS-flagged regex; clarify comments
- tests/css-url-assets.test.ts: merge the two `vite` imports into one
(import/no-duplicates lint error that failed CI Check).
- client-build-config.ts: replace `/\/+$/` trailing-slash strip with a
linear loop so CodeQL stops flagging a (false-positive) ReDoS.
- index.ts: reword the css-url mark transform's `map: null` comment — the
marker shifts columns transiently but is stripped before final output.
- css-url-assets.ts: note the basename-uniqueness assumption in assetsByBase.
Addresses review feedback on #1725.
Co-authored-by: Nathan Nguyen <146415969+NathanDrake2406@users.noreply.github.com>
* test(css): use dedicated minimal fixtures for url() asset tests
The url() asset tests previously added routes to the shared app-basic /
pages-basic fixtures and built those whole apps in isolation. That made
the tests CPU-heavy and, run concurrently under CI, contended with
app-router.test.ts's static-export test enough to push it past its 30s
timeout (it passes comfortably locally and on main; the extra parallel
load was the trigger). The added app-basic route also slightly inflated
every app-basic-wide test (static export renders all routes).
Move the fixtures into dedicated minimal committed apps
(tests/fixtures/css-url-assets-{app,pages}) that build in ~1-2s. Same
observable contract (byte-identical dark.svg/dark2.svg referenced from
one stylesheet, plus the split-chunk provenance case), now isolated from
the heavyweight shared fixtures and fast enough not to contend.
Co-authored-by: Nathan Nguyen <146415969+NathanDrake2406@users.noreply.github.com>
* docs(test): update css-url-assets header to reference the dedicated fixtures
The file header still described the pre-refactor layout (pages-basic /
app-basic paths that no longer exist). Point it at the dedicated
css-url-assets-{pages,app} fixtures. Comment-only.
Co-authored-by: Nathan Nguyen <146415969+NathanDrake2406@users.noreply.github.com>
---------
Co-authored-by: Nathan Nguyen <146415969+NathanDrake2406@users.noreply.github.com>
|
||
|
James Anderson
|
51690038d8 |
fix(pages-router): cancel in-flight nav on gSSP/gSP data redirect (#1465) (#1691)
* fix(pages-router): return __N_REDIRECT JSON for gSSP/gSP data redirects (#1465) When getServerSideProps/getStaticProps returns `{ redirect }` on a client-side `_next/data` navigation, vinext emitted a real HTTP redirect. The client's fetch transparently followed it to the destination's HTML (not a valid data envelope), forcing a hard reload with console errors and leaving navigation on the wrong page. Mirror Next.js: for data requests, reply 200 with `__N_REDIRECT` / `__N_REDIRECT_STATUS` in pageProps. The client router now detects these markers and re-dispatches a fresh client navigation to the destination, which increments the navigation id and supersedes (cancels) the in-flight navigation so the intermediate page never commits. HTML requests still emit a real HTTP redirect. Fixed in both server paths to keep dev/prod parity (see AGENTS.md): - prod: buildPagesRedirectResponse() in pages-page-data.ts branches on isDataReq - dev: writeGsspRedirect() in dev-server.ts branches on isDataReq - client: navigateClientData() handles pageProps.__N_REDIRECT via handleDataRedirect(), preserving push/replace mode - tests: data-endpoint redirect shape (vitest) + client cancellation (e2e) * fix(pages-router): hard-navigate to external __N_REDIRECT destination (#1465) Address review: handleDataRedirect() passed the originating page URL to the hard-navigation fallback for external/non-absolute __N_REDIRECT destinations, which would loop back to the redirecting page (e.g. an OAuth callback at https://example.com/...). Hard-navigate to the destination instead, matching Next.js's handleHardNavigation({ url: destination }). Drop the now-unused fallback parameter and add a server test asserting an external destination is preserved verbatim in the data envelope. |
||
|
한만욱
|
82b2720701 |
feat(app-router): support useRouter bfcacheId semantics (#1588)
* feat(app-router): implement useRouter bfcacheId semantics * fix(app-router): refine bfcacheId navigation behavior * test(app-router): align bfcache history state expectations * fix(app-router): handle catch-all bfcache identities * fix(app-router): align bfcache context semantics * fix(app-router): avoid hidden bfcache DOM retention * fix(app-router): hydrate bfcache ids from history state * fix(app-router): handle bfcacheId redirects and slots * test(app-router): expect active slot route ids * Fix bfcache id history restoration * Clarify bfcache id sentinel formatting * Address bfcacheId review feedback * Tie bfcacheId sentinels to shared constants * Address bfcacheId review clarity * Tighten bfcache slot identity check * Deduplicate bfcache history sync check * Handle empty bfcache segment ids * Fix intercepted active slot route ids * Clarify bfcacheId form state coverage * Address bfcacheId review clarity * fix(app-router): skip invisible tree-path segments in bfcache identity getVisibleTreePathSegments only stripped route groups, not parallel @slots or "." default segments. Tree paths contain raw filesystem segments, so a slot preceding visible URL segments over-counted the consumed pathname segments and produced a wrong identity prefix. That re-minted the bfcacheId for layouts/slots that actually persisted across parallel-route navigations (e.g. @modal), diverging from Next.js bfcacheId semantics. Delegate to the canonical isInvisibleSegment helper so the filter stays in sync with the route graph, and add a parallel-slot layout regression test (fails without the fix). Also tighten the hydration-map unit test to actually exercise the "no seeding from minted ids" property instead of asserting a local literal, and correct the SSR bfcache context comment. * fix(app-router): move isInvisibleSegment to browser-safe utils The previous commit imported isInvisibleSegment from app-route-graph.ts into app-browser-state.ts, but app-route-graph.ts imports node:path, node:fs and node:crypto. Pulling it into the browser bundle broke client hydration (app-with-src E2E: instrumentation-client and dev-overlay-recovery timed out waiting for hydration / the navigation runtime). Move the pure isInvisibleSegment helper into the browser-safe routing/utils.ts module (already in the client bundle), re-export it from app-route-graph.ts so Node-side import sites keep working, and have app-browser-state.ts import it from utils.js. The app-route-graph type import is now type-only. * refactor(app-router): address bfcacheId review feedback - Reject history entries with a missing bfcache version in the document-scoped version gate (stop coercing null -> 0) so stale ids from older builds / external pushState can't be restored into a fresh document on the version-0 path. - Parse AppElements metadata once per element map per commit and resolve slot bindings via a precomputed Map (was repeated readMetadata parses plus an O(slots^2) find on every navigation commit). - Read single history-state keys without cloning the whole state object; the write path still clones before mutating. - Dedupe the two identical non-negative-safe-integer validators and share path-segment / catch-all helpers via browser-safe routing/utils. - Move tree-path consumed-segment counting into routing/utils and fix optional-catch-all over-counting. - Drop the unused bfcacheIds param from createHistoryStateWithPreviousNextUrl. * refactor(routing): make catch-all segment predicates module-private isCatchAllSegment/isOptionalCatchAllSegment are only used by countConsumedPathnameSegments in the same module; drop the exports to clear knip's unused-export warning. * test(app-router): lock in bfcache version-gate rejection of missing versions Extract the version-gate decision into a pure, exported isHistoryStateBfcacheVersionCurrent(state, currentVersion) in app-history-state (re-exported via app-browser-state); the browser entry's isCurrentBfcacheVersion now delegates to it. Add a direct regression test asserting a history entry with bfcache ids but no version key is NOT treated as current even when the current version is 0 — the exact cross-document stale-id hole that the earlier `?? 0` coercion left open. --------- Co-authored-by: James <james@eli.cx> |
||
|
Nathan Nguyen
|
560f270ee6 |
fix: align app router navigation parity (#1705)
* fix: align app router navigation parity * fix(app-router): preserve canonical RSC and hard-nav semantics App Router RSC requests now keep the route pathname shape when adding _rsc. Stripping a trailing slash made trailingSlash routes fetch a non-canonical URL and let the server redirect the client-side RSC request back to the slash form. The MPA navigation scheduler also keys pending hard navigations by both href and history mode so a later replace does not reuse an earlier push closure. Redirect handling keeps the global browser bridge out of passive server rendering while retaining client-side redirect recovery. Adds focused regression coverage for trailingSlash RSC requests, same-href push-to-replace hard navigation, and client redirect guard loops. Updates existing navigation observers to the canonical route URL plus _rsc transport. * fix(app-router): stabilize action and prefetch parity * fix(app-router): reset MPA dedupe after bfcache restore A bfcache-restored App Router document can keep the scheduler's last pending hard-navigation marker after the external navigation already left the page. Reissuing the same href and history mode then entered the suspended MPA path without scheduling another location change. Expose a scheduler reset for the browser lifecycle boundary and call it on persisted pageshow events. Cover the scheduler contract so the same external navigation can run again after reset. * fix(app-router): isolate redirect bridge from passive boundaries Unhandled client redirect recovery currently lives inside every RedirectBoundary, while Pages Router params fallback can keep stale App Router params after an App-to-Pages transition. That makes passive boundaries own browser runtime wiring and lets stale app params win over live Pages Router params. Move the unhandled redirect bridge into BrowserRoot under AppRouterContext, keep RedirectBoundary passive, and prefer live Pages Router params when that compat context exists. Add an App-to-Pages navigation regression fixture. * fix: extract shared decodeRedirectError helper, rename stale test - Add decodeRedirectError() to shims/navigation.ts as the single source of truth for parsing redirect error digests - Replace three private helpers in error-boundary.tsx with shared decodeRedirectError() call - Replace inline digest parsing in AppRouterRedirectBridge with shared decodeRedirectError() call (fixes unsafe decodeURIComponent) - Rename test that claimed to test redirect() but actually tests <Link> navigation * refactor: eliminate unnecessary type assertions - isRedirectError: use in-narrowing instead of as-casts - AppRouterRedirectBridge: remove as after guard already narrows - RedirectErrorBoundary: inline digest access, use in for handled check * ci: retrigger checks * fix: emit noindex for Forbidden/Unauthorized boundaries, extract retryScrollTo helper * docs: clarify decodeRedirectError comment about defensive join |
||
|
Nathan Nguyen
|
8ae5abd56c | fix(app-router): preserve forwarded action redirect wrappers (#1582) | ||
|
Nathan Nguyen
|
b1f2e57bc4 |
fix(pages): preserve import.meta.url source paths (#1598)
* fix(pages): preserve import.meta.url source paths
Pages Router production bundles previously left direct import.meta.url expressions for the bundler to evaluate from generated output. Server renders could expose dist/server/entry.js, and prerendered Pages output could miss the client entry needed for hydration to replace the server value with the Turbopack-style browser source URL.
The fix rewrites direct import.meta.url reads in user modules to source-file URLs at transform time, keeps new URL asset bases intact, and loads the Pages client entry from the build manifest for Node production and prerender servers.
Regression coverage ports the Next.js import-meta deploy test, including the prerender and hydrated browser path.
* fix(tests): run import.meta browser coverage in e2e
The import-meta regression test launched Playwright from a Vitest integration shard. That shard does not install Playwright browser binaries in CI, so it failed before exercising the runtime behavior.
Move the browser assertion into the existing pages-router-prod Playwright project, keep the lower-boundary server and client chunk assertions in Vitest, and cache the import-meta plugin root paths after an early source-text bailout to address review feedback.
* refactor(import-meta-url): canonicalize each module once per transform
The transform path canonicalized the same module id twice: once in the
eligibility check and again when computing the replacement value, so every
rewritten module paid for two fs.realpathSync.native() syscalls.
The eligibility check now returns the canonical id and threads it through to
the value computation, leaving a single realpath per module.
Also widens the extension match to .cts/.mts/.cjs and documents that the
new URL() skip only covers the bare URL identifier, matching Vite's own
asset-detection scope.
* test(pages): drop unrelated prerender from top-level-await setup
The top-level-await production test ran runPrerender in its setup, but the
assertions pass identically whether the routes are prerendered or rendered on
demand. The step added unrelated build work to a test that is not about
prerendering or client-entry injection.
* fix(pages): share client entry fallback
* fix(import-meta-url): address PR review feedback
- Handle ChainExpression (import.meta?.url) in new URL() asset-detection
skip so the optional-chain variant preserves Vite asset resolution
- Add explicit ChainExpression check in visit() to avoid fragile
reliance on coincident inner-node start/end ranges
- Add test for new URL('./file', import.meta?.url) preservation
- Fix decodeHtmlText entity ordering (& before ") in both
unit and E2E test helpers
- Add doc comments: excluded prefixes rationale, node_modules
early-exit optimization intent
* fix(pages): select client entry by marker in build manifest
findClientEntryFileFromManifest returned the first chunk flagged
isEntry, but a client build can emit several entry chunks (client
entry plus instrumentation/middleware) and manifest order is not
guaranteed. The on-disk fallback already disambiguates by the
vinext-client-entry / vinext-app-browser-entry marker; apply the
same check to the manifest path so both lookups agree, falling back
to the first entry only when nothing is marked.
* refactor(pages): address review feedback on client-entry/import-meta-url
- prod-server: hoist findClientEntryFile out of the manifest if/else
since it already handles a missing manifest via the on-disk fallback
- import-meta-url: document the two known limitations (destructured
access, aliased new URL() base) that match Vite's own behavior
- import-meta-url: drop the dead visit(value.callee) for new URL() —
the callee is always the bare URL identifier
---------
Co-authored-by: James <james@eli.cx>
|
||
|
Steve Faulkner
|
0c1193fd76 |
fix: include i18n context in Pages ISR cache keys (#1698)
Pages Router ISR cache entries were keyed only by pathname. With i18n domain routing, the same pathname can render different content depending on the matched domain and default locale. A cache entry created for one domain could be reused for another domain. Add an i18n cache variant to the Pages ISR key when i18n is configured: - domain:<domain> for matched domain-locales - locale:<locale> for locale-prefix/default-locale routing Adds a Pages i18n production fixture page with getStaticProps + revalidate and verifies example.fr and example.com maintain separate ISR entries for the same pathname. |
||
|
James Anderson
|
f4687b3537 |
fix(app-router): treat module-only Vite ref errors as action-not-found (#1340) (#1668)
When a server action id resolved from request headers carries the `#<exportName>` suffix (e.g. `/app/foo.ts#bar`), `loadServerAction()` splits the id at `#` and calls `requireModule(file)` with the module path only. `@vitejs/plugin-rsc`'s reference-validation virtual module therefore emits `[vite-rsc] invalid server reference '/app/foo.ts'`, omitting the `#<exportName>` portion. The previous match in `isServerActionNotFoundError` compared the full actionId against the Vite error string, so missing/stale references surfaced as a generic 500 instead of the Next.js-compatible 404 with the `x-nextjs-action-not-found` header. Also accept the pre-`#` module-path form for both the dev-time `[vite-rsc] invalid server reference '<id>'` and prod-time `server reference not found '<id>'` error shapes. Adds a focused unit test reproducing the dev-mode case and integration tests asserting that throwing `notFound()` from a server action returns 404 (both fetch and progressive form paths), porting fixture from Next.js: test/e2e/app-dir/actions/app/server/form.js. |
||
|
James Anderson
|
c2f9f5b53f |
fix(app-router): cache pages with revalidate=Infinity/false (#1487) (#1666)
* fix(app-router): cache pages with revalidate=Infinity/false (#1487) App Router pages exporting `revalidate = Infinity` (or `revalidate = false`, which resolves to Infinity in segment config) were re-rendered on every request: the HTML response policy returned `shouldWriteToCache: false`, and the cache-write policy rejected non-finite revalidate values outright. The downstream Cache-Control header was correct (1y s-maxage), but vinext itself never served identical bytes for repeat requests inside the same process. This change treats `revalidate = Infinity` as "cache indefinitely" rather than "never write to cache": - `resolveAppPageHtmlResponsePolicy` now writes the cached HTML/RSC bundle in production and reports `MISS` on first render, matching the standard ISR write flow. Dev mode continues to emit `STATIC` with no cache write. - `resolveAppPageCacheWritePolicy` accepts `Infinity` as a valid revalidate interval; only `null`, `NaN`, and non-positive values reject the write. The MemoryCacheHandler (used by tests and as the default) already handles Infinity correctly: `revalidateAt = now + Infinity` keeps the entry fresh forever until `revalidatePath`/`revalidateTag` invalidates it. Adds two production-server regression tests: - `export const revalidate = 60` returns HIT with identical timestamp on the second request (covers the standard finite case). - `export const revalidate = Infinity` returns HIT with identical timestamp on the second request (covers the previously broken indefinite case). Refs #1487. Other symptoms listed in the issue (`unstable_cache`, fetch cache directives, force-static, force-no-store, on-demand revalidate) are out of scope here and tracked separately. * test(app-page-response): update Infinity revalidate assertions Production + revalidateSeconds: Infinity now writes to the ISR cache (cacheState: "MISS", shouldWriteToCache: true). Update the existing assertion to match, and add a dev-mode case asserting Infinity stays as a no-write STATIC response in development. |
||
|
James Anderson
|
3ff06784a0 |
fix(app-router): forbidden()/unauthorized() escalate past intermediate layouts (#1547) (#1647)
When a deep page called `forbidden()`/`unauthorized()`/`notFound()` and the
intermediate layout segments had no matching boundary file, vinext's
page-error fast path wrapped the resolved root boundary in *all* of the
route's layouts. That left the intermediate layout's chrome ("Dynamic with
Layout") rendered next to the boundary fallback, diverging from Next.js
where the nearest ancestor boundary owns the entire fallback subtree.
`renderPageSpecialError` now resolves the deepest layout-level boundary
via `resolveAppPageParentHttpAccessBoundary` and slices the layouts to
`[0..boundaryLayoutIndex]`, so layouts beneath the boundary owner stop
rendering. When the route-level boundary file lives next to the page
(not aligned to a layout), all layouts are kept as before.
Ported the Next.js regression cases from
`test/e2e/app-dir/forbidden/basic/forbidden-basic.test.ts` and
`test/e2e/app-dir/unauthorized/basic/unauthorized-basic.test.ts`.
|
||
|
James Anderson
|
2d9f3ffc36 |
fix(app-router): accept icons.other as single descriptor (#1492) (#1667)
Next.js's `metadata.icons.other` accepts either a single
`IconDescriptor` or an array of them, but `MetadataHead` iterated
it directly and threw `TypeError: metadata.icons.other is not iterable`
when given a single object. That crash bubbled up into the page
render so no `<link>` tags (favicon, shortcut, apple, custom) reached
the head at all for any page declaring `icons.other` as an object.
Normalize `metadata.icons.other` to an array before iterating, widen
its type to `OtherIconDescriptor | OtherIconDescriptor[]`, and also
forward the optional `type` attribute so descriptors like
`{ rel: "mask-icon", url, type: "image/svg+xml" }` round-trip
correctly. Ported coverage from
`test/e2e/app-dir/metadata-icons/metadata-icons.test.ts`.
Refs cloudflare/vinext#1492
|
||
|
James Anderson
|
76fe5d8b05 |
fix(pages-router): render custom pages/500 on SSR throw (#1676)
When getServerSideProps (or any other render-time code) threw in the production server entry, vinext collapsed the response into a plain "Internal Server Error" text body. Next.js instead renders the user's custom pages/500.tsx (with pages/_error.tsx as a fallback). Match that behaviour so frameworks that rely on custom error pages — and the Next.js getserversideprops deploy suite — see the expected output. The catch block in `_renderPage` now re-renders through the same entry using `pages/500.tsx` (or the `_error` route) with status 500. A new `__isInternalErrorRender` guard prevents infinite recursion if the error page itself throws; data (`/_next/data/...`) requests still short-circuit to the plain text body since they can't return HTML. Mirrors Next.js base-server.ts `renderToResponseImpl` 500 fallback. The dev server already renders custom 500/error pages via `renderErrorPage()` — this aligns prod with that behaviour and adds explicit regression coverage for both paths via a new gssp-throw fixture. Refs #1458 |
||
|
James Anderson
|
5bcbee60e0 |
fix(app-router): isolate global-not-found chunk to fix CSS cascade (#1549) (#1654)
When `app/global-not-found.tsx` is defined, route-miss 404s render that
module standalone (it ships its own `<html>`/`<body>` and replaces the
root layout). Previously the entry registered global-not-found.tsx as a
static `import * as`, which co-located its CSS with the root layout's CSS
in a single chunk. Lightningcss then dropped overlapping declarations
between the two stylesheets as dead code, so any rule global-not-found
defined that the layout also defined was silently removed — breaking the
CSS cascade for 404s.
Switching the entry to `() => import("…/global-not-found.tsx")` puts the
module in its own JS chunk and lets the SSR pipeline (and `Resources`
metadata) treat its CSS independently of the rest of the app. The
fallback renderer now accepts a `loadGlobalNotFoundModule` loader (cached
per worker) instead of an eagerly-imported module.
Also adds a regression test asserting the SSR markup for `/does-not-exist`
links only global-not-found's stylesheet — never the root layout's — so
the cascade can never resolve to the wrong color.
Ported from Next.js: test/e2e/app-dir/initial-css-order/initial-css-order.test.ts
|
||
|
James Anderson
|
15fbe66c47 |
test(pages-router): regression coverage for edge runtime & OG image API routes (#1338) (#1648)
Adds Pages Router fixtures and dev + production integration tests for the
two scenarios reported as failing in the Next.js Deploy Suite:
- `export const config = { runtime: 'edge' }` API route returns a Web
Response (issue #1338 case 1 — edge API routes were observed as 500).
- `next/og` `ImageResponse` with `runtime: 'edge'` inside a Pages Router
API route returns `image/png` (issue #1338 case 2 — OG routes were
observed as 404).
Ports route handlers from `next.js/test/e2e/edge-pages-support/app/pages/api/hello.js`
and `next.js/test/e2e/og-api/app/pages/api/og.js`. The existing
implementation already wires both code paths correctly via
`handlePagesApiRoute` and the `vinext:og-inline-fetch-assets` plugin; this
PR locks in coverage so a Pages Router-only build cannot silently regress
either path.
Refs #1338
|
||
|
Nathan Nguyen
|
5e90f16a56 |
fix(app-router): preserve push autoscroll across refresh (#1602)
Calling router.push() and router.refresh() in the same transition could leave vinext on the current page instead of committing the pushed route. That diverged from Next.js and broke the default autoscroll target for the pushed page. The refresh path dispatched a current-route RSC refresh before the nested push transition could finish scheduling, so the refresh superseded the intended navigation. Track App Router push/replace scheduling for the current task, keep refresh cache invalidation, and skip only the overtaking current-route refresh. Add Next parity coverage for refresh, server-action refresh, focus targets, fragments, hoisted children, and metadata routes. |
||
|
한만욱
|
07be579048 | test(app-router): cover clean replace after search load (#1626) | ||
|
James Anderson
|
53a97e2002 |
fix(navigation): stabilize Pages Router useParams snapshot for SSR (#1639)
`useParams()` / `useSearchParams()` / `usePathname()` from `next/navigation`
work as cross-router compat shims under Pages Router, deriving their state
from `getPagesNavigationContext()` published by `next/router`. React's
`useSyncExternalStore` requires `getServerSnapshot` to return a stable
reference (Object.is) across calls in a single render — otherwise React
logs:
"The result of getServerSnapshot should be cached to avoid an infinite loop"
and may re-render until the snapshots stabilize, which is wasteful and a
hydration-mismatch hazard. The SSR branch of `getPagesNavigationContext`
was returning a fresh object each call.
Cache the shape on a `WeakMap` keyed on the ALS-isolated SSR context, so
the same `{ pathname, searchParams, params }` shape (with reference-stable
`params` / `searchParams`) is returned for every hook call inside a single
request. The WeakMap key is per-request, so concurrency safety is
preserved.
Adds a Vitest unit assertion (reference equality across repeated SSR
calls) and a Playwright regression matching the Next.js deploy-suite case
(`test/e2e/app-dir/use-params/use-params.test.ts` — 'should work on pages
router'): a Pages Router dynamic page using `useParams()` inside a fixture
that also has an `app/` directory must render the matched param after
hydration.
Refs #1466
|
||
|
James Anderson
|
a20c49ead2 |
fix(middleware): stop merging original query into middleware rewrite target (#1632)
* fix(middleware): stop merging original query into middleware rewrite target
Next.js middleware constructs the rewrite-target URL itself — by mutating
`request.nextUrl` or by passing a fresh URL — and whatever search params
that URL carries IS the final query. vinext was silently re-merging the
original request's query on top, which resurrected keys that middleware
had explicitly removed via `searchParams.delete(...)`.
Tests:
- Add `mw-clear-query-params` Pages Router fixture + dev test ported from
Next.js `test/e2e/middleware-rewrites/test/index.test.ts` ("should
clear query parameters").
- Add prod-server tests for the original-query-preservation paths
(middleware now propagates query via `request.nextUrl.clone()` in the
fixture, mirroring Next.js `app/middleware.js`).
- Add App Router parity test for original-query-preservation.
Closes #1342
* test(middleware): address bonk review nits for #1342
- Fix misleading comment in app-router.test.ts that referenced Object.assign
merge semantics — the PR's premise is that vinext should NOT auto-merge.
- Add prod-server regression test for searchParams.delete on rewrite target,
mirroring the dev coverage so both code paths are exercised explicitly.
|
||
|
James Anderson
|
d458b71e6b |
test(link): port Next.js OnNavigate fixture as Pages Router regression (#1357) (#1628)
* test(link): port Next.js OnNavigate fixture as a Pages Router regression Add a local Playwright fixture mirroring `.nextjs-ref/test/e2e/link-on-navigate-prop/shared/OnNavigate.tsx` so the upstream `onNavigate` parity scenarios are exercised against vinext's own Pages Router runtime. The `onNavigate` prop itself was implemented in #1371 and is covered by unit tests in `tests/link-navigation.test.ts`. The nightly Next.js deploy suite (workflow run 26552059147) still surfaces failures under `test/e2e/link-on-navigate-prop/index.test.ts` for the Pages Router half of the suite, where even a plain `<button onClick={() => setIsLocked(...)}>` does not update state. The App Router half of the same suite passes, suggesting a Pages Router-specific hydration regression rather than a missing `onNavigate` implementation. This commit ports the upstream fixture so the same scenarios run against vinext in CI's existing Playwright suite, narrowing the reproduction without depending on the full Next.js deploy harness. Refs #1357. * test(link): address bot review on OnNavigate Pages Router port - Assert `isClicked` / `isNavigated` flip to `true` in the internal-click test by stubbing `history.pushState` so vinext's Link runs its synthetic handlers in full but the URL never commits and the source DOM stays available for state assertions. Split the navigation check into its own test that asserts `#subpage-heading`. - Port the three remaining upstream scenarios from `.nextjs-ref/test/e2e/link-on-navigate-prop/index.test.ts`: modifier-key click, plain external link (alert-driven), and external link with `replace` (history-length check). Add the corresponding `#external-link` and `#external-link-with-replace` anchors to the fixture to match `shared/OnNavigate.tsx`. - Explain in the download-test why the native `click` listener doesn't interfere with React's synthetic onClick / onNavigate. - Switch `setIsLocked((v) => !v)` to `setIsLocked(!isLocked)` to match upstream's exact callback shape (parity port). Refs #1357. |
||
|
James Anderson
|
a1a448a49f |
fix(route-handler): default Path=/ on returned Set-Cookie when merging mutable cookies (#1484) (#1627)
When a route handler called `cookies().set()` alongside a returned
`Response` that carried raw `Set-Cookie` headers without a `Path`
attribute, vinext appended the raw line verbatim. The resulting comma-
joined `set-cookie` view then read like `bar=bar2, baz=baz2` with no
per-cookie `Path=/`, diverging from Next.js whose `appendMutableCookies`
re-runs every returned cookie through `ResponseCookies.set` (which
normalises a missing `path` to `/`).
`applyMutableCookieFallbacks` now ensures each returned cookie carries
an explicit `Path=` attribute, matching Next.js's behaviour in
`packages/next/src/server/web/spec-extension/adapters/request-cookies.ts`.
Ported the Next.js coverage from
`test/e2e/app-dir/actions/app-action.test.ts` ("should support setting
cookies in route handlers with the correct overrides") as
`nextjs-compat/set-cookies.test.ts`.
Fixes #1484
|
||
|
Nathan Nguyen
|
bdbd1f833d |
fix(app-router): preserve source identity for intercepted renders (#1589)
* fix(app-router): preserve source identity for intercepted renders Intercepted App Router renders used the visible target pathname as the payload route identity. That let a modal navigation such as /feed to /photos/42 replace the retained source tree with target-scoped route and page ids, so refreshes, server actions, chained modals, and history traversal could lose source page state. The violated invariant is that an intercepted render has separate display and matched-route identities. This commits a fenced render identity helper, keeps source route and page ids stable for retained source-route payloads, leaves the target pathname in interception proof, and includes source context in RSC cache variants. Regression coverage asserts source payload keys, cache variants, modal refresh/action/history flows, and source client/search state preservation. * fix(app-router): normalize intercepted rsc cache source * fix(app-router): harden intercepted cache regeneration * refactor(app-router): share intercepted path normalization * fixup! fix(app-router): preserve source identity for intercepted renders fixup! fix(app-router): preserve source identity for intercepted renders |
||
|
Nathan Nguyen
|
8e3438d267 |
fix(app-router): complete default autoscroll page target parity (#1601)
* fix(app-router): complete default autoscroll page target parity App Router page navigations can commit loading shells, final payloads, and superseded payloads out of order. A pending scroll intent that is only keyed by navigation start can be consumed by the wrong committed tree, which makes default page target scrolling and focus timing diverge from Next.js when loading boundaries or rapid navigations are involved. Bind scroll intents to approved render commits, consume only the matching intent from the committed scroll target, and clear hard-navigation failures only for their own intent. Extend the Next.js compat fixture coverage for skipped first children, loading boundary timing, same-page search param changes, focus behavior, horizontal scroll preservation, and stale intent races. * test(app-router): update link navigation scroll intent assertion * fix(app-router): address review — consume scroll intent on no-commit, tighten test - Consume scroll intent before returning no-commit in renderNavigationPayload, preventing the navigateClientSide fallback from scrolling for an abandoned navigation. - Replace waitForTimeout(700) with expect.poll for robust race-condition stability assertion (CI-flake-safe, stronger proof of stability). - Update wrapperless targeting comment to clarify outer function / inner class split. - Add three unit tests covering no-commit cleanup, stale intent rejection, and claim-without-commitId consumption paths. |
||
|
Divanshu Chauhan (divkix)
|
b6ac0c9abd |
test(pages-router): regression test for literal form action=action:foo (#1476) (#1613)
* test(pages-router): regression test for literal form action=action:foo (#1476) Adds a Pages Router fixture page with a literal action:foo string in a form action attribute, plus an integration test asserting the attribute survives SSR serialization intact. This guards against future regressions where React 19 or the RSC plugin might strip the literal string because it resembles a server action ID. Ported from Next.js test/e2e/app-dir/action-in-pages-router. * test: clarify action:foo regression test comment Update the comment to accurately describe that this test verifies React 19 SSR preserves literal string action attributes, not server action invocation. This addresses review feedback on the PR adding the regression test for #1476. |
||
|
Nathan Nguyen
|
314fbea508 |
fix(app-router): align RSC navigation scroll targeting with Next.js (#1599)
* fix(app-router): align RSC navigation scroll targeting with Next.js App Router RSC navigations scrolled the document after appNavigate resolved, which lost Next.js segment targeting and focus semantics. That left persistent layout offsets and horizontal scroll behaviour coarser than Next.js during normal navigation. Next.js treats scroll as a navigation intent consumed by the newly navigated segment after commit. This adds a small intent state, wraps the page slot in a client scroll target, and keeps the existing hash-only and popstate paths separate. Adds focused Playwright coverage ported from Next.js autoscroll and navigation focus tests. * test(form): stop asserting app router scroll detail * fix(app-router): prevent focus from changing scroll * fix(app-router): share scroll intent across Vite module instances The App Router scroll intent was stored in plain module-level variables in app-router-scroll-state.ts. navigateClientSide stages the intent and the committed AppRouterScrollTarget consumes it, but next/navigation and the scroll-target module can resolve to separate Vite module instances. If the writer and consumer hold different copies, the staged intent is invisible to the consumer and the post-commit scroll/focus silently no-ops. This is the same multi-instance hazard the rest of the navigation state already guards against with Symbol.for globals (see _CLIENT_NAV_STATE_KEY in navigation.ts and AGENTS.md "RSC and SSR Are Separate Vite Environments"). Move the single pending intent and the id counter onto a Symbol.for global so every module instance shares one slot, matching the established navigation state pattern. Function signatures are unchanged, so callers need no edits. Also document why AppRouterScrollTarget must remain a class component: findDOMNode needs a mounted class instance to locate the first rendered DOM node without a wrapper element, mirroring Next's default scroll handler. |
||
|
Nathan Nguyen
|
0046e0691a |
fix(app-router): preserve encoded prerender params (#1597)
* fix(app-router): preserve encoded prerender params App Router prerendered pages with encoded dynamic params could be bypassed in production and rendered again with decoded route params. This diverged from Next.js for generateStaticParams values like "sticks & stones". The root cause was split across build and startup. The prerender render path only had the encoded URL, so normal route matching decoded the page params. After preserving encoded params for the render, production startup still seeded the ISR cache under the encoded artifact path while live requests lookup cache entries by the runtime-normalized pathname. Carry trusted prerender params through the prerender-only request path, keep decoded params for dynamicParams validation, and seed prerendered App Router artifacts under the normalized runtime cache key while reading files from their encoded output path. * chore(app-router): clean up prerender header internals Reuse the shared record guard for prerender route param parsing and remove the local duplicate. Keep Next.js INTERNAL_HEADERS as the exact upstream list, with vinext-only internal headers tracked separately but stripped by the same request pipeline. This fixes the CI unit failure while preserving the prerender header trust boundary. Factor repeated Node-header conversion and forwarded-protocol handling in the production server to avoid local assertions and duplicate loops. * fix(app-router): scope prerender params to route pattern Bind the internal prerender route-param header to the route pattern that produced it so encoded render params cannot cross a rewrite boundary into a different dynamic route shape. The App Router handler now only applies encoded prerender params when the payload routePattern matches the final matched route. Otherwise it falls back to the decoded params from normal route matching. Tighten the header parser to validate the payload shape with Object.entries/Object.keys and add a regression for a prerendered source route rewritten to a dynamic target route with a different param name. * fix(app-router): verify prerender params match route Require trusted prerender route params to prove both the final route pattern and the final decoded matched params before using their encoded render values. This prevents same-pattern rewrites from carrying stale prerender params into a different concrete route match, while keeping valid prerender renders encoded and static validation decoded. Add handler regression coverage for same-pattern rewrites and helper coverage for catch-all array comparison. * docs(app-router): clarify prerender route-param trust boundary The prerender route-param readers and serializer carried implicit coupling that read as security verification but actually delegated it. A reader new to readTrustedPrerenderRouteParams could assume it verifies the prerender secret, when verification happens once upstream at prod-server's nodeToWebRequest boundary and every downstream read trusts that. The double read-then-reattach in createAppRscHandler also depends on the secret header surviving filterInternalHeaders, with no note of that coupling. Document the trust contract: secret is verified at the outer boundary, downstream reads operate on already-trusted requests gated by VINEXT_PRERENDER=1, and the secret header must stay out of the internal strip list for the second read to succeed. Also document the empty-params to null serialization contract. No behavior change; comments only. |