fix(build): preserve server export conditions

[james-elicx]

Summary

• remove the browser export condition from every vinext rsc and ssr environment, regardless of deployment adapter
• apply the same filtering to optimize-deps Rolldown resolution so dependency optimization and production resolution select the same server export
• preserve all other host-specific conditions and leave the client environment unchanged

This is the narrowly scoped production-code extraction of vinext:server-conditions from #2877. Environment identity is the contract: vinext rsc and ssr graphs render server code, so filtering does not depend on detecting the Cloudflare plugin.

Why

A deployment adapter can add browser to its default resolution conditions, but a vinext rsc or ssr graph must not select a package browser export. Doing so diverges from Next.js server resolution and can execute a client-only implementation during SSR. The filter removes only browser; conditions for the active host remain available before resolution falls through to import.

Tests

The behavioral ESM-externals and real workerd tests intentionally remain in #2877, as requested. This extraction does not duplicate or move them.

Validation on this branch:

• vp run vinext#build
• vp run check
• git diff --check

Related: #2877

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/@vinext/cloudflare@2918

npm i https://pkg.pr.new/create-vinext-app@2918

npm i https://pkg.pr.new/@vinext/types@2918

npm i https://pkg.pr.new/vinext@2918

commit: ef43f42

[github-actions]

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
workers-cache	preview	production
web	preview	production

[james-elicx]

/bigbonk review for issues

[github-actions]

Performance benchmarks

Compared ef43f42 against base 88ea15a using alternating same-runner rounds. Next.js was unchanged and skipped.

1 improved · 0 regressed · 5 within ±1.5%

Scenario	Framework	Baseline	Current	Change
Client bundle size (gzip)	vinext	135.4 KB	135.4 KB	⚫ -0.0%
Client entry size (gzip)	vinext	122.8 KB	122.8 KB	⚫ 0.0%
Dev server cold start	vinext	3.06 s	3.01 s	🟢 -1.7%
Production build time	vinext	3.16 s	3.14 s	⚫ -0.5%
RSC entry closure size (gzip)	vinext	114.8 KB	114.9 KB	⚫ +0.0%
Server bundle size (gzip)	vinext	195.2 KB	195.2 KB	⚫ +0.0%

View detailed results and traces

_{🟢 improvement · 🔴 regression · ⚫ change below 1.5% · paired base/head}

[ask-bonk]

Review: fix(build): preserve server export conditions (#2918)

I verified the core mechanism end-to-end and it is correct. Summary below, plus a few non-blocking notes.

Verified correct

1. The bug is real. @cloudflare/vite-plugin@1.31.0 sets defaultConditions = ["workerd", "worker", "module", "browser"] and applies it to the Worker environments via its config hook:

   • resolve.conditions = [...defaultConditions, "development|production"] (index.mjs:25438)
   • optimizeDeps.rolldownOptions.resolve.conditionNames = [...defaultConditions, "development"] (index.mjs:25480)

   Both include browser, so a vinext RSC/SSR graph could select a package's browser export during SSR — diverging from Next.js server resolution. The filter removes exactly browser and leaves workerd/worker/module/development(|production) intact, so resolution falls through to import. Correct.

2. Environment targeting is right. The Cloudflare worker environment in vinext apps is named rsc with ssr as a child (see examples/app-router-cloudflare/vite.config.ts: viteEnvironment: { name: "rsc", childEnvironments: ["ssr"] }). The Cloudflare plugin applies conditions to exactly the environments in environmentNameToWorkerMap, which are these same names. So the name !== "rsc" && name !== "ssr" guard matches precisely the environments that received browser.

3. Hook ordering is right. The Cloudflare plugin injects conditions through its config hook (environments: getEnvironmentsConfig(...), index.mjs:25619), not configEnvironment. Vite merges config-hook output before running per-environment configEnvironment, so by the time this enforce: "post" configEnvironment runs, config.resolve.conditions already contains the merged browser. The filter sees and removes it.

4. Mutation is safe. Mutating config in place in configEnvironment is an established pattern here (see the hasCloudflarePlugin block at index.ts:3411). The PR reassigns new filtered arrays (not in-place array mutation), so no shared/frozen array is mutated. The non-null assertions on config.optimizeDeps!.rolldownOptions!.resolve!.conditionNames are guarded by the preceding optional-chained optimizerConditions?.includes("browser"), so they can't throw.

5. No apply guard is correct. SSR resolution divergence and dep-optimizer resolution both matter in dev, so running in serve + build is intended. No downstream code reads resolve.conditions, so removing browser has no other side effects. vp check passes on the changed file.

Non-blocking notes

1. Missing unit test in this extraction. The PR states behavioral tests "intentionally remain in #2877," but this repo already has a well-established pattern for unit-testing configEnvironment hooks in isolation without a real runtime — see tests/compiler-define.test.ts and tests/client-global-define.test.ts, which locate the plugin by name and call plugin.configEnvironment!("rsc", config, { command }) against a plain config object. A focused test here would assert:

   • rsc/ssr with conditions: [..., "browser"] → browser removed, all others preserved
   • same for optimizeDeps.rolldownOptions.resolve.conditionNames
   • client and a no-browser config → untouched (no-op)

   This is cheap, requires no workerd, and guards against a future adapter/Vite change silently reintroducing browser. Since the fix is trivial to break silently, I'd recommend adding it in this PR rather than deferring to #2877.

2. Name/comment now slightly misleading. Commit 2 removed the hasCloudflarePlugin gate so the filter applies to any adapter that injects browser (per the PR's "regardless of deployment adapter" intent). But the plugin is still named vinext:cloudflare-server-conditions and the comment frames it around the RSC/SSR contract generically. Consider renaming to something adapter-neutral (e.g. vinext:server-export-conditions) to match the generalized behavior. Cosmetic.

3. Rolldown-only optimizer branch. The optimizer filter only handles optimizeDeps.rolldownOptions.resolve.conditionNames, not the esbuild branch (optimizeDeps.esbuildOptions.conditions) that the Cloudflare plugin still emits when !isRolldown. That's correct for this repo (Vite 8 bundles Rolldown, isRolldown is always true here), but it's an implicit coupling to the Rolldown path. A one-line comment noting the esbuild branch is intentionally not handled would help future readers.

Overall: the fix is correct and safe. My only real recommendation is adding the focused configEnvironment unit test given the established pattern makes it near-free and it's the exact kind of regression that would slip through silently.

[ask-bonk]

Review posted to PR #2918.

Summary

I verified the fix end-to-end against the actual @cloudflare/vite-plugin@1.31.0 source and the vinext plugin structure. The fix is correct and safe.

Verified correct:

• The bug is real — the Cloudflare plugin's defaultConditions includes browser and applies it to both resolve.conditions and optimizeDeps.rolldownOptions.resolve.conditionNames on the worker environments.
• Environment targeting (rsc/ssr) is precise — those are exactly the environment names the Cloudflare plugin maps to workers (viteEnvironment: { name: "rsc", childEnvironments: ["ssr"] }).
• Hook ordering is correct — the Cloudflare plugin sets conditions via the config hook, which merges before configEnvironment runs, so the enforce: "post" filter sees and removes browser.
• Mutation pattern matches existing code; non-null assertions are properly guarded; vp check passes.

Non-blocking notes I raised:

1. Missing unit test — the PR defers tests to fix(build): support Pages Router ESM externals #2877, but the repo already has an established pattern (tests/compiler-define.test.ts) for unit-testing configEnvironment hooks with no runtime needed. A focused test here is near-free and guards against silent regression.
2. Name/comment mildly misleading — commit 2 generalized the filter to all adapters, but the plugin is still named vinext:cloudflare-server-conditions.
3. Rolldown-only optimizer branch — the esbuild branch isn't handled (correct for this Vite 8 repo, but worth a comment noting the intentional coupling).

No blocking issues found.

github run

[james-elicx]

/bigbonk review for issues

[ask-bonk]

Review: fix(build): preserve server export conditions (#2918)

I re-verified this independently against the vendored @cloudflare/vite-plugin@1.31.0 source, the vinext plugin structure, and the existing configEnvironment mutation pattern. The fix is correct and safe. I concur with the prior review; adding a few points it didn't cover.

Verified correct

1. The bug is real and the source matches. dist/index.mjs defines defaultConditions = ["workerd", "worker", "module", "browser"] (25410) and applies it to the worker environments in createCloudflareEnvironmentOptions:

   • resolve.conditions = [...defaultConditions, "development|production"] (25438)
   • optimizeDeps.rolldownOptions.resolve.conditionNames = [...defaultConditions, "development"] (25480)

   Both include browser, so a vinext RSC/SSR graph could pick a package's browser export during SSR. The filter removes exactly browser and leaves workerd/worker/module/development(|production) intact, so resolution falls through to import. Correct.

2. Hook ordering is right. The Cloudflare plugin injects conditions through its config hook (environments: getEnvironmentsConfig(...), 25619), not configEnvironment. Vite merges config-hook output into each per-environment config before running configEnvironment, so this enforce: "post" configEnvironment observes the merged browser and removes it.

3. In-place mutation + return null is the established, working pattern here — not just a stylistic match. The existing configEnvironment at index.ts:3449-3451 mutates config.resolve.extensions in place and returns null; Vite passes the actual per-environment config object by reference, so the mutation propagates. The new plugin follows this exactly and reassigns fresh filtered arrays (no in-place array mutation of a possibly-shared array), so it can't corrupt state shared across environments. Non-null assertions are guarded by the preceding optimizerConditions?.includes("browser") check.

4. Environment targeting is architecturally guaranteed, not incidental. vinext's own init/validation code hard-requires name: "rsc" with childEnvironments: ["ssr"] for the Cloudflare worker env (init-cloudflare.ts:1023,1053,1079; utils/project.ts:288). So the name !== "rsc" && name !== "ssr" guard matches precisely the environments an adapter would have populated with browser.

5. The fix is consistent with vinext's existing design intent. vinext already documents that browser export conditions are wrong for server code — see the client-optimizer exclude comment at index.ts:3265, which describes the exact failure mode (file-type resolving to core.js instead of the node-export index.js). This PR generalizes that same protection to conditions injected by deployment adapters. No vinext server environment sets browser intentionally, so there's no false-positive strip.

6. vp check packages/vinext/src/index.ts passes (format, lint, type). No test references the plugin name, so the rename suggestion below is non-breaking.

Non-blocking notes

1. Missing unit test in this extraction (strongest recommendation). The PR defers behavioral tests to #2877, but the repo already has a near-free pattern for unit-testing configEnvironment in isolation — see tests/compiler-define.test.ts:159-161 (plugin.configEnvironment!("rsc", config, { command }) against a plain object, no runtime). A focused test would assert: rsc/ssr with conditions: [..., "browser"] → browser removed and all others preserved (same for optimizeDeps.rolldownOptions.resolve.conditionNames); client and a no-browser config → untouched. This exact class of regression (a future adapter/Vite bump silently reintroducing browser) would otherwise slip through completely silently. I'd add it here rather than deferring.

2. Plugin name is now misleading. Commit 2 (filter browser from all server graphs) dropped the hasCloudflarePlugin gate so the filter is adapter-neutral, but the plugin is still vinext:cloudflare-server-conditions. Rename to something like vinext:server-export-conditions. Cosmetic; no tests depend on the name.

3. Rolldown-only optimizer branch is an implicit coupling. The optimizer filter handles only optimizeDeps.rolldownOptions.resolve.conditionNames, not the esbuild branch (optimizeDeps.esbuildOptions.conditions) that the Cloudflare plugin still emits when !isRolldown (25491-25497). That's correct for this Vite 8 repo (isRolldown is always true), but a one-line comment noting the esbuild branch is intentionally unhandled would help future readers — and would pair well with the AGENTS.md guidance to treat esbuild config as a migration target, not a pattern to copy forward.

Verdict

Correct, safe, and consistent with vinext's existing server-resolution design. No blocking issues. The single recommendation I'd act on before merge is adding the focused configEnvironment unit test, since the pattern makes it trivial and it guards a silent-regression path.

[ask-bonk]

Review posted to PR #2918.

Summary

I independently verified the fix end-to-end against the vendored @cloudflare/vite-plugin@1.31.0 source, the vinext plugin structure, and the existing configEnvironment patterns. The fix is correct and safe — no blocking issues.

Verified correct:

• The bug is real — defaultConditions in the Cloudflare plugin includes browser, applied to both resolve.conditions (index.mjs:25438) and optimizeDeps.rolldownOptions.resolve.conditionNames (index.mjs:25480).
• Hook ordering works — the plugin sets conditions via config hook (25619), which merges before the enforce: "post" configEnvironment runs.
• In-place mutation + return null is the established working pattern (index.ts:3449-3451), and the PR reassigns fresh arrays so no shared array is corrupted.
• Environment targeting (rsc/ssr) is architecturally guaranteed by vinext's own init/validation code, not incidental.
• The fix aligns with vinext's existing design intent — the client-optimizer comment at index.ts:3265 already documents why browser conditions are wrong for server code (file-type → core.js failure mode).
• vp check passes.

Non-blocking notes (I concur with the prior review and added depth):

1. Missing unit test — the repo has a near-free pattern (tests/compiler-define.test.ts:159) for testing configEnvironment in isolation. This is the one thing I'd recommend acting on before merge, since a future adapter/Vite bump reintroducing browser would slip through silently.
2. Plugin name misleading — commit 2 made it adapter-neutral but it's still named vinext:cloudflare-server-conditions.
3. Rolldown-only optimizer branch — the esbuild branch isn't handled (correct for Vite 8, but worth a comment).

github run

[james-elicx]

/bigbonk review for issues

[ask-bonk]

Review: fix(build): preserve server export conditions (#2918)

I verified the fix against the vendored @cloudflare/vite-plugin@1.31.0 source, the vinext plugin structure, and the existing configEnvironment patterns. The fix is correct and safe — no blocking issues.

Verified correct

1. The bug is real. dist/index.mjs defines defaultConditions = ["workerd", "worker", "module", "browser"] (25410) and applies it to both resolve.conditions (25438) and optimizeDeps.rolldownOptions.resolve.conditionNames (25480) on the worker environments. Both include browser, so a vinext RSC/SSR graph could select a package's browser export during SSR. The filter removes exactly browser and leaves everything else intact; resolution falls through to import, then default. Removing browser can never leave zero matching conditions (Node/Vite always fall back to default), so it can't cause a resolution failure.

2. Hook ordering is right. The Cloudflare plugin injects conditions via its config hook (environments: getEnvironmentsConfig(...), 25619), not configEnvironment. Vite merges config-hook output into each per-environment config before running configEnvironment, so this enforce: "post" hook observes the merged browser and removes it.

3. In-place mutation + return null is the established, working pattern. The existing configEnvironment at index.ts:3449-3451 mutates config.resolve.extensions in place and returns null. Vite passes the per-environment config by reference, so the mutation propagates. The new hook follows this exactly and reassigns fresh filtered arrays (no in-place array mutation of a shared array). The non-null assertions at 4146 are guarded by the preceding optimizerConditions?.includes("browser") check, so they can't throw.

4. vp check / tsc --noEmit pass on the changed file.

Notes from prior reviews that are now stale

• The "plugin name is misleading (vinext:cloudflare-server-conditions)" note from the two earlier reviews is already resolved. Commit ef43f42 renamed it to vinext:server-conditions (adapter-neutral). The current code (index.ts:4128) is correct.

Non-blocking notes

1. Missing unit test (recommended before merge). The PR defers behavioral tests to #2877, but the repo has a near-free pattern for testing configEnvironment in isolation — see tests/compiler-define.test.ts:159 (plugin.configEnvironment!("rsc", config, { command }) against a plain object, no runtime). A focused test would assert: rsc/ssr with conditions: [..., "browser"] → browser removed, all others preserved (same for optimizeDeps.rolldownOptions.resolve.conditionNames); client and a no-browser config → untouched. This is the exact class of regression (a future adapter/Vite bump silently reintroducing browser) that would otherwise slip through completely silently.

2. Scoping to rsc/ssr env names depends on the vinext init contract — worth a comment. The filter only matches environments literally named rsc/ssr. For App Router this is architecturally guaranteed: init-cloudflare.ts:1023-1090 hard-validates viteEnvironment: { name: "rsc", childEnvironments: ["ssr"] }. But when cloudflare() is used without viteEnvironment.name, the worker environment name defaults to workerNameToEnvironmentName(topLevelName) (index.mjs:18405 — the wrangler worker name with dashes→underscores), not rsc/ssr. examples/pages-router-cloudflare/vite.config.ts uses a bare cloudflare(), so its worker env would be named pages_router_cloudflare and the browser filter would not apply to it. This isn't a regression introduced by this PR (the example is just out of sync with what vinext init scaffolds, which does add viteEnvironment: { name: "rsc" } for Pages Router too — init-cloudflare.ts:534-539), and the fix is correct for any project initialized the vinext-recommended way. But a one-line comment noting that this filter assumes the vinext env-naming contract (rsc/ssr) would help future readers, and it may be worth aligning the Pages Router example config or a follow-up to broaden the guard.

3. Rolldown-only optimizer branch. The optimizer filter handles only optimizeDeps.rolldownOptions.resolve.conditionNames, not the esbuild branch (optimizeDeps.esbuildOptions.conditions) the Cloudflare plugin emits when !isRolldown (25491-25497). Correct for this Vite 8 repo (isRolldown is always true), but a one-line comment noting the esbuild branch is intentionally unhandled would help — and pairs with the AGENTS.md guidance to treat esbuild config as a migration target, not a pattern to copy forward.

Verdict

Correct, safe, and consistent with vinext's existing server-resolution design (cf. the client-optimizer exclude comment at index.ts:3262-3269 documenting the same browser-export failure mode). No blocking issues. The one thing I'd act on before merge is the focused configEnvironment unit test — the pattern makes it trivial and it guards a silent-regression path.

[ask-bonk]

Review posted to PR #2918.

Summary

I independently verified this PR against the vendored @cloudflare/vite-plugin@1.31.0 source and the vinext plugin internals. The fix is correct and safe — no blocking issues.

Verified correct:

• The bug is real — defaultConditions includes browser, applied to both resolve.conditions (index.mjs:25438) and optimizeDeps.rolldownOptions.resolve.conditionNames (25480) on worker envs.
• Hook ordering works — Cloudflare injects via config hook (25619), which merges before the enforce: "post" configEnvironment runs.
• In-place mutation + return null is the established working pattern (index.ts:3449-3451); non-null assertions are properly guarded; removing browser can never leave zero matching conditions.
• tsc --noEmit passes.

Notably, I caught a stale note from the two prior reviews: they flagged the plugin name vinext:cloudflare-server-conditions as misleading, but commit ef43f42 already renamed it to vinext:server-conditions. That note is resolved.

New finding not in prior reviews: The filter scopes to environments literally named rsc/ssr. This is architecturally guaranteed for App Router (init hard-validates it), but a bare cloudflare() (like examples/pages-router-cloudflare/vite.config.ts) gets a worker env named after the wrangler worker name (index.mjs:18405), so browser would not be filtered there. Not a regression, but the guard implicitly depends on the vinext init contract — worth a comment or a follow-up to align the example.

Non-blocking (concur with prior reviews):

1. Missing unit test — near-free via the tests/compiler-define.test.ts:159 pattern; guards a silent-regression path.
2. Rolldown-only optimizer branch — correct for Vite 8, but worth a one-line comment.

github run