fix: exclude vinext from optimizeDeps to prevent virtual module resolution errors#3
Merged
Merged
Conversation
…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
Contributor
|
Collaborator
|
/bigbonk give this a review |
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'>
This was referenced Mar 15, 2026
NathanDrake2406
referenced
this pull request
in NathanDrake2406/vinext
Mar 16, 2026
…preview coverage
james-elicx
pushed a commit
that referenced
this pull request
Mar 16, 2026
8 tasks
4 tasks
Merged
3 tasks
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>
This was referenced May 16, 2026
5 tasks
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.
This was referenced Jun 15, 2026
This was referenced Jun 25, 2026
This was referenced Jul 3, 2026
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.
Problem
When vinext is installed from npm (not symlinked),
vite devfails with:esbuild's dependency optimization scans
vinext/dist/and hitsvirtual: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.excludeAPI rather than an esbuild plugin, so it won't break in future Vite majors.Fixes #1