fix(build): define process.browser per environment

[james-elicx]

Summary

• define process.browser as true for client consumers and false for RSC, SSR, and Worker consumers, including each environment optimizer
• fold server-side process.browser guards before plugin-RSC import analysis so browser-only conditional exports are not resolved from dead branches
• preserve evaluation order, nested folding, shadowed bindings, and normal JavaScript operand-return semantics while handling optional, computed, commented, and escaped member spellings
• reject conflicting compiler.define and compiler.defineServer entries

This is the focused parent for the ESM-externals compatibility work in #2877; that PR will be stacked on this one after this draft is opened.

Validation

• vp test run tests/build-optimization.test.ts tests/compiler-define.test.ts tests/process-browser-define.test.ts tests/type-of-window.test.ts tests/tsconfig-paths-vite8.test.ts tests/client-global-define.test.ts — 205 passed, 2 skipped
• vp check packages/vinext/src/index.ts packages/vinext/src/plugins/typeof-window.ts tests/build-optimization.test.ts tests/compiler-define.test.ts tests/process-browser-define.test.ts tests/type-of-window.test.ts tests/tsconfig-paths-vite8.test.ts vite.config.ts
• vp run vinext#build
• two independent exact-head reviews: no findings

Final verification

• exact head: d0b8c67
• CI: https://github.com/cloudflare/vinext/actions/runs/31609426170 — green
• paired performance: https://github.com/cloudflare/vinext/actions/runs/31609426230 — production build mean -0.980%, median -0.873%; all six pairs faster
• Big Bonk: https://github.com/cloudflare/vinext/actions/runs/31609828779 — no blocking issues
• Next.js deploy suite: https://github.com/cloudflare/vinext/actions/runs/31610346306 — 2,644 passed / 160 failed / 631 skipped of 3,435
• regression audit: no candidate regression; the only failure absent from the frozen baseline is the recurring hash-scroll-offset flake, reproduced across unrelated heads

[james-elicx]

/bigbonk review for issues

[pkg-pr-new]

Open in StackBlitz

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

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

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

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

commit: d0b8c67

[ask-bonk]

UnknownError: ProviderInitError

github run

[ask-bonk]

@james-elicx Bonk workflow was cancelled.

View workflow run · To retry, trigger Bonk again.

[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 d0b8c67 against base bdc7703 using alternating same-runner rounds. Next.js was unchanged and skipped.

0 improved · 0 regressed · 6 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.03 s	3.03 s	⚫ -0.1%
Production build time	vinext	3.35 s	3.32 s	⚫ -0.9%
RSC entry closure size (gzip)	vinext	114.4 KB	114.4 KB	⚫ 0.0%
Server bundle size (gzip)	vinext	193.8 KB	193.8 KB	⚫ -0.0%

View detailed results and traces

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

[james-elicx]

/bigbonk review for issues

[ask-bonk]

/bigbonk review

Reviewed at head 20f3b49. Verified tsc --noEmit clean, and ran tests/type-of-window.test.ts, tests/compiler-define.test.ts, and the process.browser build-optimization integration test — all green.

No blocking issues found. The design is sound and the risky parts hold up under scrutiny. Notes below are analysis + a couple of low-severity robustness observations, not merge blockers.

What I checked closely

Value-semantics of the &&/|| operand folding (evaluateConsumerCondition).
The folds A && false → ((A), false) and A || true → ((A), true) are not value-preserving JavaScript (A && false returns A when A is falsy, not false). This is only correct in a boolean/condition context. The code is safe only because these effect-preserving folds are gated behind pruneUnreachableImports, which is set exclusively when scansImports === true (build.write === false) — i.e. the import-scan pass whose output feeds plugin-RSC static analysis and is never executed. Confirmed in index.ts that replaceProcessBrowser = scansImports and pruneUnreachableImports = scansImports, so real executed code never hits the lossy path. In dev/prod the actual process.browser substitution comes from Vite's per-environment define, and dead dynamic import() branches simply never resolve. This is the correct seam; worth keeping the invariant ("effect-folding is analysis-only") documented so a future change doesn't reuse pruneUnreachableImports in an executed path.

Dev/prod parity. Server envs get process.browser = "false" via configEnvironment define, and the optimizer pipeline mirrors it via optimizeDeps.rolldownOptions.transform.define. The "survives Vite's environment config merge" test confirms the consumer define merges with (not replaces) the NODE_ENV/JSX optimizer policy. Guarded dynamic imports are only pruned at build scan time, matching the fact that Vite dev never resolves a dead import().

Cache key correctness. The transform cache variant ${typeofWindow}:${processBrowser|-} co-varies exactly with scansImports (since replaceProcessBrowser === scansImports), so scan vs non-scan passes for the same env never collide on a cache entry. Good.

compiler.define / defineServer conflict rejection. Adding || key === "process.browser" correctly closes a gap key in defines couldn't catch (the per-env define is injected in configEnvironment, not the config-time defines object). Covered by the new compiler-define test.

Filter broadening. The transform.filter widened from \btypeof\s+window\b to also match \bprocess\b and escape sequences. This runs the handler over many more files (anything mentioning process), but the early-out in replaceConsumerEnvironmentConditions (process + browser both present, or an escape) bails before parsing for the common process.env.NODE_ENV case — verified by the "skips unrelated process references" test. Acceptable; just a slightly larger candidate set.

Low-severity observations (non-blocking)

• effects.filter(hasRange) can theoretically empty the array while result.effects.length > 0, which would make effects[0].start (lines 318/349) throw. In practice unreachable — effects are always a LogicalExpression.left node, which parseAst always ranges — so this is purely defensive tidiness, not a live bug.
• A && false style folds emit a SequenceExpression that drops the operand's runtime value. Correct for the analysis-only use as noted above; flagging only so the "analysis-only" constraint stays explicit if this helper is ever reused.

Nice work — the scope-aware binding checks, escaped/computed/optional/commented member spellings, and nested else if handling are all covered by tests and hold up.

[ask-bonk]

Posted the review to PR #2899.

Summary

Reviewed PR #2899 (fix(build): define process.browser per environment) at head 20f3b49. Verified type checks and ran the relevant test suites (type-of-window, compiler-define, and the process.browser build-optimization integration test) — all green.

No blocking issues found. The design is correct. Key findings:

• The lossy &&/|| operand folds (A && false → ((A), false)) are not value-preserving JavaScript, but they're safely gated behind pruneUnreachableImports, which is only ever set during the build import-scan pass (build.write === false) whose output feeds plugin-RSC static analysis and is never executed. Real code gets process.browser folded by Vite's per-environment define instead. Correct seam.
• Dev/prod parity holds — server envs get false via both define and the optimizer rolldownOptions.transform.define, and dead dynamic import() branches never resolve in dev.
• Cache variant key co-varies exactly with the scan/non-scan mode, so no collisions.
• The compiler.define/defineServer reservation correctly closes a gap key in defines couldn't catch.
• Filter broadening is mitigated by the early-out for unrelated process.env references.

Two low-severity, non-blocking observations were noted (a theoretically-unreachable empty-effects crash guard, and keeping the "analysis-only" invariant documented for the operand folds).

github run

[james-elicx]

/bigbonk review for issues

[ask-bonk]

/bigbonk review

Reviewed at head 20f3b49. Ran tests/type-of-window.test.ts, tests/compiler-define.test.ts, and tests/process-browser-define.test.ts (all green), typechecked the two touched source files clean, and traced the folding logic against a batch of adversarial inputs (nested &&/||/??, effect-on-right, statement-vs-expression positions, labeled statements). Everything parses and preserves left-operand effects.

No blocking issues. The architecture is correct and the risky part — the lossy A && false → ((A), false) / A || true → ((A), true) operand folds — is properly contained. Confirming the invariant chain that makes it safe, plus two low-severity notes.

Why the lossy folds are safe (verified)

The effect-preserving folds are gated behind pruneUnreachableImports, which in index.ts is set only to scansImports (build.write === false). Critically, replaceProcessBrowser is also gated on scansImports (index.ts:5977), so process.browser is only ever folded by this transform during the write-less import-scan pass whose output feeds plugin-RSC static analysis and is never executed. In dev and in real (write) builds, process.browser substitution comes entirely from Vite's per-environment define (value-preserving literal replacement), and dead dynamic import() branches simply never resolve. The tests/type-of-window.test.ts "preserves logical operand values outside import analysis" case pins the non-scan path to value-preserving output. Correct seam.

Notes (non-blocking)

1. The "effect-folding is analysis-only" invariant is load-bearing but only implicit. The entire correctness argument rests on replaceProcessBrowser === scansImports === pruneUnreachableImports. If a future change ever wires pruneUnreachableImports (or the SequenceExpression folds) into a code path that is actually executed, ((A), false) silently changes runtime semantics with no test to catch it — the existing value-preservation test only exercises the typeofWindow-only path, which never triggers the pruneUnreachableImports branches. Worth a one-line comment on the pruneUnreachableImports branches in evaluateConsumerCondition (typeof-window.ts:191-198, 206-213) stating "emits effect-only SequenceExpression that drops the operand value; only valid for the write-less import scan," so the constraint survives refactors.

2. effects.filter(hasRange) can theoretically empty a non-empty effects array, making effects[0].start (typeof-window.ts:318) and effects.at(-1)! (349/353/379) throw. Unreachable in practice — every effect is a LogicalExpression.left node, which parseAst always ranges — so this is defensive tidiness, not a live bug. If you want belt-and-suspenders, fall back to the non-effects rewrite path when the filtered array is empty.

What I checked and confirmed good

• Dev/prod parity: server envs get process.browser = "false" via both configEnvironment define and the optimizer optimizeDeps.rolldownOptions.transform.define, mirroring the established vinext:client-global-define pattern (index.ts:6100). The build-optimization "survives Vite's environment config merge" test confirms the consumer define merges with (not replaces) the NODE_ENV/JSX optimizer policy.
• typeof window is deliberately not mirrored into the optimizer define — it's a native Rolldown fold, unlike process.browser. Correct asymmetry.
• Cache variant ${typeofWindow|-}:${processBrowser|-} co-varies with scan/non-scan mode, so scan and non-scan passes for one env never collide.
• compiler.define / defineServer conflict rejection (|| key === "process.browser") correctly closes the gap key in defines can't catch, since the per-env define is injected in configEnvironment, not the config-time defines object. Matches Next.js reserving process.browser.
• Filter broadening to \bprocess\b...\bbrowser\b + escapes is mitigated by the early-out in replaceConsumerEnvironmentConditions for the common process.env.NODE_ENV case (tested).
• Scope handling (shadowed process/window bindings, function params, switch/loop/static-block/namespace var scoping) and the escaped/computed/optional/commented member spellings are all covered by tests and hold up under the traced inputs.
• Production branch pruning is verified end-to-end by the new build-optimization test: __BROWSER_ONLY_MODULE__ is pruned from RSC/SSR but kept on the client, while a universal side-effect module on the left of && process.browser is preserved.

Solid work.

[ask-bonk]

Posted my review to PR #2899.

Summary

Reviewed fix(build): define process.browser per environment at head 20f3b49. Ran the three most relevant test files (all green), typechecked the touched source clean, and traced the folding logic against adversarial nested &&/||/??, effect-on-right, and statement-vs-expression inputs.

No blocking issues found. The design is correct:

• The lossy operand folds (A && false → ((A), false)) are only ever triggered during the write-less import-scan pass (build.write === false), whose output feeds plugin-RSC static analysis and is never executed. Real code gets value-preserving process.browser substitution from Vite's per-environment define. Confirmed replaceProcessBrowser === scansImports === pruneUnreachableImports.
• Dev/prod parity holds — server envs get false via both define and the optimizer pipeline, matching the existing client-global-define pattern.
• typeof window is deliberately not mirrored into the optimizer (native Rolldown fold), unlike process.browser — correct asymmetry.
• The compiler.define/defineServer conflict rejection correctly closes a gap and matches Next.js reserving process.browser.

Two low-severity, non-blocking notes:

1. The "effect-folding is analysis-only" invariant is load-bearing but only implicit — worth a one-line comment on the pruneUnreachableImports branches so a future refactor can't silently reuse the lossy folds in an executed path (the existing value-preservation test only covers the typeofWindow-only path).
2. effects.filter(hasRange) could theoretically empty a non-empty array and make effects[0].start throw — unreachable in practice, defensive tidiness only.

github run

[james-elicx]

/bigbonk review for issues

[james-elicx]

/bigbonk review for issues

Please review exact current head 213eebe0d3da204dad337dfb25164a78c425e236, including the performance-filter delta from 20f3b4902bfd723db56158d1431123f735f863d0.

[ask-bonk]

/bigbonk review

Reviewed exact head 213eebe0d3da204dad337dfb25164a78c425e236, including the performance-filter delta from 20f3b49. Ran tests/type-of-window.test.ts, tests/compiler-define.test.ts, tests/tsconfig-paths-vite8.test.ts, tests/process-browser-define.test.ts, and the process.browser tests/build-optimization.test.ts integration test — all green. tsc --noEmit on the vinext package is clean. Traced the folding logic against adversarial nested &&/||/??, var-hoisting consequents, dangling else, and labeled-statement inputs; every output parses and preserves left-operand effects.

The design is correct and the previously-flagged invariant chain (replaceProcessBrowser === scansImports === pruneUnreachableImports, so the lossy A && false → ((A), false) folds only run in the write-less import-scan pass that feeds plugin-RSC static analysis and is never executed) holds up. Dev/prod parity, the optimizer-define mirroring, the cache variant key, and the compiler.define/defineServer reservation are all sound and covered by tests.

One new finding introduced by the performance-filter delta.

consumerEnvironmentConditionFilter is quadratic on files with many process tokens and no browser (typeof-window.ts:28)

The delta narrowed the filter from \b(?:typeof\s+window|process)\b|… (linear) to:

\btypeof\s+window\b|\bprocess\b[\s\S]*?\bbrowser\b|\\(?:…)

The \bprocess\b[\s\S]*?\bbrowser\b branch is O(n²) on inputs that contain many process occurrences but no browser: the engine restarts the lazy [\s\S]*? bridge at every process start position, each time scanning to end-of-file, failing to find browser, and backtracking. Measured scaling on "process.env.NODE_ENV; ".repeat(n):

input	length	time
n=10k	220 KB	145 ms
n=20k	440 KB	567 ms
n=40k	880 KB	2268 ms

Doubling the input ~quadruples the time — clean quadratic. A ~1 MB server file with only process.env.* refs takes ~3.5 s just for the filter check.

This matters because this transform.filter runs over every module in every environment, and the filter was deliberately broadened here to run on more files. Server code and bundled deps that reference process.env.* heavily but never mention browser are exactly the common case that now pays the worst-case cost. Files that do contain a browser token hit the first process match immediately and return fast (~0.2 ms), so the slow path is specifically "lots of process, no browser" — a realistic shape for large server bundles.

Note the filter is also order-sensitive (process must precede browser), while the internal early-out in replaceConsumerEnvironmentConditions (typeof-window.ts:239-241) already uses the order-independent /\bprocess\b/.test(code) && /\bbrowser\b/.test(code). So the filter is doing strictly more work than the precise gate it feeds. The straightforward fix is to keep the filter branch linear — e.g. match \bprocess\b alone (no [\s\S]*? bridge) and let the existing order-independent early-out do the precise rejection, or run two separate anchored .test() calls. A single lookahead-pair rewrite does not fix it (it is also quadratic/worse). Severity: low — it is a build/scan-time throughput regression, not a correctness bug, and only bites large process-heavy/browser-free files.

Confirmed good (no action)

• Lossy operand folds are analysis-only. pruneUnreachableImports is set only when scansImports (build.write === false), and replaceProcessBrowser is gated on the same flag (index.ts:5975-5977, 5993). Executed dev/prod code gets value-preserving process.browser substitution from Vite's per-environment define; the "preserves logical operand values outside import analysis" test pins the non-scan path to value-preserving output.
• effects.filter(hasRange) cannot empty a non-empty effects array — every effect is a LogicalExpression.left, which parseAst always ranges. The effects[0]/effects.at(-1)! accesses (typeof-window.ts:318, 322, 353, 379) are safe.
• Per-environment define uses consumer === "client" (index.ts:5943, 5983), so worker/rsc/ssr all correctly get false; the optimizer rolldownOptions.transform.define mirrors it. The build-optimization "consumer type across client, RSC, SSR, and Worker" and "survives Vite's environment config merge" tests confirm the define merges with (not replaces) the NODE_ENV/JSX optimizer policy.
• compiler.define/defineServer conflict rejection (|| key === "process.browser", index.ts:2383, 2398) correctly closes the gap key in defines can't catch, since the per-env define is injected in configEnvironment, not the config-time defines object. Matches Next.js reserving process.browser.
• Cache variant key ${typeofWindow|-}:${processBrowser|-} co-varies with scan/non-scan mode, so scan and non-scan passes for one env never collide.
• Scope handling (shadowed process/window bindings, params, switch/loop/static-block/namespace scoping) and escaped/computed/optional/commented member spellings are covered by tests and hold up.
• Production branch pruning verified end-to-end: __BROWSER_ONLY_MODULE__ is pruned from RSC/SSR but kept on the client, and the universal side-effect module on the left of && process.browser is preserved.

No blocking issues. The one filter-complexity note above is the only thing worth addressing.

[ask-bonk]

Posted my review to PR #2899.

Summary

Reviewed exact head 213eebe, including the performance-filter delta from 20f3b49. Ran all five relevant test files (all green), typechecked the vinext package clean, and traced the folding logic against adversarial inputs (nested &&/||/??, var-hoisting consequents, dangling else, labeled statements) — everything parses and preserves left-operand effects.

One new finding, introduced by the performance-filter delta:

• consumerEnvironmentConditionFilter is quadratic (typeof-window.ts:28). The delta narrowed the filter to \bprocess\b[\s\S]*?\bbrowser\b, which is O(n²) on files with many process tokens but no browser — the lazy bridge restarts and scans to EOF at every process position. Measured clean quadratic scaling: 145 ms → 567 ms → 2268 ms as input doubles; ~3.5 s for a 1 MB process.env-heavy server file. This filter runs over every module in every environment, and server bundles referencing process.env.* without browser are exactly the worst case. The internal early-out already uses an order-independent process-AND-browser check, so the filter is doing strictly more work than the gate it feeds. Severity: low (build/scan-time throughput, not correctness). Fix: keep the filter branch linear (match \bprocess\b alone and rely on the existing early-out).

Confirmed correct (no action): the analysis-only invariant chain for the lossy folds, the theoretically-empty effects guard being unreachable, per-environment define + optimizer mirroring across client/RSC/SSR/Worker, the compiler.define/defineServer reservation, the cache variant key, scope handling, and end-to-end production branch pruning.

No blocking issues — the filter-complexity note is the only thing worth addressing.

github run

[james-elicx]

/bigbonk review for issues

Please review exact current head 166133759b9765e760bb50b3b6dab41c4a0710c9, especially the linear native-filter fix for the prior quadratic-regex finding.

[ask-bonk]

/bigbonk review

Reviewed exact head 166133759b9765e760bb50b3b6dab41c4a0710c9 (fix(build): keep environment condition filter linear), focused on the linear native-filter fix for the prior quadratic-regex finding. Ran tests/type-of-window.test.ts, tests/process-browser-define.test.ts, and tests/compiler-define.test.ts (all green — 34 tests), and vp check on the touched files is clean.

The prior quadratic finding is fully resolved. No blocking issues.

The fix resolves the quadratic filter (verified)

The commit replaces the O(n²) \bprocess\b[\s\S]*?\bbrowser\b branch with a plain \bbrowser\b literal (typeof-window.ts:28). I reproduced the exact reported scaling on "process.env.NODE_ENV; ".repeat(n):

input	old regex	new regex
n=10k (220 KB)	149 ms	0.19 ms
n=20k (440 KB)	568 ms	0.16 ms
n=40k (880 KB)	2265 ms	0.31 ms

The old branch quadruples per doubling (clean quadratic); the new branch is flat regardless of size. The common server case — files with process.env.* but no browser — now short-circuits the filter entirely and never even enters the handler. The new "filters large process-heavy sources in linear time" test (< 500 ms on a 40k-repeat source) pins this against regression.

The broadened filter is still a correct superset of the precise gate (verified)

Dropping the process anchor means browser-only files now pass the filter where they didn't before. That is safe because the filter's only job is to be a fast superset of the precise internal gate in replaceConsumerEnvironmentConditions (typeof-window.ts:239-241), which still requires order-independent process and browser (or an escape sequence). I confirmed the gate's rejection behavior:

• browser-only code → filter passes, gate rejects → replaceConsumerEnvironmentConditions returns null early (no AST parse). Correct; negligible cost on a rare file shape.
• process-only code (process.env.NODE_ENV) → filter rejects → handler never runs. This is the win.
• process.browser → both pass → transform runs.

Every input the gate accepts still passes the new filter:

• gate case process && browser → filter matches \bbrowser\b
• gate case escape-sequence → filter matches the escape branch
• typeof window → filter matches \btypeof\s+window\b

So no transform is ever incorrectly skipped. The escaped-spelling cases (process.brow\u0073er, proce\u0073s.browser) remain covered by the escape branch and by the added "browser && process.browser" test case (typeof-window.test.ts:62-64).

Everything from prior reviews still holds

The rest of the PR is unchanged from the previously-approved head, and the invariant chain is intact: the lossy A && false → ((A), false) operand folds remain gated behind pruneUnreachableImports === scansImports === replaceProcessBrowser (write-less import-scan pass only, feeds plugin-RSC static analysis, never executed). Dev/prod parity via per-environment define + optimizer mirroring, the cache variant key, and the compiler.define/defineServer reservation are all unchanged and sound.

The two low-severity non-blocking notes from earlier reviews still stand (the load-bearing-but-implicit "effect-folding is analysis-only" invariant would benefit from a one-line comment; effects.filter(hasRange) emptying is unreachable in practice), but neither is affected by this commit and neither blocks merge.

Solid, targeted fix.

[ask-bonk]

Posted the review to PR #2899.

Summary

Reviewed exact head 166133759b9765e760bb50b3b6dab41c4a0710c9 (fix(build): keep environment condition filter linear), focused on the linear native-filter fix for the prior quadratic-regex finding.

The prior quadratic finding is fully resolved. No blocking issues.

Key verifications:

• Quadratic fix confirmed effective. The commit swaps the O(n²) \bprocess\b[\s\S]*?\bbrowser\b branch for a plain \bbrowser\b literal. I reproduced the exact reported scaling: old regex 149ms → 568ms → 2265ms (quadruples per doubling); new regex flat at ~0.2ms regardless of size. A new linear-time regression test (< 500ms on a 40k-repeat source) pins it.

• The broadened filter is still a correct superset of the precise internal gate. Dropping the process anchor only lets browser-only files through the filter, which the precise gate then rejects with an early null return (no AST parse). Every input the gate accepts still passes the filter, so no transform is ever incorrectly skipped. The common process.env.*-only server case now short-circuits entirely.

• Escaped-spelling cases (process.brow\u0073er, etc.) remain covered by the escape branch and the added "browser && process.browser" test case.

• Everything from prior reviews still holds — the invariant chain for the lossy operand folds, dev/prod parity, cache variant key, and the compiler.define/defineServer reservation are unchanged and sound.

All 34 relevant tests pass and vp check is clean on the touched files. Solid, targeted fix.

github run

[james-elicx]

/bigbonk review for issues

Please review exact current head d0b8c67b7e1fc9e8e7367585d3c39ef51d8070d5, especially the selective local-syntax filter, regex complexity, and escape/comment/optional-chain coverage.

[ask-bonk]

/bigbonk review

Reviewed exact head d0b8c67b7e1fc9e8e7367585d3c39ef51d8070d5, focused on the two commits since the last reviewed head (1661337): 8491143 (narrow environment condition filtering) and d0b8c67 (filter process browser syntax locally). Ran tests/type-of-window.test.ts, tests/process-browser-define.test.ts, and tests/compiler-define.test.ts — 35 passed. vp check on the touched files is clean. index.ts is unchanged by these two commits, so all prior findings on the define/optimizer/reservation logic still hold.

No blocking issues. The new local-syntax filter is correct, linear, and a proper superset of the precise AST gate.

The new filter is not quadratic (verified)

The delta replaced the linear \bbrowser\b branch with a process-anchored member-syntax matcher:

\bprocess\b[\s)]*(?:(?:\?\.|\.)\s*(?:browser\b|\/[/*])|(?:\?\.\s*)?\[[\s(]*(?:["']browser["']|["'][^"'\\\n\r]*\\|\/[/*])|\/[/*])

Unlike the earlier \bprocess\b[\s\S]*?\bbrowser\b regression, this does not bridge arbitrary source. The [\s)]* / [\s(]* runs are bounded, simple character classes that don't overlap ambiguously with the literals that follow, so there is no catastrophic backtracking. Measured on adversarial inputs (all flat/linear):

input	time
"process ".repeat(80k) (880 KB)	2.4 ms
process + 1M spaces	3.1 ms
process[" + 1M a (unterminated)	2.5 ms
process[ + 1M (	3.3 ms
typeof + 1M spaces	4.4 ms

The new "filters large one-sided sources in linear time" test pins four one-sided shapes (process-heavy, browser-heavy, trailing-space, unterminated-bracket) under 500 ms total, which guards the regression direction.

The filter remains a correct superset of the precise gate (verified)

The filter's only job is to be a fast superset of the order-independent internal gate in replaceConsumerEnvironmentConditions (typeof-window.ts:243-248), which parses and folds only real process.browser member expressions. I confirmed every JS-valid spelling the AST would fold still passes the filter:

• Dot / whitespace / comment spellings: process.browser, process . browser, process/* c */.browser, process. /* c */ browser, process//c\n.browser, process/**/./**/browser — all admitted (comment cases via the \/[/*] branches).
• Computed: process["browser"], process[ 'browser' ], process[/* c */"browser"], process[('browser')] — admitted.
• Optional chain: process?.browser, process?.["browser"], process?. [("browser")], process?./* c */['browser'] — admitted.
• Parenthesized object: (process).browser, ((process)).browser, (process)?.browser — admitted via [\s)]*.
• Escaped identifiers: proce\u0073s.browser, process.brow\u0073er — admitted via the standalone \u/\x escape branch. Confirmed JS only permits \u/\u{} in identifiers (not \x), so that branch covers every parseable escaped-identifier spelling.
• Computed with escaped string content: process["brow\u0073er"] (escape branch), plus line-continuation and identity-escape spellings process["brow\<newline>ser"] / process["brow\ser"] (the ["'][^"'\\\n\r]*\\ computed-backslash branch).

So no foldable process.browser is ever silently skipped. New tests cover the optional-chain, computed-escape, and comment spellings both at the filter level and end-to-end ("folds computed process.browser escape and optional-chain spellings" asserts the dynamic import("browser-only") is pruned).

Over-admission is harmless (verified)

The narrowing intentionally keeps two over-admitting branches: the standalone \u/\x escape alternation admits any file containing a Unicode/hex escape, and the \/[/*] branches admit process followed by a comment even when no browser follows (e.g. process /* c */ .env). Both are harmless — such files pass the filter, then the AST gate + parse find no foldable member and return null (no rewrite). This matches the design intent (filter = cheap superset, AST = precise gate) and is the same trade-off prior heads made. Critically, the common process.env.*-only server case (no comment, no escape) is still rejected outright, so the perf win the previous fix delivered is preserved.

Everything from prior reviews still holds

index.ts is untouched by these commits. The invariant chain (replaceProcessBrowser === scansImports === pruneUnreachableImports, lossy A && false → ((A), false) folds only in the write-less import-scan pass that feeds plugin-RSC static analysis and is never executed), dev/prod parity via per-environment define + optimizer mirroring, the cache variant key, and the compiler.define/defineServer reservation are all unchanged and sound.

The two long-standing low-severity, non-blocking notes still stand and are unaffected by this delta:

1. The "effect-folding is analysis-only" invariant is load-bearing but only implicit — a one-line comment on the pruneUnreachableImports branches would keep it safe against future refactors.
2. effects.filter(hasRange) emptying a non-empty array is unreachable in practice (every effect is a LogicalExpression.left, always ranged) — defensive tidiness only.

Clean, well-tested, targeted fix. No merge blockers.

[ask-bonk]

Posted my review to PR #2899.

Summary

Reviewed exact head d0b8c67b7e1fc9e8e7367585d3c39ef51d8070d5, focused on the two new commits since the last reviewed head: 8491143 (narrow environment condition filtering) and d0b8c67 (filter process browser syntax locally). Ran the three relevant test files (35 passed) and vp check clean. index.ts is untouched by these commits.

No blocking issues found. The new selective local-syntax filter is correct, linear, and a proper superset of the precise AST gate.

Key verifications:

• Regex complexity — linear, not quadratic. The delta replaced the earlier O(n²) \bprocess\b[\s\S]*?\bbrowser\b bridge with a process-anchored member-syntax matcher using bounded [\s)]* / [\s(]* runs. Measured flat/linear on all adversarial shapes (process-heavy, unterminated brackets, 1M-space runs) — all under ~4.5 ms even at ~1 MB. The new linear-time regression test pins four one-sided shapes under 500 ms.

• Escape/comment/optional-chain coverage is complete. Every JS-valid process.browser spelling the AST would fold still passes the filter: dot/whitespace/comment spellings, computed ["browser"], optional chains (process?.["browser"], process?. [("browser")]), parenthesized objects ((process).browser), escaped identifiers (\u/\x branch — confirmed JS only allows \u in identifiers), and computed strings with escaped/line-continuation content (the computed-backslash branch). No foldable member is silently skipped.

• Over-admission is harmless. The \u/\x escape branch and the \/[/*] comment branches over-admit some files, but those hit the AST gate and return null with no rewrite — same trade-off prior heads made. The common process.env.*-only server case is still rejected, preserving the perf win.

Two long-standing low-severity non-blocking notes carry over (implicit "analysis-only" fold invariant would benefit from a comment; the unreachable empty-effects guard). Neither is affected by this delta. Clean, targeted fix.

github run