Skip to content

fix: exclude vinext from optimizeDeps to prevent virtual module resolution errors#3

Merged
southpolesteve merged 2 commits into
mainfrom
fix/exclude-vinext-from-optimizedeps
Feb 24, 2026
Merged

fix: exclude vinext from optimizeDeps to prevent virtual module resolution errors#3
southpolesteve merged 2 commits into
mainfrom
fix/exclude-vinext-from-optimizedeps

Conversation

@threepointone

Copy link
Copy Markdown
Contributor

Problem

When vinext is installed from npm (not symlinked), vite dev fails with:

✘ [ERROR] Could not resolve "virtual:vinext-rsc-entry"

esbuild's dependency optimization scans vinext/dist/ and hits virtual:vinext-* imports that only exist at Vite plugin resolution time. This occurs in all three environments (client, rsc, ssr).

Fix

Adds optimizeDeps.exclude: ["vinext"] at the top level and in each environment config (rsc, ssr, client). This prevents esbuild from scanning vinext's dist files during pre-bundling.

Uses the stable optimizeDeps.exclude API rather than an esbuild plugin, so it won't break in future Vite majors.

Fixes #1

…ution errors

When vinext is installed from npm (not symlinked), esbuild's dependency
optimization scans vinext/dist/ and hits virtual:vinext-* imports that
only exist at Vite plugin resolution time. This causes build failures
in all three environments (client, rsc, ssr).

Adding optimizeDeps.exclude: ['vinext'] at the top level and in each
environment prevents esbuild from scanning vinext's dist files entirely.

Fixes #1
@github-actions

Copy link
Copy Markdown
Contributor
Example Preview Production Original
app-router-cloudflare preview production
pages-router-cloudflare preview production
app-router-playground preview production original
realworld-api-rest preview production
nextra-docs-template preview production
benchmarks preview production
hackernews preview production original

@southpolesteve

Copy link
Copy Markdown
Collaborator

/bigbonk give this a review

@southpolesteve
southpolesteve merged commit 3ed38d1 into main Feb 24, 2026
17 checks passed
@southpolesteve
southpolesteve deleted the fix/exclude-vinext-from-optimizedeps branch February 24, 2026 17:13
southpolesteve added a commit that referenced this pull request Feb 27, 2026
…mmands, COST.md

- Benchmarks: Rollup 2.4x faster, Rolldown 6.2x faster, 55-56% smaller bundles
- "use cache" now marked Full (was "Not yet") — directive, cacheLife, cacheTag all work
- Added vinext deploy and vinext check to CLI reference
- Updated vinext deploy section from "coming soon" to actual capabilities
- Added connection() and static export as Full features
- Test counts: 844 vitest + 278 Playwright E2E
- COST.md: 207 commits, ~45K source lines, ~25K test lines
- Downgraded PPR (#3) from P1 to P2 (still experimental in Next.js)
james-elicx referenced this pull request in NathanDrake2406/vinext 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 referenced this pull request in NathanDrake2406/vinext Mar 16, 2026
james-elicx pushed a commit that referenced this pull request Mar 16, 2026
* Add production coverage for global-error compat

* Address global-error review feedback

* docs: update tracker notes for tests #3 and #4 to reflect production preview coverage
github-actions Bot pushed a commit that referenced this pull request Apr 27, 2026
… gate

Addresses bonk PR #916 review item #3 (carried over). Extracts
'peerDisconnectCode' as an exported pure predicate so the matching
logic can be tested in isolation without process-state mutation, and
adds a thin 'isSocketErrorBackstopInstalled()' query so the test can
verify the Vitest install-gate short-circuit fires in worker processes.

Tests cover:
- ECONNRESET / EPIPE / ECONNABORTED accepted, other codes rejected
- non-Error / null / primitive reasons handled (unhandledRejection
  fires with arbitrary reason values)
- Install gate skips Vitest workers (process.env.VITEST === 'true')

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
james-elicx pushed a commit that referenced this pull request Apr 27, 2026
… gate

Addresses bonk PR #916 review item #3 (carried over). Extracts
'peerDisconnectCode' as an exported pure predicate so the matching
logic can be tested in isolation without process-state mutation, and
adds a thin 'isSocketErrorBackstopInstalled()' query so the test can
verify the Vitest install-gate short-circuit fires in worker processes.

Tests cover:
- ECONNRESET / EPIPE / ECONNABORTED accepted, other codes rejected
- non-Error / null / primitive reasons handled (unhandledRejection
  fires with arbitrary reason values)
- Install gate skips Vitest workers (process.env.VITEST === 'true')

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
james-elicx added a commit that referenced this pull request Apr 27, 2026
…e events can't remove it (#916)

* debug(dev): hoist socket-error backstop to module top-level + opt-in trace

Two changes intended to pin down why the process-level handler from
PR #913 still doesn't catch the ECONNRESET trace some users report:

1. Move installation from inside configureServer to module top-level,
   guarded by Symbol.for to prevent double-install. Earlier versions
   tied teardown to httpServer 'close', which Vite emits on dep
   re-optimization, full reloads, and other lifecycle events — leaving
   a window where the listener is absent when a stale stream errors.
   Module-level install + Symbol guard removes that window entirely.

2. Add an opt-in console.warn marker behind
   VINEXT_DEBUG_SOCKET_ERRORS=1 that fires whenever the listener
   absorbs a peer-disconnect error. The visible
   `node:events:487 throw er; // Unhandled 'error' event` output
   otherwise leaves it ambiguous whether the listener never ran
   (install / lifecycle issue) or ran but was somehow bypassed
   (different async surface / dispatcher). The marker disambiguates.

No behavior change for users not setting the env flag beyond the
hoisting. Filter codes and re-throw shape are unchanged from #913.

Refs #905.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* fix: address bonk review on dev socket-error backstop

Three issues from PR #916 review:

1. Module-top-level install was wrong — it ran during 'vinext build',
   'vp pack', and Vitest worker imports too, so the JSDoc 'dev-only'
   claim was inaccurate and a real ECONNRESET from a pre-render fetch
   in CI would have been silently swallowed. Move the install back into
   configureServer (which only fires in dev) but **without** binding
   teardown to httpServer 'close' — that was the original lifecycle bug
   PR #913 had. Symbol.for guard makes re-invocation a no-op so the
   listener still survives server restarts within a session.

2. Restore the middleware-mode skip dropped in the previous commit.
   Embedders running vinext in middleware mode (Express/Connect) keep
   ownership of their own process-level handlers. Gated on
   server.httpServer, matching the prior PR #913 behavior.

3. Document the listener-ordering implication of installing during
   configureServer (registers late in the queue, so Sentry / structured
   logging / test-runner hooks still observe non-peer-disconnect errors
   before vinext's sync re-throw aborts iteration), and the Symbol.for
   first-loaded-wins caveat for the multi-version case.

Refactor: collapse the duplicated `(err as Error & { code }).code` casts
into a single `peerDisconnectCode` helper that returns the matched code
or undefined, so the debug log path reuses it without recasting.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* fix(dev): re-hoist socket-error backstop to module load (configureServer too late)

The previous attempt to address bonk's review by moving install back
into configureServer was wrong — it didn't restore working behavior:

  $ VINEXT_DEBUG_SOCKET_ERRORS=1 vp dlx vinext@455a61b dev
  ...
  GET /stage/oliver 200 in 1.1s
  node:events:487
        throw er; // Unhandled 'error' event
  Error: read ECONNRESET
  ...

No `[vinext] dev socket-error backstop installed` startup line. The
function never ran. Cause: in vite-plus's plugin lifecycle,
`server.httpServer` is null at the moment configureServer fires, so
the `if (server.httpServer)` middleware-mode guard skipped install.
Requests still get served (httpServer is created later) but the
listener was never attached.

Hoist back to module load. To address bonk's "loaded in build/test
contexts too" concern, gate install on:

  - `process.env.VITEST === "true"`  → Vitest worker imports
  - `process.argv` contains "build"  → `vinext build` / `vp build`

`process.argv` is read because the CLI entry imports index.ts before
it has a chance to set an env var, and `configResolved` runs too late
for module-load gating.

Updated JSDoc to be honest about the install context, the listener-
ordering implication, the middleware-mode embedder situation, and the
Symbol.for first-loaded-wins caveat.

Refs #905.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* refactor(dev): positive-gate socket backstop on argv[2]==='dev'

Replace the negative VITEST/build skip list with a single positive
check: install only when argv[2] === "dev". Matches `vinext dev`,
`vp dev`, `vite dev`, and any CLI following the "<bin> dev"
convention. Everything else — build, Vitest workers, library
embedders with a custom runner — skips install, so genuine
peer-disconnect errors surface normally in those contexts.

Cleaner default: no listener unless we're confident this is a dev
server invocation.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* refactor(dev): gate socket backstop on Vite's command==='serve'

Replace argv-sniffing with the canonical signal: Vite's `config()`
hook receives `{ command: "serve" | "build" }` directly. Install
the backstop only when `command === "serve"`. Works identically for
`vinext dev`, `vp dev`, `vite dev`, and library embedders that call
`createServer` themselves — anywhere Vite considers itself a dev
server.

`config()` runs before `configureServer` (so before httpServer
matters) and before the dep-optimization or full-reload events that
broke the earlier `httpServer.close`-tied teardown. Symbol.for guard
keeps the install idempotent across server restarts within a session.

Removes:
  - Module-load-time install + argv[2] === "dev" check
  - VITEST / build env-var negative skip list

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* fix(dev): narrow gate to !isPreview so vite preview skips install

`command === "serve"` covers both `vite dev` and `vite preview` (the
post-build static server). Preview doesn't stream RSC and doesn't
need this guard, so narrow to `command === "serve" && !env.isPreview`.

`vinext start` runs prod-server.ts directly without Vite, so it was
already correctly excluded — `config()` never fires there.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* fix: extend socket-error backstop to vinext start (Next.js parity)

Next.js installs uncaughtException + unhandledRejection handlers in
their router-server unconditionally — no dev/prod gate. Both the
Node.js dev server and the Node.js prod server share the same
process-level error guards. See vercel/next.js
packages/next/src/server/lib/router-server.ts:809-810.

vinext was only guarding the dev path. `vinext start` runs
prod-server.ts directly as a Node HTTP server (used for self-hosted
deploys; Cloudflare Workers prod doesn't load this module — the
runtime owns socket lifecycle there) and had the same theoretical
exposure to peer-disconnect crashes through the pipeline() / fetch()
paths it streams responses through.

Refactor:
  - Extract the install function from index.ts into a new
    src/server/socket-error-backstop.ts (drop the "Dev" prefix —
    no longer dev-only).
  - Keep the call from the vinext:config plugin's config() hook,
    gated on command === "serve" && !isPreview (covers vinext dev /
    vp dev / vite dev / library embedders).
  - Add a parallel call at the top of startProdServer() in
    prod-server.ts (covers vinext start).

vinext is more conservative than Next.js's log-only handler — we
filter strictly on peer-disconnect codes and sync re-throw the rest,
so genuine bugs still surface. The parity is in *where* we install,
not what we swallow.

Vitest workers and `vinext build` never reach either entry point, so
peer-disconnect errors in those contexts surface normally.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* fix(dev): auto-install socket-error backstop at module load

The Vite config() hook gate (`command === "serve" && !isPreview`)
proved unreliable in vite-plus's lifecycle — the hook didn't fire
(or didn't pass command correctly), and install was silently skipped.
Field reproducer: published 501dd95 had the call site in the bundle
at the right spot, but `VINEXT_DEBUG_SOCKET_ERRORS=1` produced no
startup marker, immediate ECONNRESET crash on first request.

Move the install back to module load — invoked unconditionally as a
side effect when socket-error-backstop.ts is imported. This was the
last shape verified to work via the diagnostic flag.

Drop the import-and-call pattern in index.ts in favor of a
side-effect import (`import "./server/socket-error-backstop.js"`),
which is enough to trigger the auto-install. The import order keeps
the install ahead of every other vinext server code path.

Skip in Vitest workers via `process.env.VITEST === "true"` so
genuine peer-disconnect errors during test runs surface normally.
Build runs are unaffected (short-lived, no peer-disconnect-prone
streams) — matches Next.js's pattern of installing in any
HTTP-serving entry without further gating.

prod-server.ts still calls installSocketErrorBackstop() explicitly
for `vinext start`. Idempotent via Symbol.for guard.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* refactor: call installSocketErrorBackstop directly in index.ts

Remove the side-effect import + auto-install indirection. Just call
the function explicitly at module top-level in index.ts, where it's
obvious. Same observable behavior, clearer code.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* fix: gate socket backstop on NODE_ENV to keep prerender ECONNRESET fatal

Bonk's PR #916 review caught a real corruption bug: the build path
calls startProdServer() during prerender (build/run-prerender.ts:181,
build/prerender.ts:398, :720), so the install fires twice during a
build — once at index.ts module load, once inside startProdServer.
User fetch() calls inside prerendered pages can hit ECONNRESET from
flaky upstream APIs. With the backstop installed, those errors are
silently absorbed instead of crashing the build, producing corrupt
prerendered HTML/RSC output.

Add NODE_ENV gating inside installSocketErrorBackstop:
  - Skip if NODE_ENV === "production"  → covers vinext build + prerender
  - Skip if NODE_ENV === "test"        → covers test runners that follow convention
  - Skip if VITEST === "true"          → kept for Vitest specifically

Vite sets NODE_ENV=production for the build command before plugins
load, so the gate fires correctly. Trade-off: vinext start with
NODE_ENV=production set in shell will also skip install — losing
strict Next.js parity for that path. Acceptable: prod-server's
pipeline() callbacks already handle the streaming case, and the
real-world bug reports are all dev-server.

Also fix the now-stale comment at prod-server.ts:817 that still
referenced the reverted "Vite plugin config() hook" install path,
and document the listener-ordering trade-off more honestly in the
JSDoc — `index.ts` imports synchronously at the top of every user's
vite.config.ts, registering vinext's listener earlier than user /
tooling crash reporters. Sync re-throw on non-peer-disconnect errors
still surfaces the crash but later-registered observers don't see
the event; users who need crash-reporter visibility for those errors
must register their handler before importing vinext.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* fix: bypass socket backstop during prerender via VINEXT_PRERENDER

Replace the install-time NODE_ENV gate (which broke vinext start
parity by skipping installation entirely when NODE_ENV=production)
with a fire-time VINEXT_PRERENDER check inside the listener. The
listener is always installed, but during prerender it re-throws all
errors unconditionally — acting as if no listener were present —
so user fetch() ECONNRESETs during prerender crash the build with
the original stack instead of being silently absorbed into corrupt
prerendered output.

The fire-time check (vs. install-time) is necessary because
index.ts loads at Vite plugin import — well before prerender begins
— and the Symbol.for guard then makes any later install call a
no-op. A static install-time gate can't catch the prerender phase
that follows.

Set VINEXT_PRERENDER=1 in run-prerender.ts at the top of
runPrerender() so the flag covers the entire prerender orchestration
including startProdServer setup. prerender.ts already sets the same
var around its actual render passes; this widens the scope.

Restores Next.js-parity install for vinext start (NODE_ENV gate is
gone). Test-runner skip stays install-time on VITEST / NODE_ENV=test
since those contexts genuinely shouldn't have the listener.

Refs PR #916 review feedback.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* fix: address bonk PR #916 follow-up review

Three issues bonk flagged after the prerender fix landed:

1. Stale comment in prod-server.ts:816-820 — described the NODE_ENV
   self-gate that was reverted in b3e8759. Replace with one that
   describes the current behavior: idempotent install via Symbol.for
   guard, kept here for entry points that load prod-server without
   index.ts (Next.js parity), prerender bypass is fire-time via
   VINEXT_PRERENDER not install-time.

2. prerender.ts's finally blocks `delete VINEXT_PRERENDER` clobbers
   any value set by the caller. After prerenderApp returns inside
   runPrerender (which sets the flag for the whole orchestration),
   the var is deleted instead of restored to runPrerender's "1",
   leaving narrow gap windows around shared-server close where
   ECONNRESET would be absorbed instead of re-thrown. Save the
   prior value and restore it. Doesn't cause prerender output
   corruption (rendering already complete by these gaps) but it's
   the correct hygiene.

3. Document the orchestrator-induced ECONNRESET trade-off in the
   backstop's JSDoc — unconditional re-throw during prerender
   means a hung-route + orchestrator timeout surfaces the build
   crash as ECONNRESET rather than the route's own error.
   Acceptable but worth calling out so debuggers know to set
   VINEXT_DEBUG_SOCKET_ERRORS=1 to disambiguate.

Plus minor JSDoc wording nit: the "Symbol.for guard prevents
re-install" line was load-bearing in the wrong way — the relevant
point is timing, not de-dup. Reworded.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* fix: address bonk PR #916 follow-up review (round 2)

- run-prerender.ts: save/restore VINEXT_PRERENDER to avoid leaking the
  env var into Vitest workers (mirrors prerender.ts pattern).
- index.ts: correct stale install-site comment — install gate is
  Vitest-only; build/prerender bypass is fire-time via VINEXT_PRERENDER.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* test: focused unit test for socket-error-backstop predicate + install gate

Addresses bonk PR #916 review item #3 (carried over). Extracts
'peerDisconnectCode' as an exported pure predicate so the matching
logic can be tested in isolation without process-state mutation, and
adds a thin 'isSocketErrorBackstopInstalled()' query so the test can
verify the Vitest install-gate short-circuit fires in worker processes.

Tests cover:
- ECONNRESET / EPIPE / ECONNABORTED accepted, other codes rejected
- non-Error / null / primitive reasons handled (unhandledRejection
  fires with arbitrary reason values)
- Install gate skips Vitest workers (process.env.VITEST === 'true')

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
Co-authored-by: ask-bonk[bot] <ask-bonk[bot]@users.noreply.github.com>
james-elicx added a commit that referenced this pull request May 21, 2026
* fix(basepath): enforce scoping on rewrites/redirects/routes

Sub-issues addressed under #1333:

1. Rewrites/redirects/headers were firing on requests outside the
   configured `basePath`. Threaded a `BasePathMatchState` through the
   matchers and prod-server / deploy worker / dev plugin so default
   rules (no `basePath: false` opt-out) only evaluate when the request
   was under basePath, and opt-out rules only evaluate outside it.

5. Pages Router requests that landed outside basePath fell through to
   internal route matching. After redirects and beforeFiles rewrites
   run, the request now 404s when it was outside basePath and no
   `basePath: false` rule rewrote it — matching Next.js's
   `resolve-routes.ts:304-309`.

4. With rewrites no longer firing on out-of-basepath requests, the
   downstream code that prepends basePath to destinations no longer
   sees a stripped-but-already-internal pathname, eliminating the
   `/docs/docs/...` doubling for the rewrite-driven cases.

App Router uses the same gating; `basePath: false` rules don't yet
fire there because `normalizeRscRequest` 404s out-of-basepath requests
before the matchers see them — flagged as follow-up in the rule
gating site. External rewrite proxying (sub-issue #2) and static
asset doubling (sub-issue #3, partly addressed by #1337/#1383) are
deferred.

Refs #1333

* test: update deploy.test signature assertions and format basepath tests

* review: address bonk nits — add inverse basePath:false tests and TODO comment
james-elicx added a commit that referenced this pull request Jun 6, 2026
* 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 <[email protected]>
james-elicx added a commit that referenced this pull request Jun 9, 2026
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).
james-elicx added a commit that referenced this pull request Jun 9, 2026
… context (#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 #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.

Virtual module imports break esbuild dependency optimization when vinext is installed from npm

2 participants