Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
7 changes: 6 additions & 1 deletion src/bundler/transpiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -542,6 +542,10 @@

let env_loader = self.env_mut();
let mut is_production = env_loader.is_production();
// `bun run` uses `LoadAllWithoutInlining`, which skips injecting env
// `NODE_ENV` into `define.dots` — so the define-map check below never
// sees it. Sample development here too so it is symmetric with production.
Comment thread
robobun marked this conversation as resolved.
Outdated
let mut is_development = env_loader.get_node_env() == Some(b"development");

// `load_defines` injects a default `process.env.NODE_ENV`; sample the
// explicit sources first so that default isn't mistaken for user intent
Expand Down Expand Up @@ -572,17 +576,18 @@
// inside the `&mut self` scope without `unsafe`.
self.options.load_defines(self.arena, Some(env_loader))?;

let mut is_development = false;
if had_explicit_node_env {
if let Some(node_env) = self.options.define.dots.get(b"NODE_ENV".as_slice()) {
if !node_env.is_empty() {
if let Some(s) = node_env[0].data.value.e_string() {
if s.eql_comptime(b"production") {
is_production = true;
is_development = false;
} else if s.eql_comptime(b"development") {
is_development = true;
is_production = false;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
}

Check warning on line 590 in src/bundler/transpiler.rs

View check run for this annotation

Claude / Claude Code Review

--define NODE_ENV=<non-prod/non-dev> no longer clears env-derived is_development

Minor edge case: with env `NODE_ENV=development` + `--define process.env.NODE_ENV='"test"'` + tsconfig `"react-jsx"`, the env-seeded `is_development=true` now survives (since `"test"` matches neither `eql_comptime` branch) and forces the dev runtime — pre-PR this fell through to tsconfig. Note the mirror case (env `NODE_ENV=production` + `--define="test"`) already leaked env→prod before this PR via `is_production = env_loader.is_production()`, so this makes dev symmetric with a pre-existing quir
Comment thread
robobun marked this conversation as resolved.
}
}
}
Expand Down
51 changes: 51 additions & 0 deletions test/bundler/transpiler/jsx-tsconfig-react-jsx.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,57 @@
"m.jsx": `const a = <div p="1">x</div>;\nglobalThis.s = a;\n`,
};

// `NODE_ENV` in the environment overrides tsconfig dev/prod for `bun run` in
// both directions; `--define process.env.NODE_ENV` overrides both.
describe.each(["NODE_ENV", "BUN_ENV"])("bun run: env %s overrides tsconfig jsx dev/prod", envVar => {
test.concurrent.each([
// [tsconfig jsx, env value, expected runtime]
["react-jsx", "development", "dev jsxDEV"], // env wins (was: tsconfig won -> prod)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
["react-jsx", "production", "prod jsx"], // both agree
["react-jsxdev", "development", "dev jsxDEV"], // both agree
["react-jsxdev", "production", "prod jsx"], // env wins
] as const)(`tsconfig "%s" + ${envVar}=%s -> %s`, async (jsx, envValue, expected) => {
using dir = tempDir("jsx-tsconfig-env", {
...shimFiles,
"tsconfig.json": JSON.stringify({ compilerOptions: { jsx, jsxImportSource: "shim" } }),
});
await using proc = Bun.spawn({
cmd: [bunExe(), "run", "m.jsx"],
env: { ...bunEnv, NODE_ENV: undefined, BUN_ENV: undefined, [envVar]: envValue },
cwd: String(dir),
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect({ stderr, stdout: stdout.trim(), exitCode }).toEqual({ stderr: "", stdout: expected, exitCode: 0 });
});
});

describe("bun run: --define process.env.NODE_ENV overrides env NODE_ENV for jsx dev/prod", () => {
test.concurrent.each([
// [env NODE_ENV, --define value, tsconfig jsx, expected]
["development", '"production"', "react-jsxdev", "prod jsx"], // --define > env > tsconfig
["production", '"development"', "react-jsx", "dev jsxDEV"], // --define > env > tsconfig
] as const)(
"env NODE_ENV=%s + --define process.env.NODE_ENV=%s -> %s",
async (envValue, defineValue, jsx, expected) => {

Check warning on line 64 in test/bundler/transpiler/jsx-tsconfig-react-jsx.test.ts

View check run for this annotation

Claude / Claude Code Review

Test title third %s binds to tsconfig value, not expected output

The title format string has 3 `%s` placeholders but the tuple has 4 elements `[envValue, defineValue, jsx, expected]`, so `-> %s` binds to `jsx` (e.g. `react-jsxdev`) rather than `expected` (e.g. `prod jsx`). The generated test name reads `... -> react-jsxdev`, which looks like the expected runtime output but is actually the tsconfig value — compare line 40 where `-> %s` correctly binds to `expected`. Consider `"env NODE_ENV=%s + --define process.env.NODE_ENV=%s (tsconfig %s) -> %s"` so the arro
Comment thread
robobun marked this conversation as resolved.
using dir = tempDir("jsx-tsconfig-define", {
...shimFiles,
"tsconfig.json": JSON.stringify({ compilerOptions: { jsx, jsxImportSource: "shim" } }),
});
await using proc = Bun.spawn({
cmd: [bunExe(), "run", "--define", `process.env.NODE_ENV=${defineValue}`, "m.jsx"],
env: { ...bunEnv, NODE_ENV: envValue, BUN_ENV: undefined },
cwd: String(dir),
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect({ stderr, stdout: stdout.trim(), exitCode }).toEqual({ stderr: "", stdout: expected, exitCode: 0 });
},
);
});

describe("tsconfig compilerOptions.jsx", () => {
test.each([
["react-jsx", "prod jsx", "shim/jsx-runtime"],
Expand Down
Loading