Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/runtime/cli/Arguments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1448,8 +1448,8 @@
} else {
api::JsxRuntime::Automatic
},
development: false,
development: true,
side_effects: jsx_side_effects,

Check notice on line 1452 in src/runtime/cli/Arguments.rs

View check run for this annotation

Claude / Claude Code Review

--jsx-side-effects passed alone is silently ignored

Pre-existing, but while you're in this block: the gate at lines 1433–1437 checks `jsx_factory`/`jsx_fragment`/`jsx_import_source`/`jsx_runtime` but not `jsx_side_effects` (read at line 1389), so `bun --jsx-side-effects a.jsx` with no other `--jsx-*` flag skips the block entirely and the flag is silently dropped. Adding `|| jsx_side_effects` to the gate would close this sibling of the same "--jsx-* flag doesn't take effect" class this PR is fixing.
Comment thread
robobun marked this conversation as resolved.
});
} else {
let prev = opts.jsx.take().unwrap();
Expand All @@ -1464,7 +1464,7 @@
} else {
prev.runtime
},
development: false,
development: prev.development,
side_effects: jsx_side_effects,
});
}
Expand Down
62 changes: 62 additions & 0 deletions test/bundler/transpiler/jsx-cli-flags.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
import { describe, expect, test } from "bun:test";
import { bunEnv, bunExe, tempDir } from "harness";

// Passing a --jsx-* CLI flag must not flip the automatic runtime from
// jsx-dev-runtime (development) to jsx-runtime (production). The flag only
// overrides the field it names; dev/prod selection follows NODE_ENV exactly as
// it does when no --jsx-* flag is present.
describe.concurrent("jsx: --jsx-* CLI flags preserve development runtime", () => {
Comment thread
robobun marked this conversation as resolved.
const shimFiles = {
"node_modules/react/package.json": JSON.stringify({
name: "react",
version: "1.0.0",
exports: {
".": "./index.js",
"./jsx-runtime": "./prod.js",
"./jsx-dev-runtime": "./dev.js",
},
}),
"node_modules/react/index.js": "module.exports = {};",
"node_modules/react/prod.js":
"exports.jsx = () => ({ rt: 'PROD' }); exports.jsxs = exports.jsx; exports.Fragment = {};",
"node_modules/react/dev.js": "exports.jsxDEV = () => ({ rt: 'DEV' }); exports.Fragment = {};",
"a.jsx": "console.log(JSON.stringify(<div/>));",
};

const cases: Array<[extraArgs: string[], nodeEnv: string | undefined, expected: "DEV" | "PROD"]> = [
// Baselines (no --jsx-* flags): dev by default, prod only when NODE_ENV=production.
[[], undefined, "DEV"],
[[], "development", "DEV"],
[[], "production", "PROD"],
// Any --jsx-* flag must not change the dev/prod selection.
[["--jsx-import-source=react"], undefined, "DEV"],
[["--jsx-import-source=react"], "development", "DEV"],
[["--jsx-import-source=react"], "production", "PROD"],
[["--jsx-fragment=Fragment"], undefined, "DEV"],
[["--jsx-fragment=Fragment"], "development", "DEV"],
[["--jsx-fragment=Fragment"], "production", "PROD"],
[["--jsx-factory=h"], undefined, "DEV"],
[["--jsx-runtime=automatic"], undefined, "DEV"],
[["--jsx-runtime=automatic"], "production", "PROD"],
];

Check failure on line 41 in test/bundler/transpiler/jsx-cli-flags.test.ts

View check run for this annotation

Claude / Claude Code Review

Bunfig-merge branch (line 1467, prev.development) has no test coverage

The bunfig-merge branch change at Arguments.rs:1467 (`development: prev.development`) has no test coverage — every case in this file creates a tempDir with no `bunfig.toml`, so `opts.jsx` is always `None` and only the fresh-construct branch (line 1451) is exercised. Reverting line 1467 back to `development: false` breaks none of these 12 tests, which REVIEW.md's "Confirm deleting each load-bearing clause of your fix breaks at least one test" rule flags as a rejected pattern. Adding one case with
Comment thread
robobun marked this conversation as resolved.
Outdated
Comment thread
coderabbitai[bot] marked this conversation as resolved.

for (const [extraArgs, nodeEnv, expected] of cases) {
const label = `bun ${extraArgs.join(" ") || "(no flags)"} NODE_ENV=${nodeEnv ?? "<unset>"} -> ${expected}`;
test(label, async () => {
using dir = tempDir("jsx-cli-dev", shimFiles);
const env: Record<string, string | undefined> = { ...bunEnv, NODE_ENV: nodeEnv };
if (nodeEnv === undefined) delete env.NODE_ENV;
await using proc = Bun.spawn({
cmd: [bunExe(), ...extraArgs, "a.jsx"],
env,
cwd: String(dir),
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stderr).toBe("");
expect(stdout.trim()).toBe(JSON.stringify({ rt: expected }));
expect(exitCode).toBe(0);
});
}
});
Loading