mirror of
https://github.com/cloudflare/vinext.git
synced 2026-09-14 19:04:59 +08:00
vinext@0.1.8
4 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 James Anderson
|
f5355964aa |
fix(pages-router): set x-nextjs-deployment-id on SSG _next/data responses (#1863)
* fix(pages-router): set x-nextjs-deployment-id on SSG _next/data responses Pages Router `/_next/data/<buildId>/<page>.json` responses were missing the `x-nextjs-deployment-id` response header that Next.js sets on these routes when a deployment ID is configured. The client router reads this header to detect when a new deployment has been rolled out and triggers a hard navigation for deployment-skew protection. Root cause: the `isDataReq` short-circuit path in `pages-page-handler.ts` that builds the JSON envelope (`buildNextDataJsonResponse`) never injected the header. Next.js's `pages-handler.ts` sets `NEXT_NAV_DEPLOYMENT_ID_HEADER` on every `isNextDataRequest` response (success, redirect, and notFound cases). Fix: read `process.env.__VINEXT_DEPLOYMENT_ID || process.env.NEXT_DEPLOYMENT_ID` (the runtime-available deployment ID, inlined at build time by Vite's define transform) and set `x-nextjs-deployment-id` on the `_next/data` response headers when a deployment ID is configured. Maps to test: `pages ssg data deployment skew - hard navigate when a new deployment occurs > header with deployment id > header is set on data routes`. Fixes #1829 * fix(pages-router): complete x-nextjs-deployment-id parity for _next/data (bonk #1829) Address all three ask-bonk findings on PR #1863: 1. Incomplete parity — add x-nextjs-deployment-id to redirect and notFound _next/data exits in pages-page-data.ts (buildPagesRedirectResponse, buildPagesDataNotFoundResponse). Thread deploymentId through ResolvePagesPageDataOptions and pass it from pages-page-handler.ts, matching the success-path pattern already in that file. 2. Dev/prod parity — patch dev-server.ts to set x-nextjs-deployment-id on all three _next/data exits: the writeGsspRedirect isDataReq path, the getStaticPaths/ getServerSideProps/getStaticProps notFound isDataReq paths, and the success JSON envelope short-circuit (isDataReq block). Mirrors the prod handler. 3. Add regression tests asserting x-nextjs-deployment-id is present on _next/data success / redirect / notFound responses in both pages-page-data.test.ts (unit, via deploymentId option) and pages-page-handler.test.ts (integration, via process.env.__VINEXT_DEPLOYMENT_ID), and absent when no env var is set. * fix(pages-router): add x-nextjs-deployment-id to wrong-buildId/route-miss _next/data 404s (#1829) Fixes the deployment-skew gap identified in review: `buildNextDataNotFoundResponse` now includes the `x-nextjs-deployment-id` header so all three wrong-buildId/route-miss exits (normalizePagesDataRequest, prod-server Node path, pages-page-handler route-miss) surface the deployment ID to the client router. Adds unit tests for the new header on all relevant exits. * fix(pages-router): set deployment-id header on remaining two dev-mode _next/data 404 exits (#1829) Patches the stale-buildId 404 in the vite-dev middleware (index.ts) and the route-miss data 404 in createSSRHandler (dev-server.ts) to set x-nextjs-deployment-id, matching prod parity via buildNextDataNotFoundResponse(). * test(pages-router): cover dev-mode _next/data deployment-id header exits Drive the two dev-only exits flagged in review through the real fixture dev server: the wrong-buildId data 404 in the plugin middleware (index.ts) and createSSRHandler's route-miss data 404 (dev-server.ts), plus the dev success path and the no-deployment-id case. * fix(pages-router): skip deployment-id header on /_error and /500 data success responses Next.js pages-handler.ts guards the success-path x-nextjs-deployment-id injection with `!isErrorPage && !is500Page`. Mirror that exclusion in both the prod success short-circuit (pages-page-handler.ts) and the dev one (dev-server.ts), and lock it in with a prod unit test (/_error and /500) plus a dev integration test (/500). |
||
|
Nathan Nguyen
|
08504a7745 |
fix(app-router): preload next/dynamic chunks with CSP nonce (#1594)
* fix(app-router): preload next/dynamic chunks with CSP nonce
Rendered next/dynamic boundaries produced no SSR preload links for their client chunks. That diverged from Next.js and broke nonce-based CSP tests that inspect preload tags before hydration.
The runtime assumed React.lazy alone was enough for dynamic imports. It never carried the dynamic module IDs from the source call into SSR, and client JS chunk URLs did not use Next.js's static/chunks path shape.
Add a focused dynamic metadata transform, resolve boundary files from the client build manifest, emit nonce-bearing preload hints during SSR, and keep client JS assets under _next/static/chunks. Ported regression coverage verifies the nonce and the emitted path shape.
* fix: honor assetPrefix for dynamic preload globals
* test: cover asset-prefixed dynamic preload nonces
App Router production only covered the default dynamic preload URL path. That left assetPrefix regressions at the manifest-to-runtime emission boundary unguarded.
Add a startProdServer regression that builds app-basic with assetPrefix "/cdn" and asserts nonce-bearing next/dynamic chunk preloads render under /cdn/_next/static/chunks/.
* test: isolate asset-prefixed prod server globals
The assetPrefix regression starts a second production server in the same Vitest process. startProdServer and the generated RSC runtime install global manifest and module-loader state, so leaving the temporary build's globals in place breaks later production requests in the shared server.
Snapshot and restore the relevant globals around the temporary /cdn server while keeping the integration assertion at the real runtime boundary.
* refactor: reuse shared record guard in dynamic preload metadata
The dynamic preload metadata transform carried a local object guard even though the codebase already has a shared record predicate. That made the AST helper drift from the existing non-array record invariant used elsewhere.
Reuse isUnknownRecord from utils/record and keep the plugin-specific AST alias local to the transform.
* fix: make dynamic preload metadata binding-aware
The next/dynamic metadata transform matched calls by imported identifier text. Shadowed parameters and block bindings with the same name were therefore mutated even when they no longer referred to the next/dynamic import.
Walk the parsed module with lexical binding state so shadowed names are excluded, and collect object-form imports only from loader/modules rather than every property on the options object.
* fix: model switch and class scopes in dynamic preload transform
The dynamic preload metadata transform still treated imported next/dynamic names as active inside switch case lexical scopes and named class expression bodies. That could inject loadableGenerated metadata into calls that resolve to local bindings rather than the import.
The scope walker now removes switch case-block bindings while traversing case tests and consequents, leaves the switch discriminant in the outer scope, and scopes named class declarations and expressions through their children. Regression tests cover switch case shadowing and named class expressions.
* fix: address PR review feedback for next/dynamic CSP nonce
- Drop `as="style"` from `<link rel="stylesheet">` — `as` is a
preload attribute per the HTML spec, semantically incorrect on
stylesheet links. Browsers ignore it but it produces bad HTML.
- Remove brittle `import(` string gate from the transform — it could
miss `import ("./x")` (whitespace between import and paren). The
Vite filter already gates on "next/dynamic" and the AST parse is the
real validation.
- Add comment explaining MagicString mutation safety in Promise.all.
- Add comment explaining AST end-exclusive assumption on closeParen.
- Add test for whitespace-separated dynamic import syntax.
* fix: update global.d.ts comment to mention asset prefix, fix formatting
* refactor: extract computeClientRuntimeMetadata shared helper
Extract the duplicated client-manifest runtime metadata computation
(lazy chunks, dynamic preloads, client entry) into a single shared
helper in utils/client-runtime-metadata.ts.
Both prod-server.ts (Pages Router Node.js production server) and
index.ts (Cloudflare closeBundle hook) now call the same helper,
eliminating the highest-risk seam in the PR where basePath/
assetPrefix handling produced bugs.
* test: add TSX-generic and object-form metadata regression tests
- Add post-type-strip generic call test verifying multi-line
dynamic() calls work after TS/JSX stripping by esbuild.
- Add object-form existing loadableGenerated preservation test
to guard against duplicate injection in the object overload.
* chore: remove unused ClientRuntimeMetadata type export
* revert: discard unrelated lockfile change from previous commit
* test: replace deploy test simulators with computeClientRuntimeMetadata
simulateCloseBundleAppRouter and simulateCloseBundlePagesRouter
no longer hand-roll lazy chunk and dynamic preload computation.
They call the shared computeClientRuntimeMetadata helper directly,
matching production wiring.
Also:
- Update helper doc comment to mention Node server startup
- Remove unused imports (manifestFileWithAssetPrefix,
manifestFileWithBase, computeDynamicImportPreloads,
dynamicImportPreloadsWithBase) from deploy.test.ts
* test: add absolute assetPrefix CSP nonce tests and clean up redundant globals
* fix(app-router): keep CSP nonce on next/dynamic preloads from Server Component call sites
DynamicPreloadChunks rendered in the environment of the dynamic() call site. For a Server Component call site it ran in the RSC environment, where the script-nonce React context is unavailable, so the emitted dynamic preload <link>s dropped the request nonce — a CSP violation under `script-src 'nonce-…' 'strict-dynamic'`. The existing tests missed this because every fixture called dynamic() from "use client" modules, which render in the SSR pass where the nonce is present.
Extract DynamicPreloadChunks into its own "use client" module (mirroring Next.js's PreloadChunks and vinext's next/script shim) so it always renders in the SSR pass where withScriptNonce() installs the provider, regardless of whether dynamic() is called from a Server or Client Component.
Add fixture regression tests: server-call-site JS and CSS preloads carry the nonce, an ssr:false boundary emits no server preload (Next.js loadable.tsx parity), and preloads are still emitted without a nonce when no CSP is set.
* test(app-router): add browser CSP-nonce e2e for Server Component next/dynamic call site
The existing csp.spec.ts browser test only exercises the client call-site page (/nextjs-compat/dynamic). Add a Playwright test for the Server Component call site (/nextjs-compat/dynamic/rsc-imports-client) asserting the lazily-loaded client widget hydrates under script-src 'nonce-…' 'strict-dynamic' with no CSP violations in the console.
* fix(app-router): correct next/dynamic preload key-spaces and harden the transform
Addresses review findings on the next/dynamic CSP-nonce feature.
- Lazy chunks must stay in the SSR-manifest key-space (basePath only); only dynamic preloads take the assetPrefix. Applying an absolute-URL assetPrefix to lazy chunks broke the Pages Router modulepreload-exclusion membership test and leaked lazy chunks into <link rel=modulepreload>.
- Resolve symlinks on both sides when computing a dynamic boundary's module ID (reusing relativeWithinRoot/tryRealpathSync), so the key matches the client manifest under pnpm/Cloudflare symlinked layouts instead of silently dropping the preload.
- Throw on dynamic() calls with >2 args (Next.js react-loadable parity).
- Insert the generated options arg right after the first arg, preserving comments/whitespace (the previous substring-comma scan ate commas inside comments).
- Recurse into _next/static/chunks/ in the on-disk client-entry fallback (client JS now lands under chunks/).
- Add typeof-window guard to DynamicPreloadChunks and document the deliberate as="style" omission for stylesheet links.
- Extract buildRuntimeGlobalsScript so the Cloudflare closeBundle injection shares one source of truth with the deploy tests (which now mirror includeClientEntry + the mixed app+pages branch).
Tests: lazy/dynamic key-space split incl. absolute-assetPrefix leak regression and a realistic already-prefixed manifest; mixed app+pages client-entry injection; symlinked module-id resolution; nested dynamic() calls; >2-arg throw; comment-safe insertion; ssr:false positive content assertion; honest dev-mode E2E naming (preload-nonce coverage lives in the prod-server Vitest suite).
* fix(app-router): fix parenthesized next/dynamic loader corruption + harden re-review findings
Follow-up to the re-review of
|
||
|
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> |
||
|
Divanshu Chauhan (divkix)
|
d9a5e63c32 |
refactor(pages): extract render orchestrator from codegen string (#1784) (#1805)
* refactor(pages): extract render orchestrator from codegen string (#1784) Move `_renderPage` (~260 lines) and `collectAssetTags` (~85 lines) out of the generated template string in `entries/pages-server-entry.ts` into typed, unit-testable server modules: - `server/pages-page-handler.ts` — `createPagesPageHandler` factory that owns the full Pages Router render lifecycle (i18n, route matching, 404/500 fallback recursion, _next/data envelope, 405 method check, ISR cache-key derivation, SSR-context setup). Mirrors how `server/app-rsc-handler.ts` is structured relative to `entries/app-rsc-entry.ts`. - `server/pages-asset-tags.ts` — `collectAssetTags` + manifest helpers, extracted from the template so they can be unit-tested directly. - `server/pages-data-route.ts` — adds `normalizePagesDataRequest` so both `renderPage` and `runMiddleware` share one implementation instead of duplicating the buildId-check logic in the template string. `entries/pages-server-entry.ts` is now thin wiring: route-table codegen, embedded config/i18n/buildId, and closures that delegate to the handler. `renderPage` keeps its exact exported signature; all callers are unaffected. No behavior change — pure code motion. Fixed a latent bug where the `isrCacheKey` wrapper in the handler was missing the `buildId` argument on non-i18n paths (causing a cache-key mismatch); caught by the existing ISR CSP-nonce test. * refactor(pages): tighten handler types and improve tests - Use ComponentType from react for AppComponent, DocumentComponent, and PageComponent closures instead of unknown — matches pages-page-response.ts which already uses ComponentType. Eliminates the DocumentComponent cast. - Widen isrCacheKey router param from literal union to string — the literal union forced as-casts everywhere it was used indirectly. All call sites pass 'pages' or 'app' string literals so the widening is safe. - Replace the shallow i18n redirect test (which just checked a route miss gave 404) with a test that actually exercises the 307 short-circuit: visiting / with Accept-Language: fr triggers resolvePagesI18nRequest to return redirectUrl and the handler returns 307 to /fr. - Add second i18n test: locale prefix already present → no redirect. - Tighten 'allows POST with gSSP' test: assert no Allow header (confirms the 405 guard was bypassed, not just that some non-405 came back). |