Skip to content

bun build --production: override ambient NODE_ENV instead of defaulting - #33855

Closed
robobun wants to merge 4 commits into
mainfrom
claude/build-production-node-env
Closed

robobun wants to merge 4 commits into
mainfrom
claude/build-production-node-env

Conversation

@robobun

@robobun robobun commented Jul 9, 2026 •

Copy link
Copy Markdown
Collaborator

What does this PR do?

bun build --help documents --production as "Set NODE_ENV=production and enable minification", and the bundler docs say the flag "Sets process.env.NODE_ENV to production". But the flag was implemented as a default rather than an override, so an ambient NODE_ENV in the build process environment (common in dev shells and CI test jobs) silently won and the bundle inlined the wrong value.

Repro

// in.js
console.log(process.env.NODE_ENV);
$ NODE_ENV=development bun build in.js --production
console.log("development");   # should be "production"

Observed precedence before this change:

--define process.env.NODE_ENV=... > ambient NODE_ENV > --production > "development"

Cause

build_command.rs wrote NODE_ENV=production into the dotenv loader's map before configure_defines() ran load_process(), which then overwrote that entry from the real process environment. load_defines() in src/bundler/options.rs consults the env loader first and only falls back to self.production when the loader has no NODE_ENV at all.

Fix

  • When --production is passed, set options.production = true and options.force_node_env = ForceNodeEnv::Production before configure_defines(), so the flag's intent survives env loading.
  • load_defines() now honors force_node_env ahead of the env loader when resolving the process.env.NODE_ENV define.
  • run_env_loader() re-asserts NODE_ENV=production in the env map after load_process() when force_node_env is Production, so .env.production (not .env.test/.env.development) is loaded when --env is also passed.

New precedence:

--define process.env.NODE_ENV=... > --production > ambient NODE_ENV > "development"

An explicit --define process.env.NODE_ENV=... still wins over --production (covered by a test).

This also fixes a secondary symptom: with an ambient NODE_ENV=development, bun build --production was appending the "development" export condition, pulling in dev-only package entries.

How did you verify your code works?

Added itBundled tests in test/bundler/bundler_edgecase.test.ts covering --production with ambient NODE_ENV unset / production / development / test / empty, plus a test asserting --define still wins.

Fail-before (system bun): development/test/empty cases fail with "development" / "test" in the bundle.
Pass-after (bun bd test): all 6 pass; full edgecase suite (116 tests) green.


no test proof · iteration 1 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/bundler/bundler_edgecase.test.ts

`bun build --help` documents `--production` as "Set NODE_ENV=production",
but the flag was implemented as a default rather than an override: the
NODE_ENV write happened before load_process() ingested the real process
environment into the same map, so an ambient NODE_ENV=development (common
in dev shells and CI test jobs) silently won and the bundle inlined
process.env.NODE_ENV as "development".

Set options.production and options.force_node_env = Production when the
flag is passed, and teach load_defines to honor force_node_env ahead of
the env loader. An explicit --define process.env.NODE_ENV=... still wins
over the flag.
@coderabbitai

coderabbitai Bot commented Jul 9, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

Adds force_node_env handling so --production drives NODE_ENV and BUN_ENV to "production" in define resolution and env loading. Build command wiring now sets the flag on transpiler and resolver options, and bundler tests cover ambient env precedence and explicit define override.

Changes

Force production NODE_ENV

Layer / File(s) Summary
Defines and env loader honor force_node_env
src/bundler/options.rs, src/bundler/transpiler.rs
load_defines returns "production"/"development" from force_node_env, and run_env_loader writes NODE_ENV and BUN_ENV as "production" when force_node_env is Production.
Build command sets force_node_env on --production
src/runtime/cli/build_command.rs
BuildCommand::exec sets production mode and force_node_env = Production on both transpiler options and resolver opts for --production.
Edge-case tests for NODE_ENV precedence
test/bundler/bundler_edgecase.test.ts
New tests assert production: true forces NODE_ENV to "production" across ambient env values, and that an explicit define for process.env.NODE_ENV overrides production: true.
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly describes the main change: --production now overrides ambient NODE_ENV.
Description check ✅ Passed The description matches the template and includes purpose, repro, fix, and verification details.

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added the claude label Jul 9, 2026
@robobun

robobun commented Jul 9, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 12:08 PM PT - Jul 9th, 2026

❌ @robobun, your commit 2371f63 has 4 failures in Build #71087 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 33855

That installs a local version of the PR into your bun-33855 executable, so you can run:

bun-33855 --bun

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Found 3 issues this PR may fix:

  1. Unexpected "development" import condition in bun build #16904 - Fixes the "development" export condition being appended even when --production is passed, because ambient NODE_ENV was winning over the flag
  2. Priority issue of environment variable modes when bundling the project with Bun #13614 - Fixes the priority/precedence of environment variable modes when bundling, where ambient NODE_ENV from .env files would override the intended build mode
  3. Different result from build with --production and without #24747 - Likely fixes different build results with --production for libraries using exports conditions (like AWS SDK), caused by the wrong export condition being selected

If this is helpful, copy the block below into the PR description to auto-close these issues on merge.

Fixes #16904
Fixes #13614
Fixes #24747

🤖 Generated with Claude Code

Comment thread src/bundler/transpiler.rs Resolved
get_node_env() consults BUN_ENV before NODE_ENV, so an ambient
BUN_ENV=test would still leak through for .env suffix selection.
Write both after load_process() and extend the test matrix to cover
the BUN_ENV alias.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/bundler/transpiler.rs`:
- Around line 739-742: The DotEnvBehavior::disable branch in
transpiler::load_defines is missing the same forced NODE_ENV/BUN_ENV reassertion
used in the prefix/load_all paths. After load_process() and the force_node_env
== Production check, update env.map in the disable arm too so
env.get(b"NODE_ENV") and env.get(b"BUN_ENV") stay set to production for all
consumers. Use the existing load_process()/force_node_env logic in load_defines
as the reference point and mirror the env.map.put behavior there.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: cebf466b-ab78-45c3-80f0-0deba402bd57

📥 Commits

Reviewing files that changed from the base of the PR and between fc865b3 and f508f1d.

📒 Files selected for processing (4)
  • src/bundler/options.rs
  • src/bundler/transpiler.rs
  • src/runtime/cli/build_command.rs
  • test/bundler/bundler_edgecase.test.ts

Comment thread src/bundler/transpiler.rs Resolved

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My earlier feedback (BUN_ENV sibling) and CodeRabbit's (disable arm) are both addressed and the change looks correct, but this is a user-visible precedence change to --production and the bundler's define/env resolution — worth a quick human sign-off.

Extended reasoning...

Overview

This PR changes bun build --production so it overrides an ambient NODE_ENV/BUN_ENV in the process environment instead of merely defaulting it. Touched files:

  • src/runtime/cli/build_command.rs — under --production, call set_production(true) and set force_node_env = Production on both bundler and resolver options before configure_defines().
  • src/bundler/options.rs — load_defines() now short-circuits on force_node_env before consulting the env loader for process.env.NODE_ENV.
  • src/bundler/transpiler.rs — after load_process() in both the prefix|load_all|... and disable arms of run_env_loader(), re-assert NODE_ENV=production and BUN_ENV=production in the env map when force_node_env == Production.
  • test/bundler/bundler_edgecase.test.ts — a 2×5 itBundled matrix (NODE_ENV/BUN_ENV × unset/production/development/test/empty) plus a --define-still-wins case.

Security risks

None identified. No auth, crypto, permissions, or untrusted-input parsing is touched; the change only adjusts which build-time constant is inlined for process.env.NODE_ENV and which .env.* suffix is loaded.

Level of scrutiny

Moderate. This is a user-visible semantic change to the precedence of --production vs ambient env in production builds — arguably a bugfix (behavior now matches --help and docs), but it does alter output for anyone who was relying on ambient NODE_ENV winning. It also threads through force_node_env, which fans out to jsx.development in bundle_v2.rs and gates set_production() (options.rs:1436). I traced call ordering in build_command.rs — set_production(true) runs while force_node_env is still Unspecified, then force_node_env is locked, so production=true is actually applied; downstream set_production calls in run_env_loader become no-ops, which is the intent. The later explicit jsx.development = false under --production is unchanged, so JSX behavior is preserved.

Other factors

  • My prior inline review flagged the BUN_ENV sibling; addressed in f508f1d with the test matrix extended to cover it. CodeRabbit's DotEnvBehavior::disable mirror was addressed in 94db7a1. All threads resolved.
  • No CODEOWNERS entries for these paths.
  • Tests follow the existing itBundled conventions in bundler_edgecase.test.ts and include a negative check that --define still wins.
  • Bug-hunting system found no issues on the latest revision.

Not approving solo because this is a deliberate behavior/precedence change to a core bundler flag rather than a mechanical fix; a maintainer should confirm the new precedence (--define > --production > ambient env > default) is the intended contract.

@robobun

robobun commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator Author

The diff is green. The new bundler tests pass on all lanes.

Remaining CI failures are unrelated Windows flakes (also seen on other branches, e.g. build 71081):

  • test/js/sql/postgres-binary-array-bounds.test.ts on win 2019 x64/x64-baseline: ERR_POSTGRES_CONNECTION_REFUSED from the mock server (not a protocol assertion failure)
  • test/napi/napi.test.ts on win 2019 x64: napi_wrap > has the right lifetime GC condition not met after 100 attempts

Neither touches the bundler or env-loading paths in this change. Ready for review.

@github-actions

github-actions Bot commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

Closing this PR because it has been inactive for more than 90 days.

@robobun

robobun commented Oct 13, 2026

Copy link
Copy Markdown
Collaborator Author

Superseded by #45101. It fixes the same bug on current main with a different mechanism: the flag writes NODE_ENV after the process environment loads, and no shared bundler code changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant