Fix JSX transform not respecting NODE_ENV=production #24048
Conversation
WalkthroughDefaults and propagation for JSX development mode were changed: the "react-jsx" runtime default now sets development = false; bundler centralizes JSX development resolution via a new helper and uses task-level values when unspecified; the transpiler forces NODE_ENV=production for production builds; and tests cover NODE_ENV/--production behaviors. Changes
Possibly related PRs
Suggested reviewers
Pre-merge checks✅ Passed checks (4 passed)
[pre_merge_check_pass] The pull request description is comprehensive and well-structured. While it does not use the exact template section headings (using "## Summary" instead of "### What does this PR do?"), it provides all required information: a clear explanation of what the PR does (fixing JSX transform behavior with NODE_ENV=production), detailed root cause analysis, specific code changes across four files, configuration priority clarification, and a complete test plan with verification steps. The description fully addresses the intent of the template and provides more context than required. | 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (4)
🧰 Additional context used📓 Path-based instructions (10)test/**📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
Files:
test/**/*.{js,ts}📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
Files:
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}📄 CodeRabbit inference engine (test/CLAUDE.md)
Files:
test/**/*.{ts,tsx,js,jsx,mjs,cjs}📄 CodeRabbit inference engine (test/CLAUDE.md)
Files:
test/regression/issue/*.test.ts📄 CodeRabbit inference engine (test/CLAUDE.md)
Files:
test/regression/**📄 CodeRabbit inference engine (test/CLAUDE.md)
Files:
test/regression/issue/[[:digit:]][[:digit:]]*.test.ts📄 CodeRabbit inference engine (CLAUDE.md)
Files:
test/**/*.test.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.zig📄 CodeRabbit inference engine (.cursor/rules/javascriptcore-class.mdc)
Files:
src/**/*.zig📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)
Files:
🧠 Learnings (12)📚 Learning: 2025-10-12T02:22:34.373ZApplied to files:
📚 Learning: 2025-08-30T00:09:39.100ZApplied to files:
📚 Learning: 2025-08-30T00:09:39.100ZApplied to files:
📚 Learning: 2025-10-12T02:22:34.373ZApplied to files:
📚 Learning: 2025-10-20T04:22:55.575ZApplied to files:
📚 Learning: 2025-08-30T00:12:56.803ZApplied to files:
📚 Learning: 2025-08-30T00:12:56.803ZApplied to files:
📚 Learning: 2025-08-30T00:12:56.803ZApplied to files:
📚 Learning: 2025-10-12T02:22:34.373ZApplied to files:
📚 Learning: 2025-08-30T00:09:39.100ZApplied to files:
📚 Learning: 2025-10-12T02:22:34.373ZApplied to files:
📚 Learning: 2025-08-30T00:09:39.100ZApplied to files:
🧬 Code graph analysis (1)test/regression/issue/23959.test.ts (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (2)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/bundler/bundle_v2.zig (1)
3464-3469: Order is correct; consider a tiny helper to avoid driftYou initialize from resolve_result.jsx, then override only when env is forced. Perfect. To keep the three sites in sync, consider a small helper:
+inline fn effectiveJsxDev(current: bool, force: options.ForceNodeEnv) bool { + return switch (force) { .development => true, .production => false, .unspecified => current }; +} @@ - resolve_task.jsx.development = switch (transpiler.options.force_node_env) { - .development => true, - .production => false, - .unspecified => resolve_task.jsx.development, - }; + resolve_task.jsx.development = effectiveJsxDev(resolve_task.jsx.development, transpiler.options.force_node_env);Do similarly for the two other call sites.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
src/bundler/bundle_v2.zig(3 hunks)src/options.zig(1 hunks)src/transpiler.zig(1 hunks)test/regression/issue/23959.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
test/**
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
Place all tests under the test/ directory
Files:
test/regression/issue/23959.test.ts
test/**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
test/**/*.{js,ts}: Write tests in JavaScript or TypeScript using Bun’s Jest-style APIs (test, describe, expect) and run with bun test
Prefer data-driven tests (e.g., test.each) to reduce boilerplate
Use shared utilities from test/harness.ts where applicable
Files:
test/regression/issue/23959.test.ts
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}
📄 CodeRabbit inference engine (test/CLAUDE.md)
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}: Usebun:testfor files ending with*.test.{ts,js,jsx,tsx,mjs,cjs}
Prefer concurrent tests (test.concurrent/describe.concurrent) over sequential when feasible
Organize tests withdescribeblocks to group related tests
Use utilities likedescribe.each,toMatchSnapshot, and lifecycle hooks (beforeAll,beforeEach,afterEach) and track resources for cleanup
Files:
test/regression/issue/23959.test.ts
test/**/*.{ts,tsx,js,jsx,mjs,cjs}
📄 CodeRabbit inference engine (test/CLAUDE.md)
For large/repetitive strings, use
Buffer.alloc(count, fill).toString()instead of"A".repeat(count)
Files:
test/regression/issue/23959.test.ts
test/regression/issue/*.test.ts
📄 CodeRabbit inference engine (test/CLAUDE.md)
Place regression tests for specific issues in
/test/regression/issue/${issueNumber}.test.ts
Files:
test/regression/issue/23959.test.ts
test/regression/**
📄 CodeRabbit inference engine (test/CLAUDE.md)
Do not place tests without an issue number in the regression directory
Files:
test/regression/issue/23959.test.ts
test/regression/issue/[[:digit:]][[:digit:]]*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Place regression tests for a specific numbered GitHub issue in test/regression/issue/${issueNumber}.test.ts and ensure the issue number is real (numeric)
Files:
test/regression/issue/23959.test.ts
test/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
test/**/*.test.{ts,tsx}: Test files must be created under test/ and end with .test.ts or .test.tsx
In tests that open network ports, always use port: 0; do not hardcode port numbers or roll your own random port
Use normalizeBunSnapshot to normalize snapshot output instead of comparing raw stdout
Do not write tests that merely assert absence of strings like "panic" or "uncaught exception"
Use tempDir from "harness" to create temporary directories; do not use fs.mkdtempSync or tmpdirSync
When spawning processes in tests, assert output before asserting the exit code
Avoid shell commands in tests (e.g., find, grep); prefer Bun's Glob and built-in tools
Prefer snapshot tests over direct string equality assertions on stdout
Files:
test/regression/issue/23959.test.ts
**/*.zig
📄 CodeRabbit inference engine (.cursor/rules/javascriptcore-class.mdc)
**/*.zig: Declare the extern C symbol in Zig and export a Zig-friendly alias for use
Wrap the Bun____toJS extern in a Zig method that takes a JSGlobalObject and returns JSC.JSValue
Files:
src/bundler/bundle_v2.zigsrc/options.zigsrc/transpiler.zig
src/**/*.zig
📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)
When adding debug logs in Zig, create a scoped logger and log via Bun APIs:
const log = bun.Output.scoped(.${SCOPE}, .hidden);thenlog("...", .{})
src/**/*.zig: Use private fields in Zig with the#prefix (e.g.,struct { #foo: u32 };)
Prefer decl literals in Zig (e.g.,const decl: Decl = .{ .binding = 0, .value = 0 };)
Prefer placing@importstatements at the bottom of the Zig file (formatter may reorder automatically)
Prefer@import("bun")rather than@import("root").bunor@import("../bun.zig")In Zig code, be careful with allocators and use defer for cleanup
Files:
src/bundler/bundle_v2.zigsrc/options.zigsrc/transpiler.zig
🧠 Learnings (11)
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/bundle.test.ts : bundle.test.ts should contain DevServer-specific bundling tests
Applied to files:
test/regression/issue/23959.test.ts
📚 Learning: 2025-10-12T02:22:34.373Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-10-12T02:22:34.373Z
Learning: Applies to test/{test/**/*.test.{ts,js,jsx,tsx,mjs,cjs},test/js/node/test/{parallel,sequential}/*.js} : When spawning Bun in tests, use `bunExe()` and `bunEnv` from `harness`
Applied to files:
test/regression/issue/23959.test.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/css.test.ts : css.test.ts should contain CSS bundling tests in dev mode
Applied to files:
test/regression/issue/23959.test.ts
📚 Learning: 2025-10-12T02:22:34.373Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-10-12T02:22:34.373Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `bun:test` for files ending with `*.test.{ts,js,jsx,tsx,mjs,cjs}`
Applied to files:
test/regression/issue/23959.test.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev-and-prod.ts : Use devAndProductionTest to run the same test in both dev and production; these tests must not attempt to test hot reloading
Applied to files:
test/regression/issue/23959.test.ts
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/**/*.{js,ts} : Write tests in JavaScript or TypeScript using Bun’s Jest-style APIs (test, describe, expect) and run with bun test
Applied to files:
test/regression/issue/23959.test.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/esm.test.ts : esm.test.ts should cover ESM feature behavior in development mode
Applied to files:
test/regression/issue/23959.test.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/react-spa.test.ts : react-spa.test.ts should contain React SPA, react-refresh, and basic server component transform tests
Applied to files:
test/regression/issue/23959.test.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/{dev/*.test.ts,dev-and-prod.ts} : Import testing utilities (devTest, prodTest, devAndProductionTest, Dev, Client) from test/bake/bake-harness.ts
Applied to files:
test/regression/issue/23959.test.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/bake-harness.ts : Provide and maintain shared test utilities: devTest, prodTest, devAndProductionTest, Dev, Client, and helpers in the harness
Applied to files:
test/regression/issue/23959.test.ts
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/cli/**/*.{js,ts} : When testing Bun as a CLI, use spawn with bunExe() and bunEnv from harness, and capture stdout/stderr via pipes
Applied to files:
test/regression/issue/23959.test.ts
🧬 Code graph analysis (1)
test/regression/issue/23959.test.ts (1)
test/harness.ts (2)
tempDir(277-284)bunExe(102-105)
🔇 Additional comments (6)
src/options.zig (1)
1229-1231: Approved—mapping audit confirms consistency across codebase.The audit shows all JSX.RuntimeMap usages are correct and consistent:
- "react-jsx" correctly maps to production (development = false)
- "react-jsxdev" correctly maps to development (development = true)
- No conflicting mappings found in src/resolver/tsconfig_json.zig, src/bun.js/api/JSBundler.zig, src/ast/P.zig, or src/bunfig.zig
The fix properly resolves the prior regression.
src/transpiler.zig (1)
563-564: Propagate forced production NODE_ENV.Good symmetry with the development branch; this ensures bundling honors NODE_ENV over tsconfig. The search for
process.env.NODE_ENVtests didn't locate a specific regression test for--define:process.env.NODE_ENV="development"overriding an environment-setNODE_ENV=production; you may want to verify one exists or add it.Optional: consider factoring repeated env-application logic into a small helper to reduce drift between branches.
src/bundler/bundle_v2.zig (2)
822-827: Consistent per-task fallback across entry enqueue pathSame fix applied here; keeps task-level JSX dev when NODE_ENV is unspecified. Good consistency.
758-763: CONFIRMED: ParseTask.init properly initializes task.jsx before mutationParseTask.init (line 117 of src/bundler/ParseTask.zig) sets
.jsx = resolve_result.jsx, and before the mutation at lines 758-763, task.jsx is configured at line 1318. The.unspecifiedfallback correctly preserves the per-module jsx.development value, respecting per-task configuration over global defaults.test/regression/issue/23959.test.ts (2)
1-33: Production path regression test looks solidCorrectly isolates env via bunEnv override, uses --external react, and asserts jsx-runtime + jsx, not jsxDEV. Good. [Based on learnings]
34-88: Dev and default paths are covered appropriatelyBoth NODE_ENV=development and unset default case assert jsx-dev-runtime + jsxDEV. Matches expected behavior. [Based on learnings]
robobun
force-pushed
the
claude/fix-jsx-production-mode
branch
from
4dff87a to
a586bb7
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
test/regression/issue/23959.test.ts (1)
109-112: Address past review feedback: simplify assertions and add jsx( check.As noted in previous review, the regex alternation for JSON-escaped quotes is unnecessary since the output is plain JavaScript, not JSON. Additionally, for symmetry with test 1, add an assertion for
jsx(calls.Apply this diff:
- expect(output).toMatch(/from\s*"react\/jsx-runtime"|from\\"react\/jsx-runtime\\"/); + expect(output).toContain('from "react/jsx-runtime"'); + expect(output).toContain("jsx("); expect(output).not.toContain("jsxDEV"); + expect(output).not.toContain('from "react/jsx-dev-runtime"');
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
src/bundler/bundle_v2.zig(3 hunks)src/options.zig(1 hunks)src/transpiler.zig(1 hunks)test/regression/issue/23959.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
**/*.zig
📄 CodeRabbit inference engine (.cursor/rules/javascriptcore-class.mdc)
**/*.zig: Declare the extern C symbol in Zig and export a Zig-friendly alias for use
Wrap the Bun____toJS extern in a Zig method that takes a JSGlobalObject and returns JSC.JSValue
Files:
src/options.zigsrc/transpiler.zigsrc/bundler/bundle_v2.zig
src/**/*.zig
📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)
When adding debug logs in Zig, create a scoped logger and log via Bun APIs:
const log = bun.Output.scoped(.${SCOPE}, .hidden);thenlog("...", .{})
src/**/*.zig: Use private fields in Zig with the#prefix (e.g.,struct { #foo: u32 };)
Prefer decl literals in Zig (e.g.,const decl: Decl = .{ .binding = 0, .value = 0 };)
Prefer placing@importstatements at the bottom of the Zig file (formatter may reorder automatically)
Prefer@import("bun")rather than@import("root").bunor@import("../bun.zig")In Zig code, be careful with allocators and use defer for cleanup
Files:
src/options.zigsrc/transpiler.zigsrc/bundler/bundle_v2.zig
test/**
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
Place all tests under the test/ directory
Files:
test/regression/issue/23959.test.ts
test/**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
test/**/*.{js,ts}: Write tests in JavaScript or TypeScript using Bun’s Jest-style APIs (test, describe, expect) and run with bun test
Prefer data-driven tests (e.g., test.each) to reduce boilerplate
Use shared utilities from test/harness.ts where applicable
Files:
test/regression/issue/23959.test.ts
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}
📄 CodeRabbit inference engine (test/CLAUDE.md)
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}: Usebun:testfor files ending with*.test.{ts,js,jsx,tsx,mjs,cjs}
Prefer concurrent tests (test.concurrent/describe.concurrent) over sequential when feasible
Organize tests withdescribeblocks to group related tests
Use utilities likedescribe.each,toMatchSnapshot, and lifecycle hooks (beforeAll,beforeEach,afterEach) and track resources for cleanup
Files:
test/regression/issue/23959.test.ts
test/**/*.{ts,tsx,js,jsx,mjs,cjs}
📄 CodeRabbit inference engine (test/CLAUDE.md)
For large/repetitive strings, use
Buffer.alloc(count, fill).toString()instead of"A".repeat(count)
Files:
test/regression/issue/23959.test.ts
test/regression/issue/*.test.ts
📄 CodeRabbit inference engine (test/CLAUDE.md)
Place regression tests for specific issues in
/test/regression/issue/${issueNumber}.test.ts
Files:
test/regression/issue/23959.test.ts
test/regression/**
📄 CodeRabbit inference engine (test/CLAUDE.md)
Do not place tests without an issue number in the regression directory
Files:
test/regression/issue/23959.test.ts
test/regression/issue/[[:digit:]][[:digit:]]*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Place regression tests for a specific numbered GitHub issue in test/regression/issue/${issueNumber}.test.ts and ensure the issue number is real (numeric)
Files:
test/regression/issue/23959.test.ts
test/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
test/**/*.test.{ts,tsx}: Test files must be created under test/ and end with .test.ts or .test.tsx
In tests that open network ports, always use port: 0; do not hardcode port numbers or roll your own random port
Use normalizeBunSnapshot to normalize snapshot output instead of comparing raw stdout
Do not write tests that merely assert absence of strings like "panic" or "uncaught exception"
Use tempDir from "harness" to create temporary directories; do not use fs.mkdtempSync or tmpdirSync
When spawning processes in tests, assert output before asserting the exit code
Avoid shell commands in tests (e.g., find, grep); prefer Bun's Glob and built-in tools
Prefer snapshot tests over direct string equality assertions on stdout
Files:
test/regression/issue/23959.test.ts
🧠 Learnings (12)
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/bundle.test.ts : bundle.test.ts should contain DevServer-specific bundling tests
Applied to files:
test/regression/issue/23959.test.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/css.test.ts : css.test.ts should contain CSS bundling tests in dev mode
Applied to files:
test/regression/issue/23959.test.ts
📚 Learning: 2025-10-12T02:22:34.373Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-10-12T02:22:34.373Z
Learning: Applies to test/{test/**/*.test.{ts,js,jsx,tsx,mjs,cjs},test/js/node/test/{parallel,sequential}/*.js} : When spawning Bun in tests, use `bunExe()` and `bunEnv` from `harness`
Applied to files:
test/regression/issue/23959.test.ts
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/js/bun/**/*.{js,ts} : Place Bun API tests under test/js/bun/, separated by category (e.g., test/js/bun/glob/)
Applied to files:
test/regression/issue/23959.test.ts
📚 Learning: 2025-10-12T02:22:34.373Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-10-12T02:22:34.373Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `bun:test` for files ending with `*.test.{ts,js,jsx,tsx,mjs,cjs}`
Applied to files:
test/regression/issue/23959.test.ts
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/**/*.{js,ts} : Write tests in JavaScript or TypeScript using Bun’s Jest-style APIs (test, describe, expect) and run with bun test
Applied to files:
test/regression/issue/23959.test.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/react-spa.test.ts : react-spa.test.ts should contain React SPA, react-refresh, and basic server component transform tests
Applied to files:
test/regression/issue/23959.test.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev-and-prod.ts : Use devAndProductionTest to run the same test in both dev and production; these tests must not attempt to test hot reloading
Applied to files:
test/regression/issue/23959.test.ts
📚 Learning: 2025-10-20T04:22:55.575Z
Learnt from: CR
PR: oven-sh/bun#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-20T04:22:55.575Z
Learning: Applies to test/**/*.test.{ts,tsx} : Prefer snapshot tests over direct string equality assertions on stdout
Applied to files:
test/regression/issue/23959.test.ts
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/cli/**/*.{js,ts} : When testing Bun as a CLI, use spawn with bunExe() and bunEnv from harness, and capture stdout/stderr via pipes
Applied to files:
test/regression/issue/23959.test.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/bake-harness.ts : Provide and maintain shared test utilities: devTest, prodTest, devAndProductionTest, Dev, Client, and helpers in the harness
Applied to files:
test/regression/issue/23959.test.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/{dev/*.test.ts,dev-and-prod.ts} : Import testing utilities (devTest, prodTest, devAndProductionTest, Dev, Client) from test/bake/bake-harness.ts
Applied to files:
test/regression/issue/23959.test.ts
🧬 Code graph analysis (1)
test/regression/issue/23959.test.ts (1)
test/harness.ts (2)
tempDir(277-284)bunExe(102-105)
🔇 Additional comments (6)
src/bundler/bundle_v2.zig (3)
758-762: LGTM! Per-module JSX development resolution is correct.The change correctly defers to
task.jsx.developmentwhenforce_node_envis unspecified, allowing tsconfig JSX settings to take effect while still respecting explicit NODE_ENV values.
823-827: LGTM! Consistent with enqueueFileFromDevServerIncrementalGraphInvalidation.Same pattern applied correctly for entry point items, ensuring per-module JSX configuration is respected.
3465-3469: LGTM! Completes the per-module JSX configuration pattern.All three resolution paths (dev server invalidation, entry items, and parse task resolution) now correctly preserve per-module JSX development settings when NODE_ENV is unspecified.
test/regression/issue/23959.test.ts (3)
4-32: LGTM! Comprehensive test for NODE_ENV=production.The test correctly verifies production JSX runtime usage and includes both positive and negative assertions. Good use of
usingfor resource cleanup.
34-60: LGTM! Development mode test is well-structured.Properly verifies that NODE_ENV=development triggers the development JSX runtime.
116-148: LGTM! Addresses configuration precedence as suggested.This test verifies that NODE_ENV=production correctly overrides tsconfig.json, addressing the precedence concern from previous review. The past comment optionally suggested two precedence tests; this one covers the critical path (NODE_ENV overriding tsconfig).
robobun
force-pushed
the
claude/fix-jsx-production-mode
branch
2 times, most recently
from
cf8d24b to
c8b1b6d
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/transpiler.zig (1)
562-563: NODE_ENV=production is now correctly forced across transpiler and resolverMirrors the development branch behavior and prevents tsconfig from overriding production. Good symmetry and priority enforcement.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
src/bundler/bundle_v2.zig(3 hunks)src/options.zig(1 hunks)src/transpiler.zig(1 hunks)test/regression/issue/23959.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
**/*.zig
📄 CodeRabbit inference engine (.cursor/rules/javascriptcore-class.mdc)
**/*.zig: Declare the extern C symbol in Zig and export a Zig-friendly alias for use
Wrap the Bun____toJS extern in a Zig method that takes a JSGlobalObject and returns JSC.JSValue
Files:
src/transpiler.zigsrc/options.zigsrc/bundler/bundle_v2.zig
src/**/*.zig
📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)
When adding debug logs in Zig, create a scoped logger and log via Bun APIs:
const log = bun.Output.scoped(.${SCOPE}, .hidden);thenlog("...", .{})
src/**/*.zig: Use private fields in Zig with the#prefix (e.g.,struct { #foo: u32 };)
Prefer decl literals in Zig (e.g.,const decl: Decl = .{ .binding = 0, .value = 0 };)
Prefer placing@importstatements at the bottom of the Zig file (formatter may reorder automatically)
Prefer@import("bun")rather than@import("root").bunor@import("../bun.zig")In Zig code, be careful with allocators and use defer for cleanup
Files:
src/transpiler.zigsrc/options.zigsrc/bundler/bundle_v2.zig
test/**
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
Place all tests under the test/ directory
Files:
test/regression/issue/23959.test.ts
test/**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
test/**/*.{js,ts}: Write tests in JavaScript or TypeScript using Bun’s Jest-style APIs (test, describe, expect) and run with bun test
Prefer data-driven tests (e.g., test.each) to reduce boilerplate
Use shared utilities from test/harness.ts where applicable
Files:
test/regression/issue/23959.test.ts
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}
📄 CodeRabbit inference engine (test/CLAUDE.md)
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}: Usebun:testfor files ending with*.test.{ts,js,jsx,tsx,mjs,cjs}
Prefer concurrent tests (test.concurrent/describe.concurrent) over sequential when feasible
Organize tests withdescribeblocks to group related tests
Use utilities likedescribe.each,toMatchSnapshot, and lifecycle hooks (beforeAll,beforeEach,afterEach) and track resources for cleanup
Files:
test/regression/issue/23959.test.ts
test/**/*.{ts,tsx,js,jsx,mjs,cjs}
📄 CodeRabbit inference engine (test/CLAUDE.md)
For large/repetitive strings, use
Buffer.alloc(count, fill).toString()instead of"A".repeat(count)
Files:
test/regression/issue/23959.test.ts
test/regression/issue/*.test.ts
📄 CodeRabbit inference engine (test/CLAUDE.md)
Place regression tests for specific issues in
/test/regression/issue/${issueNumber}.test.ts
Files:
test/regression/issue/23959.test.ts
test/regression/**
📄 CodeRabbit inference engine (test/CLAUDE.md)
Do not place tests without an issue number in the regression directory
Files:
test/regression/issue/23959.test.ts
test/regression/issue/[[:digit:]][[:digit:]]*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Place regression tests for a specific numbered GitHub issue in test/regression/issue/${issueNumber}.test.ts and ensure the issue number is real (numeric)
Files:
test/regression/issue/23959.test.ts
test/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
test/**/*.test.{ts,tsx}: Test files must be created under test/ and end with .test.ts or .test.tsx
In tests that open network ports, always use port: 0; do not hardcode port numbers or roll your own random port
Use normalizeBunSnapshot to normalize snapshot output instead of comparing raw stdout
Do not write tests that merely assert absence of strings like "panic" or "uncaught exception"
Use tempDir from "harness" to create temporary directories; do not use fs.mkdtempSync or tmpdirSync
When spawning processes in tests, assert output before asserting the exit code
Avoid shell commands in tests (e.g., find, grep); prefer Bun's Glob and built-in tools
Prefer snapshot tests over direct string equality assertions on stdout
Files:
test/regression/issue/23959.test.ts
🧠 Learnings (9)
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/bundle.test.ts : bundle.test.ts should contain DevServer-specific bundling tests
Applied to files:
test/regression/issue/23959.test.ts
📚 Learning: 2025-10-12T02:22:34.373Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-10-12T02:22:34.373Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `bun:test` for files ending with `*.test.{ts,js,jsx,tsx,mjs,cjs}`
Applied to files:
test/regression/issue/23959.test.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/css.test.ts : css.test.ts should contain CSS bundling tests in dev mode
Applied to files:
test/regression/issue/23959.test.ts
📚 Learning: 2025-10-12T02:22:34.373Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-10-12T02:22:34.373Z
Learning: Applies to test/{test/**/*.test.{ts,js,jsx,tsx,mjs,cjs},test/js/node/test/{parallel,sequential}/*.js} : When spawning Bun in tests, use `bunExe()` and `bunEnv` from `harness`
Applied to files:
test/regression/issue/23959.test.ts
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/**/*.{js,ts} : Write tests in JavaScript or TypeScript using Bun’s Jest-style APIs (test, describe, expect) and run with bun test
Applied to files:
test/regression/issue/23959.test.ts
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/cli/**/*.{js,ts} : When testing Bun as a CLI, use spawn with bunExe() and bunEnv from harness, and capture stdout/stderr via pipes
Applied to files:
test/regression/issue/23959.test.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev-and-prod.ts : Use devAndProductionTest to run the same test in both dev and production; these tests must not attempt to test hot reloading
Applied to files:
test/regression/issue/23959.test.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/bake-harness.ts : Provide and maintain shared test utilities: devTest, prodTest, devAndProductionTest, Dev, Client, and helpers in the harness
Applied to files:
test/regression/issue/23959.test.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/{dev/*.test.ts,dev-and-prod.ts} : Import testing utilities (devTest, prodTest, devAndProductionTest, Dev, Client) from test/bake/bake-harness.ts
Applied to files:
test/regression/issue/23959.test.ts
🧬 Code graph analysis (1)
test/regression/issue/23959.test.ts (1)
test/harness.ts (2)
tempDir(277-284)bunExe(102-105)
🔇 Additional comments (2)
src/bundler/bundle_v2.zig (2)
3466-3469: onResolve path respects per-task JSX dev when NODE_ENV is unspecifiedGood alignment with the other paths; this closes an easy-to-miss inconsistency.
761-763: No action required; initialization order is correct.The verification confirms that
ParseTask.init()(called at line 753) properly initializes thejsxfield before it is read at line 761. Thejsxfield inParseTaskis typed asoptions.JSX.Pragma, which has adevelopment: boolfield initialized totrueby default. The code correctly assignstask.jsx = resolve_result.jsxinParseTask.init, and the.unspecifiedcase safely reads this pre-initialized value.
| import { expect, test } from "bun:test"; | ||
| import { bunEnv, bunExe, tempDir } from "harness"; | ||
|
|
||
| test("Bun.build respects NODE_ENV=production for JSX transform", async () => { | ||
| using dir = tempDir("test-jsx-prod", { | ||
| "test.tsx": `console.log(<div>Hello</div>);`, | ||
| }); | ||
|
|
||
| await using proc = Bun.spawn({ | ||
| cmd: [bunExe(), "build", "test.tsx", "--outfile=out.js", "--external", "react"], | ||
| env: { | ||
| ...bunEnv, | ||
| NODE_ENV: "production", | ||
| }, | ||
| cwd: String(dir), | ||
| stderr: "pipe", | ||
| stdout: "pipe", | ||
| }); | ||
|
|
||
| const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); | ||
|
|
||
| const output = await Bun.file(dir + "/out.js").text(); | ||
|
|
||
| // Should use production JSX runtime (jsx from react/jsx-runtime) | ||
| // NOT development runtime (jsxDEV from react/jsx-dev-runtime) | ||
| expect(output).toContain('from "react/jsx-runtime"'); | ||
| expect(output).toContain("jsx("); | ||
| expect(output).not.toContain("jsxDEV"); | ||
| expect(output).not.toContain('from "react/jsx-dev-runtime"'); | ||
|
|
||
| expect(exitCode).toBe(0); | ||
| }); | ||
|
|
||
| test("Bun.build uses development JSX transform when NODE_ENV=development", async () => { | ||
| using dir = tempDir("test-jsx-dev", { | ||
| "test.tsx": `console.log(<div>Hello</div>);`, | ||
| }); | ||
|
|
||
| await using proc = Bun.spawn({ | ||
| cmd: [bunExe(), "build", "test.tsx", "--outfile=out.js", "--external", "react"], | ||
| env: { | ||
| ...bunEnv, | ||
| NODE_ENV: "development", | ||
| }, | ||
| cwd: String(dir), | ||
| stderr: "pipe", | ||
| stdout: "pipe", | ||
| }); | ||
|
|
||
| const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); | ||
|
|
||
| const output = await Bun.file(dir + "/out.js").text(); | ||
|
|
||
| // Should use development JSX runtime | ||
| expect(output).toContain('from "react/jsx-dev-runtime"'); | ||
| expect(output).toContain("jsxDEV"); | ||
| expect(output).not.toContain('from "react/jsx-runtime"'); | ||
| expect(output).not.toContain("jsx("); | ||
|
|
||
| expect(exitCode).toBe(0); | ||
| }); | ||
|
|
||
| test("Bun.build defaults to development JSX when NODE_ENV is not set", async () => { | ||
| using dir = tempDir("test-jsx-default", { | ||
| "test.tsx": `console.log(<div>Hello</div>);`, | ||
| }); | ||
|
|
||
| const env = { ...bunEnv }; | ||
| delete env.NODE_ENV; | ||
|
|
||
| await using proc = Bun.spawn({ | ||
| cmd: [bunExe(), "build", "test.tsx", "--outfile=out.js", "--external", "react"], | ||
| env, | ||
| cwd: String(dir), | ||
| stderr: "pipe", | ||
| stdout: "pipe", | ||
| }); | ||
|
|
||
| const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); | ||
|
|
||
| const output = await Bun.file(dir + "/out.js").text(); | ||
|
|
||
| // Should default to development JSX runtime | ||
| expect(output).toContain('from "react/jsx-dev-runtime"'); | ||
| expect(output).toContain("jsxDEV"); | ||
| expect(output).not.toContain('from "react/jsx-runtime"'); | ||
| expect(output).not.toContain("jsx("); | ||
|
|
||
| expect(exitCode).toBe(0); | ||
| }); | ||
|
|
||
| test("Bun.build --production flag uses production JSX transform", async () => { | ||
| using dir = tempDir("test-jsx-flag", { | ||
| "test.tsx": `console.log(<div>Hello</div>);`, | ||
| }); | ||
|
|
||
| const env = { ...bunEnv }; | ||
| delete env.NODE_ENV; | ||
|
|
||
| await using proc = Bun.spawn({ | ||
| cmd: [bunExe(), "build", "--production", "test.tsx", "--outfile=out.js", "--external", "react"], | ||
| env, | ||
| cwd: String(dir), | ||
| stderr: "pipe", | ||
| stdout: "pipe", | ||
| }); | ||
|
|
||
| const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); | ||
|
|
||
| const output = await Bun.file(dir + "/out.js").text(); | ||
|
|
||
| // Should use production JSX runtime | ||
| expect(output).toContain("react/jsx-runtime"); | ||
| expect(output).not.toContain("jsxDEV"); | ||
|
|
||
| expect(exitCode).toBe(0); | ||
| }); | ||
|
|
||
| test("NODE_ENV=production overrides tsconfig.json jsx:react-jsx", async () => { | ||
| using dir = tempDir("test-jsx-tsconfig-override", { | ||
| "test.tsx": `console.log(<div>Hello</div>);`, | ||
| "tsconfig.json": JSON.stringify({ | ||
| compilerOptions: { | ||
| jsx: "react-jsx", | ||
| }, | ||
| }), | ||
| }); | ||
|
|
||
| await using proc = Bun.spawn({ | ||
| cmd: [bunExe(), "build", "test.tsx", "--outfile=out.js", "--external", "react"], | ||
| env: { | ||
| ...bunEnv, | ||
| NODE_ENV: "production", | ||
| }, | ||
| cwd: String(dir), | ||
| stderr: "pipe", | ||
| stdout: "pipe", | ||
| }); | ||
|
|
||
| const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); | ||
|
|
||
| const output = await Bun.file(dir + "/out.js").text(); | ||
|
|
||
| // NODE_ENV=production should override tsconfig and use production JSX runtime | ||
| expect(output).toContain('from "react/jsx-runtime"'); | ||
| expect(output).toContain("jsx("); | ||
| expect(output).not.toContain("jsxDEV"); | ||
| expect(output).not.toContain('from "react/jsx-dev-runtime"'); | ||
|
|
||
| expect(exitCode).toBe(0); | ||
| }); | ||
|
|
||
| test("NODE_ENV=production overrides tsconfig.json jsx:react-jsxdev", async () => { | ||
| using dir = tempDir("test-jsx-prod-override-jsxdev", { | ||
| "test.tsx": `console.log(<div>Hello</div>);`, | ||
| "tsconfig.json": JSON.stringify({ | ||
| compilerOptions: { | ||
| jsx: "react-jsxdev", | ||
| }, | ||
| }), | ||
| }); | ||
|
|
||
| await using proc = Bun.spawn({ | ||
| cmd: [bunExe(), "build", "test.tsx", "--outfile=out.js", "--external", "react"], | ||
| env: { | ||
| ...bunEnv, | ||
| NODE_ENV: "production", | ||
| }, | ||
| cwd: String(dir), | ||
| stderr: "pipe", | ||
| stdout: "pipe", | ||
| }); | ||
|
|
||
| const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); | ||
|
|
||
| const output = await Bun.file(dir + "/out.js").text(); | ||
|
|
||
| // NODE_ENV=production should override tsconfig react-jsxdev and force production runtime | ||
| expect(output).toContain('from "react/jsx-runtime"'); | ||
| expect(output).toContain("jsx("); | ||
| expect(output).not.toContain("jsxDEV"); | ||
| expect(output).not.toContain('from "react/jsx-dev-runtime"'); | ||
|
|
||
| expect(exitCode).toBe(0); | ||
| }); | ||
|
|
||
| test("NODE_ENV=development overrides tsconfig.json jsx:react-jsx", async () => { | ||
| using dir = tempDir("test-jsx-dev-override-jsx", { | ||
| "test.tsx": `console.log(<div>Hello</div>);`, | ||
| "tsconfig.json": JSON.stringify({ | ||
| compilerOptions: { | ||
| jsx: "react-jsx", | ||
| }, | ||
| }), | ||
| }); | ||
|
|
||
| await using proc = Bun.spawn({ | ||
| cmd: [bunExe(), "build", "test.tsx", "--outfile=out.js", "--external", "react"], | ||
| env: { | ||
| ...bunEnv, | ||
| NODE_ENV: "development", | ||
| }, | ||
| cwd: String(dir), | ||
| stderr: "pipe", | ||
| stdout: "pipe", | ||
| }); | ||
|
|
||
| const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); | ||
|
|
||
| const output = await Bun.file(dir + "/out.js").text(); | ||
|
|
||
| // NODE_ENV=development should override tsconfig react-jsx and force development runtime | ||
| expect(output).toContain('from "react/jsx-dev-runtime"'); | ||
| expect(output).toContain("jsxDEV"); | ||
| expect(output).not.toContain('from "react/jsx-runtime"'); | ||
| expect(output).not.toContain("jsx("); | ||
|
|
||
| expect(exitCode).toBe(0); | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Optional: Consider reducing test boilerplate.
The seven tests follow a very similar pattern with minor variations in environment variables and assertions. Per coding guidelines, you may consider refactoring using describe.each or a helper function to reduce boilerplate.
However, given that each test has distinct setup requirements (different env vars, flags, tsconfig files) and varying assertions, the current explicit approach is clear and maintainable. This refactoring is purely optional.
Example approach if you wanted to refactor:
describe.each([
{
name: "respects NODE_ENV=production",
env: { NODE_ENV: "production" },
flags: [],
expectedRuntime: "production",
},
// ... other test cases
])("Bun.build JSX transforms", ({ name, env, flags, expectedRuntime }) => {
test(name, async () => {
// shared test logic
});
});🤖 Prompt for AI Agents
In test/regression/issue/23959.test.ts lines 1-219, the seven tests repeat
almost identical setup and run logic; refactor to reduce boilerplate by
parameterizing the differences (env, CLI flags, optional tsconfig file contents,
and expected assertions) using either describe.each with a cases array or a
single helper function that accepts a case object and performs the shared
tempDir/create file, Bun.spawn, await results, read output, and per-case
assertions; ensure the helper/cases still allow per-case custom assertions for
dev vs prod runtime checks and preserve distinct setups (NODE_ENV, --production
flag, tsconfig contents) so test semantics remain unchanged.
## Summary Fixes a bug where `Bun.build` would use development JSX transforms (`jsxDEV` from `react/jsx-dev-runtime`) even when `NODE_ENV=production` was set, instead of using production JSX transforms (`jsx` from `react/jsx-runtime`). ## Root Causes 1. **Missing force_node_env flag for production**: When `NODE_ENV=production` was set, the transpiler set `jsx.development = false` but did not set `force_node_env = .production`. This meant that later code couldn't distinguish between "production due to NODE_ENV" vs "default settings", causing tsconfig jsx settings to incorrectly override NODE_ENV. 2. **Incorrect RuntimeMap for react-jsx**: The `RuntimeMap` mapped `"react-jsx"` (production mode) to `.development = true`, which is incorrect. It should map to `.development = false`. 3. **Wrong fallback in bundle_v2**: When `force_node_env` was `.unspecified`, the bundler used the global `transpiler.options.jsx.development` instead of the per-module `task.jsx.development` which includes tsconfig settings. ## Changes 1. **src/transpiler.zig**: Set `force_node_env = .production` when `NODE_ENV=production` is detected, matching the behavior for `NODE_ENV=development`. 2. **src/options.zig**: Fix RuntimeMap to correctly map `"react-jsx"` to `.development = false` (production mode) instead of `.development = true`. 3. **src/bundler/bundle_v2.zig**: When `force_node_env` is `.unspecified`, use `task.jsx.development` (which includes per-module tsconfig settings) instead of the global `transpiler.options.jsx.development`. ## Configuration Priority The correct priority is now enforced: 1. `NODE_ENV=production/development` (highest - overrides all) 2. `--production` flag (sets NODE_ENV internally) 3. tsconfig.json `compilerOptions.jsx` (lowest - default fallback) ## Test Plan Added comprehensive regression tests in `test/regression/issue/23959.test.ts`: - ✅ `NODE_ENV=production` uses production JSX (`jsx`) - ✅ `NODE_ENV=development` uses development JSX (`jsxDEV`) - ✅ No NODE_ENV defaults to development JSX (`jsxDEV`) - ✅ `--production` flag uses production JSX (`jsx`) Fixes #23959
robobun
force-pushed
the
claude/fix-jsx-production-mode
branch
from
c8b1b6d to
d0540ff
Compare
1 participant
Fixes #23959
Summary
Fixes a bug where
Bun.buildwould use development JSX transforms (jsxDEVfromreact/jsx-dev-runtime) even whenNODE_ENV=productionwas set, instead of using production JSX transforms (jsxfromreact/jsx-runtime).Root Causes
Missing force_node_env flag for production: When
NODE_ENV=productionwas set, the transpiler setjsx.development = falsebut did not setforce_node_env = .production. This meant that later code couldn't distinguish between "production due to NODE_ENV" vs "default settings", causing tsconfig jsx settings to incorrectly override NODE_ENV.Incorrect RuntimeMap for react-jsx: The
RuntimeMapmapped"react-jsx"(production mode) to.development = true, which is incorrect. It should map to.development = false.Wrong fallback in bundle_v2: When
force_node_envwas.unspecified, the bundler used the globaltranspiler.options.jsx.developmentinstead of the per-moduletask.jsx.developmentwhich includes tsconfig settings.Changes
src/transpiler.zig: Set
force_node_env = .productionwhenNODE_ENV=productionis detected, matching the behavior forNODE_ENV=development.src/options.zig: Fix RuntimeMap to correctly map
"react-jsx"to.development = false(production mode) instead of.development = true.src/bundler/bundle_v2.zig: When
force_node_envis.unspecified, usetask.jsx.development(which includes per-module tsconfig settings) instead of the globaltranspiler.options.jsx.development.Configuration Priority
The correct priority is now enforced:
NODE_ENV=production/development(highest - overrides all)--productionflag (sets NODE_ENV internally)compilerOptions.jsx(lowest - default fallback)Test Plan
Added comprehensive regression tests in
test/regression/issue/23959.test.ts:NODE_ENV=productionuses production JSX (jsx)NODE_ENV=developmentuses development JSX (jsxDEV)jsxDEV)--productionflag uses production JSX (jsx)All tests pass with the fix applied.