mirror of
https://github.com/cloudflare/vinext.git
synced 2026-09-14 19:04:59 +08:00
vinext@0.2.0
1503 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 James Anderson
|
558ee4121e |
fix(app-router): scroll to top with hoisted children and loading.js (#1368) (#1857)
* fix(app-router): scroll to top with hoisted children and loading.js (#1368) Move the committed scroll/focus target above the route loading Suspense and handle route content whose first DOM node is hoisted into <head>, so default-navigation autoscroll runs as a committed route effect rather than relying on a post-navigation scrollTo fallback. - loading.js: the scroll target stays mounted while the loading fallback renders (matching Next.js placing ScrollAndMaybeFocusHandler above the LoadingBoundary), so scroll fires against the loading boundary DOM. - hoisted children: when findDOMNode resolves an element hoisted into <head> (e.g. <style precedence>), scroll the document to the top as the committed-effect equivalent of Next.js deferring to a higher layout-router. Finishes the remaining scroll-to-top cases of #1368 on top of the autoscroll parity landed in #1599-1602. * docs(app-router): clarify prefetch shell omits scroll target (#1368) |
||
|
James Anderson
|
0c7ff57710 |
fix(build): exclude Vite build manifests from Cloudflare asset uploads (#1850)
On Cloudflare Workers the ASSETS binding serves any uploaded file whose path matches the request BEFORE the Worker runs. Cloudflare builds enable `build.manifest` (and `ssrManifest` for Pages Router) so the worker entry can compute lazy chunks, which writes Vite's manifests to `dist/client/.vite/`. Because nothing excluded that directory from the asset bundle, `/.vite/manifest.json` and `/.vite/ssr-manifest.json` were publicly fetchable, leaking the full source-file -> chunk mapping (including the paths of routes that are never linked from the UI). The Node production server already blocks `/.vite/` explicitly (server/static-file-cache.ts); this adds the Cloudflare-side equivalent by writing a `.assetsignore` into `dist/client` during the cloudflare-build closeBundle hook (right after the existing `_headers` generation, after the manifests have been read). wrangler matches `.assetsignore` with gitignore semantics, so the bare `.vite` entry excludes the directory and its contents from upload, and the file is never served. The merge logic preserves any user-authored `.assetsignore` and only appends missing patterns, so it is idempotent across rebuilds. The writer lives in a small typed helper (build/assets-ignore.ts) with focused unit tests; a real Cloudflare build test asserts the wiring emits `.assetsignore` with `.vite` while the manifest still exists on disk. |
||
|
James Anderson
|
cfc0f8146d |
fix(app-router): resolve explicit parallel slot with no page (#1535) (#1852)
* fix(app-router): resolve explicit parallel slot with no page (#1535) When a parallel slot defines an explicit sub-page (e.g. `@slot/baz/page.tsx`) but no corresponding children page (`baz/page.tsx`) and no `default.tsx` for the children slot, vinext minted a synthetic `/baz` route with a null children page. That route shadowed the sibling catch-all and rendered an empty children prop, so the request hung (the upstream "explicit slot but no page" case timed out). Next.js serves such a slot-only sub-route's children from the nearest sibling catch-all. Add `findCatchAllPage` and use it as the children fallback in `discoverSlotSubRoutes` when no `default.tsx` exists, so `/baz` renders `[...catchAll]/page.tsx` for children while `@slot/baz/page.tsx` fills the slot. A children `default.tsx` still wins over the catch-all when both exist. Finishes the explicit-slot-but-no-page case of #1535 (the children-slot priority case landed in #1659). Fixes #1535. * test(app-router): document catch-all children params limitation (#1535) Address ask-bonk review: the synthetic slot-only sub-route (e.g. /baz) has a static URL pattern, so a catch-all children page reading params.catchAll gets empty params (Next.js passes ["baz"]). Document the divergence inline in discoverSlotSubRoutes and add a graph-level test locking in the current static shape so a future param-population fix must update it. |
||
|
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
|
9b24683503 | fix(build): don't crash on benign floating asset-import errors (#1510) (#1846) | ||
|
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
|
89a12bb203 | fix(build): don't path-resolve bare specifier aliases for esmExternals (#1507) (#1843) | ||
|
James Anderson
|
80acc0bbf0 |
fix(app-router): clear useLinkStatus pending after interrupted navigation (#1527) (#1839)
* fix(app-router): clear useLinkStatus pending after interrupting navigation (#1527) * fix(app-router): suppress notify on Safari history replace fallback The Safari coalescing fallback in syncCurrentHistoryStatePreviousNextUrl called the patched window.history.replaceState, which fires notifyLinkNavigationStart + commitClientNavigationState. On a URL-unchanged metadata sync (refresh/traversal commit) that could clear an unrelated <Link>'s useLinkStatus() pending state a few ms early on Safari. Route the retry through replaceHistoryStateWithoutNotify so the browser sees the same native replaceState without the patched-path side effects (#1527). |
||
|
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
|
aea90aca05 |
fix(build): don't crash build when a PostCSS string plugin can't be resolved (#1509) (#1840)
* fix(build): don't crash build when a PostCSS string plugin can't be resolved (#1509) resolvePostcssStringPlugins runs in the Vite config hook for every environment (RSC/SSR/client). When a named PostCSS plugin failed to resolve from the project root (e.g. a CSS-modules + PostCSS app whose config lists a plugin that doesn't resolve from where vinext looks), the thrown error was uncaught and aborted the whole build with an opaque failure. Wrap plugin resolution in try/catch and fall back to undefined so Vite's own postcss-load-config resolves the config relative to the config file instead of crashing. Closes #1509 * docs(postcss): clarify whole-config fallback on resolution failure (#1509) |
||
|
James Anderson
|
c32cecdca8 |
fix(app-router): preserve _rsc query across redirects (#1529) (#1838)
* fix(app-router): preserve _rsc query across config redirects (#1529) * refactor(prod-server): reuse rawQs for redirect query; clarify .rsc test comment Address ask-bonk review on #1838: - prod-server.ts: reuse the existing rawQs instead of re-slicing url for the redirect Location query (identical value, removes redundancy). - app-rsc-handler.test.ts: tighten the .rsc-without-header test comment so it no longer overstates RSC re-detection — the guarantee is query preservation (mirroring Next.js), not re-arming RSC on the followed hop. |
||
|
James Anderson
|
a843ea2cfa |
fix(actions): enforce serverActions.bodySizeLimit on fetch actions (#1828) (#1834)
* fix(actions): enforce serverActions.bodySizeLimit on fetch actions (#1828) Oversized fetch (client-invoked) server-action requests were rejected with a bare 413 Payload Too Large response, which bypasses the client error boundary and surfaces the wrong status/content-type. Next.js instead throws the body-exceeded error before the action runs and, for fetch actions, emits a 500 Flight response carrying the rejected action result so the nearest error boundary catches it. Mirror that: when an RSC fetch action body exceeds the configured limit (content-length pre-check or stream read), render a Flight stream with returnValue { ok: false } and no page root, returning status 500. The no-JS progressive form path keeps its 413 (no client boundary to catch it). Closes #1828 * fix(actions): use verbatim bodySizeLimit string in body-exceeded error (#1828) Address bonk review: the body-exceeded error message must contain the verbatim serverActions.bodySizeLimit config string (e.g. "2mb") to match Next.js byte-for-byte — the upstream suites assert the log message literally ('Body exceeded 2mb limit'). The previous reconstruction from the parsed byte count produced '2 MB', diverging on casing and spacing. Thread serverActionsBodySizeLimitLabel (the verbatim config value, defaulting to "1mb") through next-config -> index -> app-rsc-entry -> the RSC action handler options, and use it directly in createBodyExceededError. Removes the lossy formatBodySizeLimit reconstructor. Tests assert the verbatim string. * fix(actions): default bodySizeLimit label to Next.js' '1 MB' literal (#1828) Address bonk re-review nit: when serverActions.bodySizeLimit is unset, Next.js emits 'Body exceeded 1 MB limit.' (its defaultBodySizeLimit = '1 MB' literal). Default serverActionsBodySizeLimitLabel to '1 MB' (was '1mb') so the unconfigured-case message matches upstream byte-for-byte too. |
||
|
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> |
||
|
James Anderson
|
6fa25f49ce |
docs(changelog): backfill vinext release notes from GitHub releases (#1822)
* docs(changelog): backfill vinext release notes from GitHub releases Backfill packages/vinext/CHANGELOG.md with entries for every published GitHub release from v0.0.27 through v0.0.55. Previously the changelog only contained the 0.1.0 entry (the first changeset-driven release); all prior release notes lived only on GitHub Releases. Content is taken verbatim from each GitHub Release body, with headings normalized to fit the changelog (drop the redundant 'What's Changed' h2, demote 'Contributors' to h3) and bullets normalized to '-'. * docs(changelog): use h4 subgroups in 0.0.54 for consistency The 0.0.54 entry used bold (**App Router**) pseudo-headings under Bug Fixes, while every other release uses #### h4 sub-headings. Convert them to h4 to match the rest of the changelog. |
||
|
github-actions[bot]
|
2b2ed00f22 |
chore: version packages (#1759)
* chore: version packages * add header --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: James <james@eli.cx>@vinext/cloudflare@0.1.0 vinext@0.1.0 |
||
|
James Anderson
|
a74f833a10 |
feat(deploy): honor Worker-entry cache setters for ISR deploys (#1821)
Older apps that wired a cache backend imperatively in their Worker entry (setCacheHandler / setDataCacheHandler / setCdnCacheAdapter) were blocked on deploy because the ISR cache-adapter check only inspected the Vite config. Add workerEntryHasCacheHandler() so those backwards-compatible entries are still considered configured. Also drop the cdn adapter suggestion from formatMissingCacheAdapterError; the message now points only to the kvDataAdapter() data cache. |
||
|
James Anderson
|
6f689c7b14 | chore: rebuild lockfile (#1815) | ||
|
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
|
1c1744e0ea |
feat(ppr): add PPR fallback-shell render lifecycle tests (#1715)
* feat(ppr): add fallback shell render lifecycle * fix(ppr): address review feedback on PR #1715 - Make await params resolvable during fallback-shell prerendering by keeping the base promise as Promise.resolve(plain) and only throwing the hanging promise on sync fallback-key access. - Defer createPprFallbackShellSuspensePromise to first fallback-key access so hasDynamicBoundary and cache-task ignoring only fire when a fallback param is actually read, matching the suspend only on access contract. - Store and clear pendingAbortCleanup in preparePprFallbackShellFinalRender to avoid leaking uncancellable abort timers. - Add tests for await params resolving known params and for lazy dynamic-boundary marking when only non-fallback params are accessed. * fix(ppr): make awaited params preserve fallback-key suspension and getOwnPropertyDescriptor fully lazy - Promise now resolves to a Proxy (resolvedParams) instead of raw plain. The inner Proxy throws a hanging promise when a fallback param is accessed after await, so `const resolved = await params; resolved.slug` suspends. - Both outer and inner Proxy getOwnPropertyDescriptor traps return a getter that creates the hanging promise lazily. Object.keys(params) no longer triggers side effects merely by enumerating keys. - Added tests for awaited-params suspension and for enumeration not marking hasDynamicBoundary eagerly. * fix(ppr): restructure thenable-params with typed Proxy handlers and conditional observer semantics - Extract createResolvedParamsProxy with ProxyHandler<T> (no cast needed) - Extract createThenableParamsProxy with single as ThenableParams<T> boundary cast - Remove all as unknown as double casts - In fallback-shell mode, Promise continuations no longer observe all keys; let the resolved params proxy report actual property access lazily - Fix getOwnPropertyDescriptor to return throwing getter instead of throwing synchronously, preventing Object.keys from triggering suspension - Restructure awaited-params test to wrap entire async flow in ALS scope - Add test: awaiting params during fallback-shell observes only accessed known params * test(ppr): assert hasDynamicBoundary stays false after known-param access in fallback-shell observer test * fix(ppr): suspend escaped fallback params and settle warmup cache-ready without in-scope task Addresses bonk review on #1715: - Capture the originating fallback-shell state at params construction and create the suspense promise against it (via the new createPprFallbackShellSuspensePromiseForState), instead of re-reading the ALS store lazily. A params object that escapes the shell scope now still suspends on fallback-key access rather than leaking the raw `[slug]` placeholder, and the `get` / `getOwnPropertyDescriptor` traps no longer diverge on the out-of-scope path. - During warmup, always call scheduleCacheReadyIfSettled when a dynamic boundary is hit (not only in the final phase), so a bare headers()/cookies() access with an empty cache-task stack still re-schedules the warmup cache-ready settle. The call is a no-op while cache tasks are pending. Adds focused tests for both paths. * fix(ppr): keep cache-task counter and fallback suspension correct across phase transition Addresses follow-up bonk review on #1715: - Add a per-state cache epoch, bumped on the warmup->final transition. Cache tasks capture the epoch at creation; a task that settles in a later epoch no longer decrements the freshly-reset pendingCacheTasks counter below zero (which would permanently block resolveCacheReadyIfSettled). - Re-derive the memoized fallback-shell hanging promise when the live AbortController changes (preparePprFallbackShellFinalRender swaps it on the phase transition), so suspension always tracks the controller the lifecycle will actually abort instead of a stale warmup signal. Adds tests for the late warmup-task settle and the post-transition re-derive. --------- Co-authored-by: James Anderson <james@eli.cx> |
||
|
Nathan Nguyen
|
7d280edfbe |
refactor(server): replace prerender HTML buffering with metadataReady contract (#1809)
* refactor(server): replace prerender HTML buffering with metadataReady contract * fix(server): wait for allReady before piping through tick-buffered transform when waitForAllReady is true * refactor(server): clean up AppSsrRenderResult normalization and metadataReady promise resolution * test(server): cover ISR revalidation waitForAllReady; address review nits - Add focused test asserting stale HTML cache regeneration passes waitForAllReady: true to handleSsr so Suspense fallbacks never leak into the cache entry - Document why metadataReady resolves eagerly (allReady is already awaited for the prerender path) in app-ssr-entry.ts - Remove stray blank line left by the readStreamAsText import removal - Type capturedOnError in the prerender special-error test instead of any * fix(pages): export PagesPageModule type consumed by pages-page-handler The dts bundle build (vp pack) failed because pages-page-handler.ts imports PagesPageModule from pages-page-data.js, but the type was not exported, so rolldown-plugin-dts could not resolve it. --------- 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). |
||
|
Nathan Nguyen
|
340da2dd19 |
refactor(server): extract pages fallback bridge into typed helper (#1808)
* refactor(server): extract pages fallback bridge into typed helper * test(app-router): update pages fallback codegen assertion * refactor(server): tighten buildRequestHeaders type to Headers | null The injected buildRequestHeaders dependency wires in buildRequestHeadersFromMiddlewareResponse, which returns Headers | null, but the dependency type declared a non-nullable Headers return. Tighten the type to match reality and add a test exercising the null path where the original request is forwarded unchanged. --------- Co-authored-by: James <james@eli.cx> |
||
|
James Anderson
|
465e6c76c1 |
fix(build): share one RSC compatibility ID across all plugin instances (#1814)
Follow-up to #1810. createRscCompatibilityId() falls back to a fresh randomUUID() per plugin instance when no deploymentId is pinned, so a hybrid app+pages build baked two different RSC-compat tokens — the App Router buildApp() instance and the separate Pages Router vite.build() instance each minted their own. The token gates client-side RSC payload acceptance (the X-Vinext-RSC-Compatibility-Id header), so cross-instance divergence is a latent deploy-skew hazard. Coordinate it the same way as the build ID: relocate createRscCompatibilityId to config/next-config.ts (single source of truth, beside resolveBuildId / resolveDeploymentId), have the CLI resolve it once and publish it via __VINEXT_SHARED_RSC_COMPATIBILITY_ID, and have the plugin always adopt it when set. Reuses deploymentId when configured (already stable). The env var is only ever set by the build CLI, so dev and standalone resolution are unchanged. Adds a coordination test asserting the shared token lands in both the server and client output. |
||
|
Nathan Nguyen
|
9bab92f012 |
test(deploy): cover generated worker package exports (#1807)
Generated worker entries can import vinext subpaths that must resolve from the published package. Missing package exports break consumers even when the generated template itself looks correct. Add deploy coverage that extracts vinext subpath imports from generated App and Pages worker entries and verifies each one is covered by packages/vinext/package.json exports. Tighten the prefer-shared-utils lint rule so copied shared helpers in repo tests are reported too. Test files mask template literal fixture payloads, while source files still scan generated-source templates. |
||
|
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 |
||
|
James Anderson
|
e7757e48b7 | ci(bigbonk): reduce job timeout to 10 minutes (#1813) | ||
|
Nathan Nguyen
|
9a721ca287 |
chore(deps): bump knip and node types (#1806)
* chore(deps): bump knip and node types Bump Knip to 6.16.0 and @types/node to 25.9.2. Knip 6.16 reports previously unreported unused exported types, stale ignore entries, and external CLI binaries. Remove unnecessary type exports, refine the script entry coverage, and keep only narrow binary ignores for system or user-project commands. Validation: vp run knip; vp run check. * chore: retrigger ci |
||
|
James Anderson
|
cc246f5db6 |
fix(build): write BUILD_ID via writeBundle so App Router builds emit it (#1810)
* fix(build): write BUILD_ID via writeBundle so App Router builds emit it The vinext:build-id plugin used closeBundle, which does not fire during the multi-environment createBuilder().buildApp() pipeline used for App Router production builds. As a result dist/server/BUILD_ID was silently never written for pure App Router apps — only hybrid apps (which run a second vite.build() pass) and Pages Router apps got the file. This was masked until server bundles started being minified by default (#1777): the e2e deploy harness (scripts/e2e-deploy.sh) falls back to regex-parsing the buildId out of dist/server/index.js when BUILD_ID is absent, and minification mangles the `get buildId()` / `buildId = "..."` patterns the regex relies on. The result was ~700 App Router deploy-suite tests failing at setup with "Failed to extract build ID from dist/server/index.js". Switch the plugin to writeBundle (mirroring the working vinext:image-config plugin), which fires for every emitted bundle; the existing one-time write guard ensures the file is written exactly once. Add a regression assertion to the App Router production build test. * fix(build): share one build ID across all plugin instances in a build A single `vinext build` can instantiate vinext() more than once: the App Router multi-environment build (createBuilder().buildApp()) and the separate Pages Router SSR build for hybrid app+pages apps are distinct plugin instances. With no user generateBuildId, each instance resolved its own random UUID, so the App Router runtime, the Pages Router runtime, the prerender manifest, and dist/server/BUILD_ID could each get a different build ID — risking ISR/seed-cache key mismatches for hybrid apps. The CLI now resolves the build ID once (honoring the user's generateBuildId) and publishes it via __VINEXT_SHARED_BUILD_ID. The plugin adopts it unless the user supplied their own generateBuildId (which is authoritative and already shared because every instance calls it). resolveBuildId()'s standalone semantics are unchanged, so the build-only coordination never leaks into dev or tests. Verified on the hybrid app-router-cloudflare example: the App Router RSC/SSR runtime, Pages entry, prerender manifest, and BUILD_ID file now all carry the same build ID. Adds a coordination test alongside the BUILD_ID emission guard. * fix(build): always adopt shared build ID, even with generateBuildId Addresses review on #1810. The previous `!rawConfig?.generateBuildId` guard reintroduced the divergence it was meant to fix: when generateBuildId returns null (documented Next.js behavior) or is non-deterministic, resolveBuildId() mints a fresh random UUID per plugin instance, so a hybrid app+pages build still got divergent IDs across the buildApp() and Pages vite.build() instances. The CLI's resolvedNextConfig.buildId is already the fully-resolved authoritative value (it ran resolveBuildId honoring generateBuildId, including the null→UUID fallback), so the plugin now always adopts the shared ID when set. Add a regression test using generateBuildId: () => null + a shared ID, and document that the CLI intentionally does not clear the env var (the build process exits). |
||
|
James Anderson
|
fe731b4827 |
test(e2e): mark flaky dev-overlay build-error HMR test as fixme (#1812)
The app-router dev error overlay test "server component HMR surfaces Vite build errors after a clean load" is flaky in CI: after writing the clean file back, the build-error overlay intermittently stays visible past the 10s timeout. Mark it test.fixme to unblock CI while the recovery path is investigated. Tracked in #1811. |
||
|
James Anderson
|
5db85cb6eb |
perf(og): dedupe resvg/yoga wasm in server bundle (#1801)
* fix(og): dedupe resvg/yoga wasm in server bundle
vinext shipped each @vercel/og WASM module twice — once per loader
strategy — even though only one strategy runs at runtime, wasting
~1.4 MiB per build.
resvg.wasm was emitted as a hashed asset by the `import("./resvg.wasm?module")`
path (used by workerd) AND copied as a second byte-identical root file
purely for the Node.js disk-read fallback. yoga.wasm was emitted as a
hashed asset AND inlined as a ~95 KiB base64 blob in index.edge for the
Node.js fallback.
This change makes each WASM ship exactly once, with every loader path
resolving to the single emitted file:
- og-font-patch (index.ts): drop the yoga base64 preamble and switch the
Node.js yoga fallback to a disk read (node:fs + new URL), mirroring resvg.
- og-assets.ts: in generateBundle, when the bundler already emitted the
WASM asset, rewrite the `new URL("./x.wasm", import.meta.url)` fallback
reference to point at the emitted sibling asset; only copy a root file in
writeBundle when no asset was emitted.
The dedup decision keys off whether the bundler emitted the asset, never
the deploy target, so it benefits Node/self-hosted builds too. No platform
detection.
* fix(og): keep chunk sourcemap in sync on wasm fallback rewrite
Use magic-string (already a dependency) for the generateBundle fallback-URL
rewrite so the chunk's sourcemap stays valid after the edit, instead of a
blind string replace that invalidated chunk.map.
Also extract copyMissingOgWasm() as a dependency-injected helper and make the
copy-fallback test hermetic: yoga.wasm is not shipped by @vercel/og (it is
synthesized from base64 by the og-font-patch transform), so the prior test
only passed when a previous build had left yoga.wasm in node_modules.
* test(og): cover cross-directory fallback rewrite; document cross-hook state
Address bonk review observations: add a test asserting the path.posix.relative
rewrite produces a directory-relative ref when the chunk and emitted asset live
in different dirs (the main behavioral win), and document the generateBundle->
writeBundle dedupedBases dependency.
|
||
|
James Anderson
|
ddc529af5e |
docs(agents): forbid installing with --no-frozen-lockfile (#1802)
* docs(agents): forbid installing with --no-frozen-lockfile * docs(agents): clarify --no-frozen-lockfile rule rationale and ask user * docs(agents): correct --no-frozen-lockfile guidance |
||
|
Nathan Nguyen
|
dab28c4b0f |
fix(app-router): refetch same-page search navigations (#1744)
* fix(app-router): refetch same-page search navigations Same-page App Router navigations with a changed search string could replay a prefetched RSC payload instead of requesting the target payload. That diverges from Next.js, which refetches page segments for same-page navigations because search params are page inputs. Bypass visited and prefetched RSC caches for navigate operations whose pathname is unchanged but search params differ. Add a prerendered production browser regression that ports the Next.js hash navigation request assertion. * fix(app-router): hide rsc cache query from middleware * test(app-router): mirror upstream hash scroll offsets * fix(app-router): address review feedback on rsc query navigation - Reuse fixture-scoped node_modules linking in the hash RSC spec instead of symlinking repo-root node_modules, matching sibling production specs. - Add an explicit assertion that no request leaked `_rsc` to the fixture middleware (HTTP 599) for a clear failure mode. - Document that the optimistic shell is intentionally not gated by shouldBypassNavigationCache (detached commit, always superseded). - Detect the RSC cache-busting param via the encoding-aware stripper so the guard and stripper can never diverge on encoded-key edge cases. * fix(app-router): share encoding-aware matcher for _rsc guard Replace the before/after url.search comparison in requestWithoutRscCacheBustingSearchParam with a dedicated hasRscCacheBustingSearchParam helper that reuses the stripper's isRscCacheBustingSearchPair matcher. This makes the guard and stripper share a single source of truth and avoids spuriously rebuilding/normalizing requests whose only difference is degenerate empty query pairs (e.g. ?a=1&&b=2). * fix(app-router): preserve cf metadata when hiding _rsc from middleware requestWithoutRscCacheBustingSearchParam reconstructed the userland request with a bare new Request(url, source), which drops the Workers-specific cf property and omits duplex: "half" for streaming bodies (throwing on Node/undici). Add a cloneRequestWithUrl helper mirroring cloneRequestWithHeaders that overrides the URL while preserving headers/cf and setting duplex, and route the _rsc strip through it. * fix(app-router): stop _rsc leaking to external rewrite proxies The beforeFiles/afterFiles/fallback applyRewrite calls forwarded the original request (with _rsc) to proxyExternalRequest, which reconstructs the upstream URL from request.url and merges its query params — leaking the internal RSC transport query to external rewrite destinations. Pass the _rsc-stripped userlandRequest to all three calls, consistent with the middleware invariant. Add a regression test that points an external rewrite at a loopback server and asserts the forwarded URL drops _rsc while preserving userland query params. * test(app-router): wait for idle network before tracking RSC requests The query-param link is in the viewport and gets prefetched on load. On slower runtimes (WebKit), that prefetch could land after rscRequestUrls.clear(), so it was misattributed to the hash-only navigations and tripped the "no with-query-param request" assertion. Wait for networkidle before clearing, mirroring upstream's waitForIdleNetwork() in the ported Next.js test. --------- Co-authored-by: James <james@eli.cx> |
||
|
James Anderson
|
d3870efe57 |
refactor(cache): deprecate imperative cache handler setters and exports (#1800)
Steer consumers toward the declarative `cache` option on the vinext() plugin (with the @vinext/cloudflare builders kvDataAdapter/cdnAdapter) instead of imperatively wiring cache handlers from a worker entry. - Mark setDataCacheHandler, setCacheHandler, and setCdnCacheAdapter as @deprecated with config-based migration examples. They remain functional as the internal registration target used by virtual:vinext-cache-adapters. - Mark the manually-instantiated handler exports MemoryCacheHandler and KVCacheHandler as @deprecated for consumers. - Update the next/cache type declarations in next-shims.d.ts to match. - Fix stale AGENTS.md claim: the default data cache is in-memory (MemoryCacheHandler) in all runtimes; KV is opt-in, not the default. |
||
|
James Anderson
|
2181c904dc |
fix(check): only flag config options used as property keys (#1778)
* fix(check): only flag config options used as property keys
vinext check reported `webpack` as an unsupported config option even
when the project had no custom webpack config. The analyzeConfig
heuristic matched the bare word `webpack` anywhere in next.config (a
comment, or a value substring like `react-server-dom-webpack`),
producing false positives.
Match the option name only when it appears as an actual object property
key (`opt:`, method shorthand `opt(`, assignment `opt =`, or quoted
`"opt":`).
* fix(check): strip comments and string values before option detection
Addresses review: the boundary+follower regex alone only fixed comments
without a trailing :/(/= and still flagged options mentioned in comments
(// webpack: removed) or embedded in string values ("webpack:1234").
Add a single-pass, lexer-grade pre-pass (modeled on hasFreeCjsGlobal)
that removes comments and empties string/template-literal values before
running the per-option regexes. It is regex-literal aware so a quote
inside /['"]/ cannot hijack string state, and quoted property keys
("webpack":) are preserved. Runs in O(n) time so it is safe on large
files, consistent with the #1776 large-file fix.
Adds tests for the reviewer's comment and string-value cases plus
scanner edge cases (regex literals, template interpolation, division).
* fix(check): scope nested option lookup to parent block; escape regex keys
Addresses Bonk re-review findings:
- Dot-notation options (e.g. i18n.domains) were detected by testing the
parent block and child name as two independent regexes anywhere in the
file, so a config with both `i18n: {…}` and `images: { domains: [...] }`
wrongly reported `i18n.domains`. Extract the parent block body (naive
brace balancing is safe on already-stripped content) and require the
child to appear inside it.
- Escape interpolated option/key names before building RegExps via a new
escapeRegExp helper, future-proofing against keys with metacharacters.
Adds tests for both scoping cases.
* fix(check): scan all parent blocks and match quoted parent keys
Closes the two non-blocking findings from Bonk re-review:
- extractBlockBody only inspected the first matching parent block, so a
child key in a later same-named block (two `experimental: {}` objects)
was missed. Renamed to extractBlockBodies, iterating every match.
- The dot-notation opener now allows an optional closing quote, so quoted
parent keys (`"experimental": {}`) are matched, consistent with how the
per-option regex handles quoted keys.
Adds tests for both cases.
* refactor(check): parse next.config to an AST instead of scanning text
Replace the hand-rolled comment/string scanner + regex heuristics with a
proper AST parse via oxc-parser (the project's own toolchain). analyzeConfig
now reads option keys directly off the exported config object, which is
exact: comments, string values, regex literals, template literals, and
identifier substrings can never be mistaken for a config key.
This deletes stripCommentsAndStrings, extractBlockBodies, and escapeRegExp
(~115 net lines) and also handles cases the text scan couldn't: configs
wrapped in plugin calls (withMDX(config)), variable-referenced configs, and
TS as/satisfies annotations. The dot-notation child lookup is now a true
parent/child relationship from the AST rather than scoped text matching.
Adds oxc-parser as a dependency (already in the install tree via vite-plus).
* refactor(check): use Vite's parseAst instead of adding oxc-parser dep
Vite already re-exports parseAst (its bundled oxc/rolldown parser), so
there's no need to add oxc-parser as a separate dependency. Parse with
{ lang: "ts" } so .ts configs with type annotations / as / satisfies parse
the same as .js. Types come from Vite's exported ESTree namespace.
Drops the oxc-parser dependency and catalog entry added in the previous
commit; vite is already a required peer dependency of vinext.
* fix(check): detect function-form next.config
Bonk re-review caught a regression vs main: function-form configs — a
documented Next.js pattern that vinext supports at runtime
(config/next-config.ts) — reported no options under the AST rewrite
because resolveObject did not look through functions.
Extend resolveObject to unwrap arrow/function configs: a concise arrow
body is the config object directly; a block body resolves the first
top-level return statement's argument. Covers both
`(phase) => ({…})` and `function(phase, { defaultConfig }){ return {…} }`.
Adds tests for both forms.
* fix(check): detect `export default function` config form
resolveObject handled FunctionExpression (module.exports = function(){…})
and arrow forms but not FunctionDeclaration, which is what
`export default function (phase) { return {…} }` parses to. That form
silently reported no options.
Add FunctionDeclaration to the function-form branch and widen the param
type to ESTree.Function. Adds a regression test.
* fix(check): detect options across all branches of multi-phase configs
resolveObject took only the first top-level return of a function body, so
the canonical next/constants multi-phase form — where the phase-specific
config is an early return nested in an if and the default is the trailing
return — only reported the default branch's keys.
Resolve all object literals a config expression can denote and merge their
keys: collect every reachable return arg (descending through if/switch/try
but not into nested functions) and handle ternary branches. Covers the
PHASE_* if/return form and the ternary function form.
Adds tests for both.
|
||
|
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> |
||
|
James Anderson
|
eca6d3fe83 |
perf(router): lazy-load App Router page and route-handler modules (#1781)
* feat(router): lazy-load App Router page and route-handler modules
Statically-imported route modules were all evaluated at Worker startup.
For typical pages this is cheap, but routes with expensive module-level
initialization make startup scale linearly and can exceed Cloudflare's
~400ms startup CPU budget. Measured on real Workers: 150 pages with heavy
module-init went from ~414-467ms startup (eager) to ~17ms (lazy); typical
pages are unaffected (~0.01ms/route either way).
The RSC manifest now emits page modules of static routes and all
route-handler modules as `() => import()` thunks instead of eager
`import * as`, so they are code-split out of the entry's top-level
evaluation and loaded on demand for the matched route only.
ensureAppRouteModulesLoaded() hydrates a route's lazy modules onto the
synchronous page/routeHandler fields before any consumer reads them; it is
idempotent and dedups concurrent loads. Hydration is invoked at the central
match point and at every mid-flight route lookup that reads modules before
buildPageElement (server-action redirect/rerender targets, interception and
ISR revalidation source routes).
Dynamic-route pages stay eager because their generateStaticParams is
referenced in the module-level generateStaticParamsMap; making those lazy
needs a prerender-resolver rework and is tracked as follow-up.
* fix(router): retry failed lazy route-module imports instead of caching the rejection
Addresses /bigbonk review feedback on the lazy route-module loader:
- ensureAppRouteModulesLoaded no longer caches a rejected dynamic import().
On failure it clears __loading (leaving __loaded false) and re-throws, so
the current request still sees the error but the next request retries —
matching the eager model's per-isolate retry semantics instead of wedging
the route into a permanent 500 and risking an unhandled rejection.
- Document why the getLazyLoaderVar import() codegen is a CodeQL false
positive (trusted filesystem-scan path, same trust model as the eager
getImportVar import * as).
- Add a rejection-path unit test covering retry-after-failure.
* docs(router): clarify why route handlers can always be lazy-loaded
Reword the getLazyLoaderVar comment per /bigbonk re-review: route handlers
are lazy because they are never sourced into generateStaticParamsMap (which
only reads layouts + page), not because they 'have no generateStaticParams'
(Next.js route handlers can export it for prerendering — a separate gap).
* feat(router): lazy-load all page modules, including dynamic routes
The previous cut kept dynamic-route pages (and anything nested under a
dynamic segment) eager, because their generateStaticParams was referenced
synchronously in the module-level generateStaticParamsMap. That gutted the
benefit for a large class of apps: anything with a dynamic root segment
(i18n `[locale]`, multi-tenant `[org]`) made every route dynamic, so
nothing was lazy — and the heavy `[slug]` page (the prime heavy-module-init
candidate) stayed eager.
Now ALL page modules are lazy. generateStaticParamsMap embeds lazy `{ load }`
page sources instead of `mod.generateStaticParams`, and
createAppPrerenderStaticParamsResolver imports them on demand at prerender
time (already async). It returns the same null "no static params" sentinel
the prerender driver relies on (skip / output:export error), so SSG
behavior is unchanged. Layout generateStaticParams sources stay eager.
The eager set is now just genuinely-shared modules: layouts, templates,
boundaries, intercepts, and global-error.
Verified on real Cloudflare: 150 heavy pages under a dynamic `[locale]`
root went from eager (~450ms startup) to ~22ms, and SSG via the `[locale]`
layout's generateStaticParams still prerenders correctly.
Tests: prerender + prerender-route-params + prerender-endpoints + features +
prod/dev server + interception + actions all green; new
app-prerender-static-params unit tests cover the lazy-source path and null
sentinel; entry-templates updated for all-lazy pages and `{ load }` sources.
* refactor(router): preserve source order in prerender static-params resolver
Per /bigbonk re-review: the resolver partitioned sources into
[...eagerFns, ...lazyFns], which only matched the intended composition order
because the codegen happens to emit layout (eager) sources before the page
(lazy) source. Resolve sources in their original declared order instead —
each source maps to its eager function or its awaited lazy function, then
non-functions are dropped — so composition order no longer depends on the
emitter's append order. Added a unit test with a lazy source ordered before
an eager one.
|
||
|
James Anderson
|
f9d83fd8f2 |
fix(build): bundle @vinext/cloudflare into vinext to break dependency cycle (#1797)
* fix(build): bundle @vinext/cloudflare into vinext to break dependency cycle vinext consumed a few runtime helpers (KVCacheHandler, CloudflareCdnCacheAdapter, ENTRY_PREFIX) from @vinext/cloudflare via a runtime `dependencies` edge. Combined with @vinext/cloudflare's `peerDependencies` on vinext, this formed a cycle that forced changesets to force-major @vinext/cloudflare on every vinext release (mitigated defensively in #1764). Bundle the small amount of @vinext/cloudflare code vinext actually imports into vinext's dist via tsdown's deps.neverBundle, and move @vinext/cloudflare to devDependencies. The published install graph now points one way (@vinext/cloudflare -> vinext). @vinext/cloudflare remains a published package: user vite.config files still import cdnAdapter()/kvDataAdapter(), and the generated worker resolves its *.runtime.js factories by absolute path. Its bundled-in vinext/shims/* imports stay external and resolve via vinext's own package exports (Node self-reference). Drop the now-obsolete create-next-app CI steps that packed and overrode @vinext/cloudflare (vinext's tarball no longer declares it as a dependency). * refactor(build): clarify @vinext/cloudflare bundling config Use a single intent-revealing neverBundle predicate with a BUNDLED_DEPS carve-out and document why the carve-out must live in neverBundle (tsdown rejects skipNodeModulesBundle + alwaysBundle as mutually exclusive, and a catch-all neverBundle takes precedence over alwaysBundle). * fix(build): bundle @vinext/cloudflare via source alias, not external predicate The previous approach replaced deps.skipNodeModulesBundle with a custom neverBundle predicate. That broke tsdown's rewriting of vinext's own `vinext/shims/*` tsconfig-path self-imports to relative paths, leaving them as bare `vinext/*` specifiers in 46 files. Across Vite's separate RSC/SSR/client dev module graphs those bare self-references resolved to distinct module instances, breaking identity checks (e.g. `instanceof ReadonlyURLSearchParams`). Keep skipNodeModulesBundle: true untouched and instead bundle @vinext/cloudflare by aliasing its cache/* subpath to source. skipNodeModulesBundle externalizes bare package specifiers before tsconfig paths apply, so the alias rewrites the import to a file path up front; tsdown then treats it as local source and bundles it. The bundled code's own vinext/shims/* imports still resolve to vinext's relative output (single module instance). Output is byte-identical to the pre-change build except the added bundled cloudflare files and one cosmetic import reorder. |
||
|
Nathan Nguyen
|
1a480016f6 |
fix(app-router): track searchParams access for static bailout (#1788)
* fix(app-router): track searchParams access for static bailout App Router pages that awaited or used the searchParams prop during prerender could still be classified as static when the prerender request had an empty query string. That seeded no-query output into the deploy cache and served it for requests like ?search=hello. The dynamic decision was based on query-string content instead of page prop access. Observe the searchParams thenable itself, including React use() status reads, and mark searchParams dynamic only when the prop is consumed. * chore: rerun ci * fix(app-router): avoid static cache for searchParams pages * fix(app-router): tolerate synthetic page paths in manifest codegen * fix(app-router): observe searchParams access for static bailout * fix(app-router): preserve app page cache hits * fix(app-router): observe loading search params during render * refactor(app-router): extract searchParams probe fan-out into helper Move the per-request page probe fan-out (matched page + parallel slot pages + interception page) out of the generated RSC entry and into a unit-testable buildAppPageProbes() helper in server/app-page-probe.ts, keeping entries/app-rsc-entry.ts thin (AGENTS.md guidance). Addresses bonk review concern #3 on #1788. Document the known slot over-bail (probing inactive parallel-route slot pages can mark an otherwise-static page dynamic; fails safe toward dynamic) and track it as a caching-efficiency follow-up in #1798 (bonk review concern #1). * fix(app-router): skip probing interception-overridden slot pages Narrow the searchParams probe fan-out so it only probes page components that actually render for the request. When an interception matches, it replaces the page of the slot named by intercept.slotKey (the element builder sets overrides[slotKey].pageModule to the interception page, which wins over slot.page in app-page-route-wiring.tsx). We now probe the interception page in place of that slot's own page instead of probing both — probing the overridden slot page marked an otherwise static request dynamic for a component that never renders. Fixes the over-bail flagged in review of #1788. * docs(app-router): clarify buildAppPageProbes slot precision Address review feedback: the doc comment overstated precision without explaining why non-overridden slots are exact. Spell out that a slot with a page.tsx always renders that page (so probing it is correct), default-only slots have no slot.page.default so probing is a no-op (not an over-bail), and a default.tsx awaiting searchParams is backstopped by the real render's observation (so no under-bail). * fix(app-router): gate searchParams interception probe on isRscRequest Interception only fires for RSC navigations (resolveAppPageInterceptState returns kind:none when !isRscRequest, app-page-request.ts:324). The probe fan-out called findIntercept unconditionally, so on HTML requests it would skip the matched route's slot page (which DOES render normally) and probe the interception page (which never renders). buildAppPageProbes now ignores the interception match when !isRscRequest, so HTML requests probe every slot's own page and skip the interception probe. The source-route interception case (a different route renders) never reaches this probe: dispatchAppPage returns the intercepted response before calling probePage, so any interception seen here is the current-route override case. Documented both in the buildAppPageProbes doc comment. Addresses review feedback on #1788. --------- Co-authored-by: James <james@eli.cx> |
||
|
James Anderson
|
c34b05c131 |
fix(image): scan image imports via AST instead of regex (#1779)
* fix(image): scan image imports via AST instead of regex
The image-imports transform scanned raw source with a regex, which
matched `import X from '...img'` text anywhere it appeared, including
inside comments, strings, and template literals. When a commented-out
image import pointed at a file that exists on disk, the rewrite emitted
a `const X = { src: __vinext_img_url_X, ... }` referencing an undefined
variable, crashing SSR in dev and returning 500 in production.
Replace the regex scan with AST-based scanning via parseAst, rewriting
only real top-level ImportDeclaration nodes that use a single default
specifier and whose source ends in an image extension. The code regex
filter is kept purely as a cheap pre-check.
Add regression tests for commented-out imports (line and block),
image-import text inside string literals, and named/namespace imports.
* fix(image): parse as tsx so enforce:pre handler accepts JSX/TS
The image-imports plugin runs with enforce:"pre", so its transform sees
raw source before the JSX/TS transform. parseAst defaults to plain JS
and threw on JSX and TS type annotations; the handler swallowed the
error and returned null, silently skipping image transforms for every
.tsx/.jsx/typed-.ts file (hero.src/width/height became undefined).
Parse with { lang: "tsx" } so JSX + TS + JS all parse with correct node
offsets. Add regression tests using real TSX (JSX in body) and typed-TS
(type annotations) syntax that plain-JS parsing would reject.
* fix(image): pick parser lang by extension to avoid breaking .ts files
Always parsing as tsx broke plain .ts files: the tsx grammar reads `<T>`
as the start of a JSX element, so TS-only syntax such as angle-bracket
casts (`<Foo>bar`) and non-comma generic arrows (`<T>(x) => x`) threw
during parse. The catch swallowed the error and returned null, silently
skipping the image transform (hero.src/width/height become undefined).
Select the language by extension: `.ts` parses as `ts`, everything else
(.tsx/.jsx/.js/.mjs, which may contain JSX) parses as `tsx`. Add
regression tests for the angle-bracket cast and generic-arrow cases.
|
||
|
Nathan Nguyen
|
b324cbe04c |
refactor(utils): dedupe shared helpers and lint redefinitions (#1793)
* refactor(utils): dedupe shared helpers and lint redefinitions Shared helper logic was being reimplemented across generated entries, runtime modules, and shims. That made small utility semantics easy to drift and gave agents no mechanical feedback when they hand-rolled copies. Add a local Oxlint rule that discovers exported shared helpers and reports local redefinitions, including helpers embedded in generated source strings. Centralize the existing low-risk duplicates through shared utility modules. * fix(lint): harden shared utility rule validation * test(lint): avoid matching oxlint success summary * fix(lint): make shared utility rule cwd independent * fix(lint): address bonk review comments - Drop redundant "default" export conditions in package.json; the generated worker template resolves these subpaths via "import" like its siblings (request-pipeline, pages-i18n, config-matchers). - Mask backtick template literals in prefer-shared-utils so an apostrophe or comment sequence inside generated template source can no longer open string/comment masking and swallow a subsequent helper definition (false negative). Template bodies stay scannable. --------- Co-authored-by: James <james@eli.cx> |
||
|
James Anderson
|
0b7721dba6 |
perf(build): minify server build environments by default (#1777)
* fix(build): minify server build environments by default Vite only minifies the client environment; non-client (rsc/ssr and the Cloudflare worker) environments default build.minify to false, so the deployed worker and SSR renderer ship unminified. Add a build-only configEnvironment default that sets minify: true for server environments, yielding to any explicit user/plugin minify setting (including false). * fix(build): patch route classification before minify; build fixtures unminified for output introspection Minifying server environments by default exposed two problems: 1. Real bug: build-time route classification ran in `generateBundle`, but rolldown's minifier renames the top-level `__VINEXT_CLASS` function and mangles its `routeIdx` parameter before that hook runs, so the stub regex never matched and the patch silently no-op'd — every route fell back to the Layer 3 runtime probe in production. Move the injection to a `renderChunk` hook with `order: "pre"` so it runs before minification; the patched body is then minified along with the rest of the chunk (the runtime calls it by reference, not by name). 2. Test artifacts: three build tests introspect emitted server chunks for readable identifiers (registerConfiguredCacheAdapters, __VINEXT_CLASS) or depend on unminified chunking (MDX frontmatter leak check). These are test-only concerns — the runtime behavior is unchanged. Build their fixtures with `build.minify: false` (a user-overridable default) so the assertions stay deterministic. * fix(build): drop now-unused RouteClassificationChunk export (knip) The renderChunk refactor removed the external consumer of this type; keep it as a local type so knip passes. * test(build): guard minified classification path; fix stale generateBundle refs Re-add real regression coverage that the route-classification injector runs before minification (the production default). The prior fix made three tests pass by building with minify:false, which is exactly the config where the old post-minify generateBundle code also passed — so the tests no longer guarded the bug. - classification integration: add a production-default (minify on) suite that asserts minify-robust signals (injected switch dispatch shape + the module-graph reason literal, which is build-time-only). Verified it FAILS when the injector is reverted to a post-minify generateBundle hook. - cache adapters: assert the minify-safe LOCAL_ADAPTER_MARKER under the default minified build instead of grepping the renamed registerConfiguredCacheAdapters symbol. - tsconfig path alias (#659): run minified; the old not.toContain('title: "Second Post"') was a quote-escaping artifact (frontmatter renders as MDX content identically in both builds, not a minify leak) — replaced with a real parse-error/raw-YAML guard. - update stale generateBundle references to renderChunk in injector error strings, app-rsc-entry stubs, and index.ts comments; note the per-chunk patch assumption near the renderChunk hook. * test(build): run classification tests against minified output (no minify:false) Previously the dispatch-logic suites built the fixture with minify:false so they could regex-extract and eval the readable __VINEXT_CLASS function — but that is the one config where the original post-minify-hook bug also passes, so it masked the regression. Rewrite the extractors to be name-agnostic: derive the (mangled) dispatch function name from its property-keyed call site (__buildTimeClassifications/ __buildTimeReasons — property keys survive minification) and eval the body (string-literal values like "static" also survive). All suites now build with vinext's production defaults (minify on). Verified by reproducing the bug (disabling the patch): the minified tests fail with "was not patched". * refactor(build): rename buildGenerateBundleReplacement; fix stale generateBundle doc refs Address review nits: the classification dispatch builder still carried the old hook name (it now runs in renderChunk, not generateBundle). Rename it to buildClassificationReplacement (parallel to buildReasonsReplacement) and fix two stale 'generateBundle' doc comments (route-classification-manifest.ts:203, layout-classification.ts:130) to say renderChunk. |
||
|
Jerry Zhao
|
c8975a48cf |
chore(deps): bump playwright to 1.60.0 (#1796)
* chore(deps): bump playwright to 1.60.0 * u |
||
|
Nathan Nguyen
|
df066cb486 |
ci: optimize CI sharding (#1768)
* ci: add fifth integration shard
* ci: weight integration shards from timing data
* ci: shard app-router e2e
* ci: rerun optimization experiment
* ci: add sixth weighted integration shard
* ci: rebalance weighted integration shards
* ci: shard unit tests
* ci: rebalance integration shards from current timings
* ci: move weighted integration shard list from YAML into script + timing manifest
* ci: move weighted integration shard list from YAML into script + timing manifest
* feat(ci): derive integration shard weights from real CI timings with provenance
Integration shard weights lived in a hand-seeded flat path->ms map
("aggregation": "manual seed"). A reviewer could not tell a measured
number from a guess, and the guesses were wrong: favicon-short-circuit
was seeded at 5s but runs ~35s in CI across five runs, a 7x under-weight
that mis-packed the shards. The seed had no provenance and no way to
regenerate from real data.
Restructure the manifest to a v2 provenance model: per file estimateMs
(the weight the planner uses), plus medianMs/p75Ms/samples and a
generatedFrom.runs list, an estimator metric, and generatedAt. Add
scripts/ci-integration-timings-refresh.mjs to aggregate Vitest blob
reports downloaded from successful CI runs (p75 per file, nearest-rank)
and rewrite the manifest deterministically, failing closed when the
blobs do not cover every discovered file. The manifest here was
regenerated from 5 successful runs (30 blobs); all six shards now pack
to 84s.
Extract planning and blob parsing into scripts/lib/* so the fragile
Vite+ blob-parser probe lives in one place. Replace the O(files*shards)
lightest-group scan with an O(n log m) binary min-heap and collapse the
three duplicated local-search move/swap helpers into one makespanAfter +
transfer primitive. Behavior preserved: the --check gate still verifies
every file lands in exactly one shard.
Harden --check to fail closed on no discovered files, missing, stale,
malformed/zero/negative timings, shard-count drift, and bucket coverage.
Add an advisory --recommend mode that models the optimal shard count from
real weights and flags when integration has dropped below the competing
cross-job bottleneck. It is advisory only and never runs in CI; the count
stays declarative in manifest.shardTotal with the matrix enforced
against it.
* ci: pass integration shard file list via env to avoid template injection
The integration shard step expanded ${{ steps.shard.outputs.files }}
directly into the run: block. That output is a list of test file paths
discovered from `vp test list`, and on pull_request runs a filename is
attacker-controllable: a fork PR adding a file whose name contains shell
metacharacters would inject it into the runner shell. GitHub code
scanning (zizmor) flagged this as template-injection, alert 163.
Route the file list and the other computed values through env vars and
reference them in the script, leaving $SHARD_FILES unquoted so the shell
still word-splits it into separate file arguments. The shell now treats
the value as data, never as script text. Verified with zizmor: the
pre-fix workflow reports template-injection on this line, the fixed
workflow reports no findings.
* feat(ci): require refresh blobs to back the claimed --run provenance
The refresh tool recorded every --run id as provenance but only checked
that each discovered file had at least one timing sample. Passing five
--run ids with blobs for a single complete run still produced a manifest
claiming five-run provenance while every file held one sample. The
manifest could claim stronger provenance than the blob directory backs.
A test file runs in exactly one shard per run, so one complete run
yields exactly one sample per file. Require samples === runIds.length for
every discovered file: too few means a claimed run's blobs are missing,
too many means the directory holds blobs beyond the claimed runs.
--allow-partial relaxes the check to "at least one sample per file" for
the re-run-failed-shard case while still recording the true per-file
sample count.
* experiment: run integration at 5 shards to benchmark the latency/cost knee
Temporary, for benchmarking only. Repacks the same provenance weights
into 5 integration shards instead of 6 (manifest shardTotal and matrix
set to 5, Check gate updated to match) so the 5 vs 6 trade-off can be
measured with the same weights, unit split, and E2E split. To be
reverted to 6 after the run is captured.
* experiment: go aggressive on wall-clock (8 integration, 3 unit, 3 app-router E2E)
Runner minutes are free on this public repo, so the objective is pure
wall-clock. Attack the whole critical-path cluster at once: integration
to 8 shards (~63s test load each, near the per-file floor), unit to 3,
and the app-router E2E project to 3-way so none of them becomes the new
ceiling once the others drop. Report job left as-is. Benchmarking only;
final counts settle after the run lands.
* ci: set integration to 10 shards, the wall-clock floor on free CI
Public repo, so runner minutes are free and the objective is pure
wall-clock. At 10 shards each integration shard carries ~51s of test
load; combined with the serial report tail this brings the integration
critical path down to roughly where the un-shardable create-next-app
(windows) job sits, so additional shards stop moving the overall wall.
Keeps unit at 3 shards and the app-router E2E project at 3-way from the
prior step. Benchmarking continues; counts can still change.
* fix(ci): default refresh shard count to the existing manifest, not a constant
ci-integration-timings-refresh.mjs defaulted --shard-total to a hardcoded
6. The documented refresh command in ci.yml omits --shard-total, so once
the matrix moved past 6 shards, following the advertised workflow rewrote
shardTotal: 6 into the manifest and the next run failed the Verify
integration shard manifest step with shard-count drift.
Default to the current manifest's shardTotal instead. manifest.shardTotal
is the single source of truth for the count: the matrix mirrors it and
--check enforces no drift, so a plain refresh now preserves whatever the
matrix uses. An explicit --shard-total still overrides it for an
intentional count change, and a missing count with no existing manifest
now fails with a clear message instead of silently picking a number.
Found by Codex review on
|