Skip to content

chore: sync main with upstream/main#1

Merged
NathanDrake2406 merged 8 commits into
mainfrom
sync/upstream-main-20260308-90771f7
Mar 7, 2026
Merged

chore: sync main with upstream/main#1
NathanDrake2406 merged 8 commits into
mainfrom
sync/upstream-main-20260308-90771f7

Conversation

@NathanDrake2406

Copy link
Copy Markdown
Owner

Summary\n- fast-forward fork history to upstream/main at 90771f7\n- bring origin/main in sync with cloudflare/vinext main\n\n## Verification\n- fetched both remotes and compared origin/main, main, and upstream/main\n- fast-forwarded local main with git merge --ff-only upstream/main\n

tomasgferreira and others added 8 commits March 7, 2026 01:27
…oudflare#289)

* fix(instrumentation): defer ssrLoadModule call to post-middleware hook

Vite 7's SSRCompatModuleRunner requires the SSR environment's transport
channel to be initialized before ssrLoadModule() can be called. During
configureServer(), the channel is not yet ready, causing a TypeError
when instrumentation.ts exists.

Move the runInstrumentation() call from the configureServer body into
the returned post-middleware function, where environments are fully
initialized.

Fixes cloudflare#167

* fix(instrumentation): address review comments on deferred ssrLoadModule

- Remove unreachable .catch() on runInstrumentation() since internal
  try/catch already handles errors and the function never rejects
- Mock console.error in transport error test to suppress noisy output
  and assert the expected error message
- Rename misleading test to accurately describe what it verifies

* test: address instrumentation review feedback
…oudflare#122)

* fix(deploy): monorepo-aware lock file and node_modules resolution

vinext deploy failed in monorepos in two ways:

1. Wrong package manager detected — detectPackageManager only checked for
   bun.lockb (legacy binary format), missing bun.lock (text format, Bun
   v1.0+). It also only searched the immediate project directory, so any
   monorepo where the lock file lives at the workspace root fell through to
   npm, causing a confusing "npm error Invalid Version:" failure.

2. Dependency detection and wrangler binary path hard-coded to the app
   directory — hasCloudflarePlugin, hasRscPlugin, hasWrangler, hasMdxRollup
   all checked {root}/node_modules only, producing false negatives for
   hoisted packages. runWranglerDeploy then crashed with ENOENT because
   the wrangler binary was at the workspace root, not the app root.

Changes:
- Add walkUpUntil<T>(start, check) to utils/project.ts — a shared primitive
  that walks ancestor directories until check() returns non-null. Both the
  lock-file and node_modules walkers are built on top of this.
- Rewrite detectPackageManager / detectPackageManagerName using walkUpUntil;
  add bun.lock (text) check alongside existing bun.lockb (binary).
- Add findInNodeModules(start, subPath) using walkUpUntil — walks up to find
  node_modules/{subPath}, returning the absolute path or null.
- Use findInNodeModules in detectProject (hasCloudflarePlugin, hasRscPlugin,
  hasWrangler), getMissingDeps (hasMdxRollup), and runWranglerDeploy.

Tests (12 new):
- detectPackageManager: pnpm, yarn, bun.lock, bun.lockb, npm fallback,
  monorepo root traversal, closest-wins precedence
- findInNodeModules: package lookup, binary lookup, null fallback,
  monorepo root traversal, closest-wins precedence

* fix(test): clear npm_config_user_agent in npm fallback test

On CI, pnpm sets process.env.npm_config_user_agent which causes
detectPackageManagerName to return 'pnpm' instead of falling through
to the npm default. Save and restore the env var around the assertion.

---------

Co-authored-by: Steve Faulkner <[email protected]>
* feat: add react plugin for client components

* fix: improve react plugin JSDoc and update stale HMR comment

---------

Co-authored-by: Steve Faulkner <[email protected]>
…loudflare#305)

* refactor: extract applyMiddlewareRequestHeaders into config-matchers

Move the x-middleware-request-* unpacking + postMwReqCtx rebuild logic
out of prod-server.ts and deploy.ts and into a shared
applyMiddlewareRequestHeaders() in config-matchers.ts, eliminating ~30
lines of duplicated code from each callsite.

* fix(test): update deploy test to reflect applyMiddlewareRequestHeaders refactor
…flare#306)

* fix: stub node:async_hooks in client builds via virtual module

Several shims (headers, cache, navigation-state, etc.) import
AsyncLocalStorage from node:async_hooks. Since resolve.alias applies
globally, these shims resolve in every environment including client.
Vite externalizes node:async_hooks to __vite-browser-external in browser
builds — an empty stub with no named exports — causing Rollup errors.

Add a vinext:async-hooks-stub plugin that intercepts node:async_hooks
in the client environment and provides a no-op AsyncLocalStorage via a
virtual module. This is semantically correct: shims already guard with
`_als.getStore() ?? fallback` patterns, so undefined store returns
produce correct client-side behavior.

Unlike the external file approach in cloudflare#293, this uses a virtual module
inline in the plugin — no additional files, no build artifacts.

Closes cloudflare#293

* fix: address async-hooks-stub code review feedback

- Convert load hook to object form with handler() for consistency
- Match bare `async_hooks` specifier in addition to `node:async_hooks`
- Add intentionally-minimal comment explaining stub scope
- Add regression tests for resolveId and load hook behavior

* refactor: extract async-hooks-stub plugin to plugins/ and test against real implementation

Move the vinext:async-hooks-stub plugin object from an inline definition in
index.ts to packages/vinext/src/plugins/async-hooks-stub.ts, exporting it as
asyncHooksStubPlugin. Import and use it directly in index.ts.

Update the regression test to import the real plugin via _asyncHooksStubPlugin
and invoke its resolveId/load handlers with a mock context, eliminating the
duplicated logic that the test previously used as a workaround for the plugin
being unexported and inline.

* fix: add filter to load hook, evaluate stub at runtime in test

- Add filter to load hook object form for consistency with resolveId
  and to opt into Vite/Rolldown hook optimization path
- Replace overlapping string-assertion test with one that evaluates
  the generated source via new Function, testing actual runtime
  behavior (getStore returns undefined, run/exit pass through return values)

* fix: remove control-char regex from load filter to fix lint

* fix: suppress no-control-regex for load filter via eslint-disable comment

* refactor: derive load filter regex from ASYNC_HOOKS_STUB_ID constant

* Update build-optimization.test.ts

Co-authored-by: ask-bonk[bot] <249159057+ask-bonk[bot]@users.noreply.github.com>

* Update index.ts

Co-authored-by: ask-bonk[bot] <249159057+ask-bonk[bot]@users.noreply.github.com>

* fix: widen run() type signature to accept rest args and typed callback

---------

Co-authored-by: James <[email protected]>
Co-authored-by: ask-bonk[bot] <249159057+ask-bonk[bot]@users.noreply.github.com>
…loudflare#314)

* refactor(ci): extract composite actions and improve workflow hygiene

- Add composite actions: setup-node-pnpm, build-vinext, deploy-example, comment-preview-urls
- Cache Playwright browsers in CI to avoid ~150MB download per E2E run
- Add concurrency group to deploy-examples.yml to prevent deploy pileup
- Replace inline CJS node script in create-next-app job with portable bash
- Convert ecosystem-run.yml inline Node scripts from CJS require() to ESM
- Apply composite actions across all workflows to eliminate repeated boilerplate

* refactor(ci): revert to inline steps, keep only setup-node-pnpm composite action

Remove the build-vinext, deploy-example, and comment-preview-urls composite
actions and inline their steps back into each workflow. Keep setup-node-pnpm
as the one reusable action (pnpm install + node setup). Retain all other
improvements from the previous commit: concurrency groups, Playwright browser
cache, bash-based dev-server check, ESM node scripts in ecosystem-run.

* refactor(ci): use setup-node-pnpm composite action consistently

Every workflow that does pnpm/node setup + install now uses the
setup-node-pnpm composite action instead of the inline 3-step triplet.
Added optional registry-url input to support publish.yml's npm auth.

Exceptions (intentionally kept inline):
- bonk.yml / bigbonk.yml: issue_comment trigger, GHAS constraint
- tip.yml notify-on-failure: node-only, no pnpm install needed
@NathanDrake2406
NathanDrake2406 merged commit 90771f7 into main Mar 7, 2026
13 of 20 checks passed
james-elicx added a commit that referenced this pull request Mar 12, 2026
- Bug #2: hybrid build skipped Pages Router pre-rendering when App Router entry
  also present — fix: only bail out on pure App Router builds (no pages entry)
- Bug #1: getOutputPath traversal guard bypassed on Windows by backslash urlPath
  — fix: reject urlPath containing backslashes before posix normalize
- Bug #4: double-counting in result.skipped when AbortController fires and
  res.text() throws — fix: replace await res.text() with res.body?.cancel()
- Bug #6: dynamic routes without getStaticPaths/generateStaticParams classified
  as 'ssr' — fix: use 'unknown' (skipped for unenumerable params, not SSR APIs)
- Bug #3: routes that throw in ssrLoadModule silently omitted from
  routeClassifications — fix: wrap in try/catch, add 'unknown' classification
- Bug #5: buildReportRows ignored knownRoutes for API routes — fix: check
  known?.get(route.pattern) first, fall back to 'api'
- Bug #7: dead !Array.isArray guard after try/catch in expandDynamicAppRoute
  — remove unreachable branch
- Bug #8: middlewareHeaders spread onto 200 pre-rendered response could include
  Location header — fix: filter out 'location' before spreading
- Bug #9: configOverride typed as Partial<NextConfig> allows unsafe non-scalar
  fields — narrow to Pick<NextConfig, 'output' | 'trailingSlash'>
@NathanDrake2406
NathanDrake2406 deleted the sync/upstream-main-20260308-90771f7 branch March 18, 2026 09:51
NathanDrake2406 added a commit that referenced this pull request Apr 23, 2026
The test's console-error filter matched any message containing "RSC
navigation error", which also captures an unrelated pre-existing
hydration race: when page.evaluate calls __VINEXT_RSC_NAVIGATE__
before the AppRouter useLayoutEffect has committed, getBrowserRouterState
throws "Browser router state is not initialized". That race has nothing
to do with the stream-parse bug this PR fixes — the outer catch still
hard-navigates to the target URL correctly — but the broad filter
treated it as a failure, producing a flaky test that only passed on
retry (seen on commit 6071901: failed first attempt, passed retry #1).
My earlier commit's tighter hit-count assertion did not mask the
flakiness; it just happened to hit the race on more retries.

Replace the string list with an isRscStreamParseError helper that
matches only the concrete diagnostics createFromFetch emits when handed
an HTML body (Connection closed, createFromFetch / createFromReadableStream
stack frames, Failed to parse RSC, Unexpected token). The pre-fix path
produces exactly these messages, so the test still proves the fix; the
hydration race no longer false-positives.
NathanDrake2406 added a commit that referenced this pull request Apr 24, 2026
…oudflare#875)

* fix(app-router): hard-navigate to browser URL on non-ok RSC fetch

Client-side RSC navigation fetch passes non-ok responses (404, 500, etc.)
directly to createFromFetch. The HTML error body is parsed as an RSC
stream, producing a cryptic "Connection closed" error, and the outer
catch hard-navigates to the same URL that just failed — risking a
reload loop and surfacing an opaque diagnostic.

Non-ok responses are not RSC payloads and must not reach createFromFetch.
Next.js handles this identically in fetch-server-response.ts:211 with a
hard ("MPA") navigation to the browser-facing URL stripped of flight
markers.

Add a !navResponse.ok guard after the stale-navigation check in the
navigation loop: hard-navigate to currentHref (the browser URL without
.rsc suffix). The existing finally block handles
settlePendingBrowserRouterState and clearPendingPathname on this return
path, so no additional bookkeeping is required.

Add a !rscResponse.ok guard in readInitialRscStream so the initial
hydration fallback path reloads instead of attempting RSC parsing on an
HTML error body. Return a never-resolving ReadableStream so the caller
does not proceed into createFromReadableStream before the reload takes
effect.

E2E tests verify: 404 RSC nav hard-navs to the non-.rsc URL; 500
(simulated via page.route interception) hard-navs without logging an
RSC parse error; the final URL never contains .rsc.

* fix(app-router): break reload loop when initial RSC fetch is persistently non-ok

readInitialRscStream reloads on a non-ok RSC response so the server can
render the correct error page as HTML. That assumes the next HTTP
response will differ, which is not guaranteed: if the RSC endpoint is
persistently broken while the HTML document is served successfully
(mismatched server handler, upstream gating, intercepted requests in
tests), the reload fetches the same HTML, the post-hydration RSC fetch
gets the same non-ok, and the page reloads forever.

Guard the reload with a sessionStorage key scoped to the current path.
The first non-ok response sets the key and reloads. The second non-ok
response for the same path clears the key and throws, letting the outer
bootstrap catch surface the error instead of triggering another reload.
A successful RSC response clears the key so a later unrelated failure
on the same path still gets one reload attempt.

Tighten the Playwright test that covers the 500 hard-nav path: after
the hard navigation settles, wait and assert both URL stability and a
bounded intercept hit count. A runaway reload loop would fail the hit
count assertion.

* fix(app-router): harden initial RSC recovery and cover non-RSC content-types

Five review follow-ups to the non-ok RSC fetch guard.

sessionStorage.getItem / setItem / removeItem can throw SecurityError in
strict-mode iframes, storage-disabled browsers, and some Safari private-
browsing configurations. The unguarded calls in readInitialRscStream
crashed hydration in exactly those environments — worse than the
original bug. Wrap every access in readReloadFlag / writeReloadFlag /
clearReloadFlag helpers that swallow the exception and fall through to
the "no prior attempt" branch.

Replace the throw after a looped reload with console.error plus a
never-resolving ReadableStream. main() is invoked as `void main()` with
no outer catch, so a throw here surfaced as an unhandled promise
rejection and fired window.onerror reporting hooks instead of leaving
the SSR'd DOM visible as the comment promised. Returning a stream that
never produces data halts hydration cleanly; the server-rendered HTML
stays on screen, client components simply never hydrate.

Clear the reload flag when readInitialRscStream enters the embedded-RSC
branch. The embed branch runs when the server successfully rendered the
page, so any prior flag is stale — without this clear, the flag could
persist across a hard-reload of the same path and skip the one legitimate
recovery attempt the user should get.

Cover the non-RSC content-type case on both the initial-hydration fetch
and the client-side navigation fetch. Next.js's equivalent guard
(fetch-server-response.ts:211) is `!isFlightResponse || !res.ok || !res.body`;
the previous commit only covered `!res.ok`. A proxy or CDN that returns
200 with a rewritten Content-Type (`text/html` instead of
`text/x-component`) would still reach createFromFetch and throw the
exact stream-parse error these guards exist to prevent. Check for a
Content-Type starting with `text/x-component` before parsing.

Tighten the Playwright hit-count assertion from <= 3 to <= 2. With the
guard in place the expected sequence is exactly two hits (the triggering
client RSC nav fetch plus one post-reload fetch that aborts); any extra
hit signals an unnecessary reload worth catching.

* test(app-router): narrow RSC error filter to stream-parse diagnostics

The test's console-error filter matched any message containing "RSC
navigation error", which also captures an unrelated pre-existing
hydration race: when page.evaluate calls __VINEXT_RSC_NAVIGATE__
before the AppRouter useLayoutEffect has committed, getBrowserRouterState
throws "Browser router state is not initialized". That race has nothing
to do with the stream-parse bug this PR fixes — the outer catch still
hard-navigates to the target URL correctly — but the broad filter
treated it as a failure, producing a flaky test that only passed on
retry (seen on commit 6071901: failed first attempt, passed retry #1).
My earlier commit's tighter hit-count assertion did not mask the
flakiness; it just happened to hit the race on more retries.

Replace the string list with an isRscStreamParseError helper that
matches only the concrete diagnostics createFromFetch emits when handed
an HTML body (Connection closed, createFromFetch / createFromReadableStream
stack frames, Failed to parse RSC, Unexpected token). The pre-fix path
produces exactly these messages, so the test still proves the fix; the
hydration race no longer false-positives.

* fix(app-router): abort hydration bootstrap when recovery cannot restore RSC

Address the half-hydrated-state side effect of the previous commit's
recovery path. When readInitialRscStream hits a persistent failure, the
returned never-resolving stream let main() continue past the await and
still assign window.__VINEXT_RSC_ROOT__, window.__VINEXT_HYDRATED_AT,
and window.__VINEXT_RSC_NAVIGATE__. External probes (test helpers,
analytics, Sentry hydration hooks) saw a hydrated page, user clicks on
Links triggered __VINEXT_RSC_NAVIGATE__, and the resulting fetch rendered
into a root that was suspended on a never-resolving initial-elements
promise, so every navigation also hung. None of those globals should
exist when hydration was deliberately aborted.

Change readInitialRscStream's return type to ReadableStream | null and
have recoverFromBadInitialRscResponse return null on the abort path
(second attempt on the same path). main() now early-returns when the
stream is null, leaving only the server-rendered HTML visible and no
hydration globals for external code to observe.

Add a console.warn on the first attempt before window.location.reload().
One-shot (the sessionStorage flag keeps it from repeating) but makes
production reload behavior traceable rather than a mystery.

Pin the invariant that makes the hard-nav destination correct after a
server-side redirect hop: the inline redirect branch keeps currentHref
in sync with history across hops, so window.location.href = currentHref
on a !ok / !isFlight response is safe. Comment flags this for future
refactors.

Tighten the test's RSC stream-parse filter to require an RSC-context
co-marker for generic strings ("Connection closed", "Unexpected token"),
so a benign third-party JSON.parse diagnostic cannot false-positive.
Rewrite the hit-count comment to match the assertion (<= 2 is the real
bound; hydration timing can make the prefetch race yield 1 hit).

* fix(app-router): tighten RSC error hygiene around hydration and unload

- recoverFromBadInitialRscResponse now returns null from both branches so
  main() aborts the hydration bootstrap in the reload-pending case too. The
  never-resolving ReadableStream sentinel would let main() proceed past
  readInitialRscStream and register __VINEXT_RSC_ROOT__ / __VINEXT_RSC_NAVIGATE__
  during the brief window before the reload takes effect, briefly exposing
  a half-hydrated surface to external probes.
- Track isPageUnloading via a pagehide listener and suppress the
  "[vinext] RSC navigation error" diagnostic when the catch fires because
  a hard-nav or anchor click aborted an in-flight RSC fetch. Mirrors the
  Next.js isPageUnloading pattern — the page is already going away, so the
  log is just noise.

* test(app-router): tighten RSC fetch error suite

- Replace the fixed 1500ms stability wait with waitForLoadState("networkidle").
  Tracking actual request activity avoids flaky wall-clock behavior in CI; a
  reload loop keeps the network busy so the wait times out and still surfaces
  the regression.
- Drop the third test. Its "URL does not contain .rsc" assertion is strictly
  weaker than the first test's exact URL match on the non-.rsc destination.

* fix(app-router): reset isPageUnloading on pageshow for bfcache restore

Without a pageshow handler, a bfcache-restored document resumes with
isPageUnloading stuck at true, causing the navigation catch to silently
swallow every subsequent RSC error for the lifetime of that tab. Pair
the listeners to match Next.js' fetch-server-response.ts.

Also tighten the RSC fetch-errors spec with a >= 1 lower bound on the
.rsc hit count so a future regression that skips the client nav fetch
entirely still fails the test instead of trivially satisfying <= 2.

* refactor(app-router): unify recoveryFromBadInitialRscResponse return and fix test listener ordering

Collapse the two branches of recoverFromBadInitialRscResponse onto a
single return null at the end of the function, so the "always returns
null" invariant is structural rather than duplicated. A future refactor
that wanted to return a sentinel from one branch would have to move the
return, which is a visible edit — not a silent divergence.

Register the Playwright console listener before page.goto in both
rsc-fetch-errors specs so errors during initial page hydration are
captured, matching the invariant the tests rely on elsewhere.

* refactor(app-router): hard-nav to response URL and isolate hydration bootstrap

On an RSC nav that hits a non-ok/wrong-content-type response, prefer the
post-redirect response URL (navResponseUrl ?? navResponse.url) over the
original currentHref so a redirect-to-error chain lands the user directly
on the failing destination instead of bouncing off the redirect source
and re-following the same 3xx. Matches Next.js'
doMpaNavigation(responseUrl.toString()). Falls back to currentHref when
no response URL is available.

Extract the post-null-check body of main() into a synchronous
bootstrapHydration helper. The null-branch structurally cannot reach
any __VINEXT_RSC_* assignment now — a future refactor that interposes
async work between the stream read and the globals is a visible change
to the helper or its caller, not a silent invariant break.

Tighten the /about.rsc intercept glob to an anchored regex so a
hypothetical future self-prefetch from /about cannot inflate
aboutRscHits past the <= 2 bound. Note the dual-guard coverage
(status 500 + text/html) on the fulfill body so maintainers don't drop
either half.

* fix(app-router): add !body parity guard and preserve trailingSlash on hard-nav

Add the missing `!rscResponse.body` branch to readInitialRscStream so
the initial-hydration path guards on all three conditions Next.js checks
in fetch-server-response.ts (`!ok || !isFlightResponse || !body`), not
just the first two. A 200 with valid text/x-component but a null body
(e.g. 204-shaped responses, edge workers that return headers without
piping) now triggers the reload/abort recovery instead of parsing an
empty stream and throwing downstream.

Preserve the trailing slash when stripping .rsc from the hard-nav
target. toRscUrl normalizes `/foo/` to `/foo.rsc` before the fetch, so
the response URL loses the slash; stripping `.rsc` gave `/foo` and
sites with trailingSlash:true incurred an extra 308 to the canonical
`/foo/` form. Now restored when currentHref had one.

* feat(app-router): preserve hash on hard-nav and cover redirect-chain path

Preserve the fragment from the user's clicked href on the hard-nav
target — a .rsc response URL never carries a fragment, so dropping it
would silently strip /foo#section down to /foo. Small ergonomic win
over Next.js' doMpaNavigation, which has the same gap.

Add an e2e test for the redirect-chain hard-nav path (/redirect-src →
307 → /about.rsc → 500). Verifies the address bar lands on /about (the
post-redirect URL) rather than /redirect-src (the original request),
exercising the navResponseUrl ?? navResponse.url branch the previous
round introduced. Also pin the embedded-RSC assumption in the existing
500-route test by snapshotting the .rsc hit count before and after
networkidle and asserting no additional hits — so a future change that
makes the embed path conditional surfaces as a count change rather
than a networkidle timeout.

* chore(app-router): drop dead !body throw and tighten redirect-chain test

Remove the unreachable `if (!rscResponse.body) throw new Error(...)` at
the bottom of readInitialRscStream. The new !body guard added earlier
in the function returns via recoverFromBadInitialRscResponse before
this branch is reached, so the throw was stale belt-and-suspenders.

Tighten the redirect-chain e2e test:
- Capture every main-frame navigation URL via framenavigated and assert
  /redirect-src never appears, so a regression that dropped the
  navResponseUrl ?? navResponse.url branch is caught even though the
  final URL would still converge to /about via the server's 307 replay.
- Pin the Playwright redirect-intercept assumption — the test relies on
  Playwright re-entering the route table on the 307 follow-up so both
  handlers fire in sequence; a future mocker migration that follows
  redirects transparently would silently skip the /about.rsc intercept.

* fix(app-router): abort hydration when sessionStorage cannot persist reload guard

If sessionStorage is denied (strict-mode iframe, enterprise-locked
browser policy), the reload-loop guard's setItem silently no-ops and
every subsequent getItem returns null — turning the recovery path into
an infinite reload loop on a persistently broken .rsc endpoint. Verify
the write by reading it back and, if it didn't persist, abort hydration
immediately so the user sees the server-rendered HTML instead.

Hoist `new URL(currentHref, window.location.origin)` to a single
`origUrl` binding in the hard-nav target builder — the same href was
parsed twice for trailing-slash detection and hash extraction.

---------

Co-authored-by: Claude <[email protected]>
NathanDrake2406 pushed a commit that referenced this pull request May 15, 2026
…ecognizedActionError (cloudflare#1206)

* fix(shims): add unstable_catchError, unstable_rethrow, unstable_isUnrecognizedActionError

Unblocks 18 build failures in the Next.js deploy suite:

- `unstable_catchError` (12 failures) — added to `shims/error.tsx`. App
  Router error-boundary HOC ported from Next.js's
  `client/components/catch-error.tsx`. Implements the same
  `getDerivedStateFromError` predicate that re-throws Next.js navigation
  signals (redirect, notFound, …) and renders the user fallback with an
  `ErrorInfo` value otherwise. `unstable_retry` throws a clear "not yet
  implemented" error (tracked as follow-up).

- `unstable_rethrow` (3 failures) — added to `shims/navigation.ts` and
  re-exported from `shims/navigation.react-server.ts`. Ported from
  Next.js's `client/components/unstable-rethrow.{ts,server,browser}.ts`.
  Specialized to the internal-error categories vinext actually emits
  (redirect + HTTP access fallback); recurses through `error.cause` for
  wrapped errors.

- `unstable_isUnrecognizedActionError` (3 failures) — added to
  `shims/navigation.ts` along with the `UnrecognizedActionError` class.
  Ported 1:1 from `client/components/unrecognized-action-error.ts`. The
  react-server condition exports a throwing stub matching Next.js's
  `navigation.react-server.ts`.

Also adds `isRedirectError` and `isNextRouterError` helpers to
`navigation.ts` (used internally by `unstable_rethrow`) and updates
`next-shims.d.ts` so the new exports type-check for users.

* fix(shims): document isRedirectError divergence and dedupe error-boundary copy

Addresses two /bigbonk review observations on cloudflare#1206:

1. **Document the permissive prefix match.** The public `isRedirectError`
   in `shims/navigation.ts` does `digest.startsWith("NEXT_REDIRECT;")`
   while Next.js's `client/components/redirect-error.ts` does full
   4-segment validation (type ∈ {push, replace}, non-empty destination,
   status ∈ {303, 307, 308}). The divergence is intentional — vinext
   emits 3-part and 4-part digests whereas Next.js's validator targets
   its 5-part canary digests — but now that the predicate is reachable
   from the public `next/navigation` surface via `unstable_rethrow`, it
   gets a prominent JSDoc block explaining the difference, the
   consequence (malformed digests return true here, false in Next.js),
   and a link to the Next.js source.

2. **Consolidate duplicate `isRedirectError`.** `shims/error-boundary.tsx`
   had its own private copy with the same prefix-match logic. Replaced
   the duplicate with an import from `./navigation.js`, matching the
   pattern already used by `shims/error.tsx`
   (`import { isNextRouterError } from "./navigation.js"`). The
   `RedirectBoundary` call site casts to the file-local `RedirectError`
   type to preserve access to the optional `handled` field — this is
   safe by construction (every error matching the prefix predicate is
   produced by vinext's `redirect()` / `permanentRedirect()` helpers,
   which yield `Error` instances).

No behavior change. `vp check` + `vp test run tests/shims.test.ts
tests/error-boundary.test.ts` (916 tests) pass.

* fix(shims): drop unused getErrorDigest export

After consolidating shims/error-boundary.tsx onto the public
isRedirectError from shims/navigation.ts, the only remaining caller of
getErrorDigest is inside utils/navigation-signal.ts itself. Knip flags
the export as unused, which fails the Check job in CI.

Convert getErrorDigest to a file-local function. No behavior change.

* fix(shims): extend unstable_rethrow + make unstable_retry functional

Parity audit findings on PR cloudflare#1206 (post /bigbonk APPROVED) revealed two
gaps from a thorough re-read of Next.js's error-rethrow surface and
`unstable_catchError` test fixtures.

## 1. `unstable_rethrow` — added missing error categories

Next.js's server-side `unstable_rethrow` rethrows seven error categories
(client-side rethrows two). vinext was only handling #1
(`isNextRouterError`). The canonical user-facing fixture
(`test/e2e/app-dir/app-static/lib/fetch-retry.js`) breaks if any of the
other categories are not propagated.

Added the two categories vinext can plausibly encounter:

- `BailoutToCSRError` + `isBailoutToCSRError` — thrown by `next/dynamic`
  with `ssr: false`. Lives in shared (non-server) code so third-party
  libraries and accidentally-bundled Next.js internals can produce it.
  Ported 1:1 from
  https://github.com/vercel/next.js/blob/canary/packages/next/src/shared/lib/lazy-dynamic/bailout-to-csr.ts

- `DynamicServerError` + `isDynamicServerError` — thrown by Next.js's
  `cookies()`/`headers()` when called in a static render context. vinext
  doesn't construct it itself but exposes the class so user code, action
  wrappers, and accidental Next.js-internal bundles can interoperate.
  Ported 1:1 from
  https://github.com/vercel/next.js/blob/canary/packages/next/src/client/components/hooks-server-context.ts

The remaining four server-only categories (`isDynamicPostpone`,
`isPostpone`, `isHangingPromiseRejectionError`,
`isPrerenderInterruptedError`) are tied to Next.js's PPR / prerender
controller machinery that vinext does not implement. User code cannot
construct them in normal use; they're deferred as a follow-up
(documented inline). A test pins this intentional gap.

## 2. `unstable_catchError.unstable_retry` — made functional

The previous implementation threw a generic "not yet implemented" error
in both server and client contexts. Now matches Next.js's App Router
branch: on the client, calls `appRouterInstance.refresh()` inside
`React.startTransition` and resets the boundary. On the server, throws
a clear "client-only" error (refresh is meaningless during SSR setup).

Pages Router's exact error message (`unstable_retry() can only be used
in the App Router. Use reset() in the Pages Router.`) is not yet
dispatched because vinext's boundary doesn't read `PagesRouterContext`;
documented in the JSDoc as a parity follow-up.

## Test coverage

Added 8 new test cases covering:
- `BailoutToCSRError` + predicate (canonical digest, foreign-object
  detection, negative cases)
- `DynamicServerError` + predicate (same shape)
- `unstable_rethrow` propagates both new categories (re-throw identity)
- `unstable_rethrow` does NOT match the four uncovered server-only
  categories (pins the intentional gap)
- `unstable_catchError.getDerivedStateFromError` accepts `null` /
  `undefined` thrown values (ported from
  test/e2e/app-dir/catch-error/throw-null and throw-undefined fixtures)
- `unstable_retry` throws "client-only" on the server
- `unstable_retry` on the client calls `appRouterInstance.refresh()` +
  resets via setState

Also added `isRedirectError` / `isNextRouterError` / new predicates
JSDoc note that these are vinext-only extensions (Next.js does NOT
export them from `next/navigation` — confirmed by reading
`packages/next/src/client/components/navigation.ts` end-to-end).

`vp test run tests/shims.test.ts tests/error-boundary.test.ts
tests/app-router.test.ts tests/app-rsc-errors.test.ts` — 1274 tests
pass. `vp check` clean.

* fix(shims): drop redundant typeof check in isDynamicServerError

Bonk-review observation on cloudflare#1206: the predicate was checking
`typeof digest === "string" && digest === _DYNAMIC_SERVER_USAGE_DIGEST`,
where the `===` against a string literal already requires the operand
to be a string. Drop the redundant guard for consistency with
`isBailoutToCSRError` above.

Non-blocking cleanup; no behavior change.
james-elicx added a commit that referenced this pull request Jun 6, 2026
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 cloudflare#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 cloudflare#1798
(bonk review concern #1).
NathanDrake2406 added a commit that referenced this pull request Jun 6, 2026
…lare#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 cloudflare#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 cloudflare#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 cloudflare#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 cloudflare#1788.

---------

Co-authored-by: James <[email protected]>
NathanDrake2406 pushed a commit that referenced this pull request Jun 9, 2026
… context (cloudflare#1868)

* fix(app-router): extend OTel tracer provider after instrumentation register() for Cache Component spans

During Cache Component fallback resume, `workUnitAsyncStorage` carries a
prerender/cache store. Without the tracer extension, calls to
`tracer.startActiveSpan()` / `tracer.startSpan()` inside user RSC code
inherited that frozen context, causing span IDs to be reused across requests
or not created at all.

Mirrors Next.js's `extendInstrumentationAfterRegistration()` in
`instrumentation-node-extensions.ts`: wraps the OTel tracer provider's
`startSpan` and `startActiveSpan` to exit `workUnitAsyncStorage` when
creating spans, then re-enters the store for the `startActiveSpan` callback.

Fixes cloudflare#1495

* fix(otel-tracer-extension): address bonk review findings

BLOCKING: replace bare require("@opentelemetry/api") with globalThis.require
guard so the call is safe in ESM Worker bundles where bare require is
undefined; matches the pattern in client-trace-metadata.ts.

SHOULD-FIX #1: restore the isUseCacheFunction warning when a "use cache"
function is passed to startActiveSpan — mirrors upstream
instrumentation-node-extensions.ts. Adds USE_CACHE_FUNCTION_SYMBOL tag in
registerCachedFunction and the inline isUseCacheFn check in the extension
(avoids importing the full cache-runtime dep graph from this lightweight file).

SHOULD-FIX #2: add focused unit tests in tests/otel-tracer-extension.test.ts
covering no-op paths, startSpan ALS exit, startActiveSpan ALS re-entry,
use-cache warning, provider/tracer double-wrap guards, and provider-swap
re-wrapping. Isolated from the full vinext plugin import graph so they run
without optional build-time packages.

SHOULD-FIX #3: replace module-global boolean tracerProviderExtended with a
WeakSet<object> (extendedProviders) so that a swapped provider gets wrapped
rather than being silently skipped forever.

NIT: correct the upstream filename in the instrumentation-runtime.ts comment
from instrumentation-globals.external.ts to instrumentation-node-extensions.ts
(where afterRegistration() is actually defined).

* refactor(cache-runtime): drop dead isUseCacheFunction export

Nothing imported `isUseCacheFunction` — `otel-tracer-extension.ts`
already keeps its own inline copy (`isUseCacheFn`) with a comment
explaining why (avoids pulling heavy cache-runtime deps into the
lightweight server helper).  Remove the unused export to eliminate the
dead code flagged in review.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants