chore: sync main with upstream/main#1
Merged
Conversation
Co-authored-by: tomoconstrutor <[email protected]>
…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
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
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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