Repository navigation
Remove the dead --dump-environment-variables flag from bun build - #44370
Conversation
`bun build` declares the flag, but no code reads it since bun 1.0.12 (#6395). `DebugOptions.dump_environment_variables` is never set, so the branch in `BuildCommand::exec` and `Transpiler::dump_environment_variables` cannot run. Delete the param, the field, the branch and the function. Remove the flag from the zsh and bash completions, together with `--dump-limits` and `--disable-bun-js`. Those are the two other debug flags of the old `DebugOptions`, and #36184 deleted their fields.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: oven-sh/bun/.coderabbit.yaml Review profile: ASSERTIVE Plan: Essentials Run ID: 📒 Files selected for processing (6)
💤 Files with no reviewable changes (4)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 4 remain after this review. WalkthroughThe build CLI no longer exposes the environment-dump option or its output path. Bash and Zsh completion lists also omit selected long options. ChangesEnvironment dump option removal
Suggested reviewers: Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to The removed option had no effect on build output, and the completion changes remove stale entries. No material merge risk is established by the inspected changes. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
|
Status: ready for review. Reproduction (bun 1.4.3 and mkdir t && cd t && echo 'console.log("hi")' > x.js
bun build --dump-environment-variables ./x.jsThe command prints the bundle and exits 0, the same as without the flag. This PR (#44370) removes the flag and the code that only the flag could reach. No maintainer has ruled on the flag. If a maintainer wants the flag connected instead, close this PR. |
There was a problem hiding this comment.
LGTM, a straightforward dead-code removal.
What was reviewed:
- Confirmed the deleted
DebugOptions.dump_environment_variablesfield, theBuildCommandbranch, andTranspiler::dump_environment_variableshave no remaining references anywhere in src/, test/, or docs/ at HEAD, so no struct-literal or call site is left dangling. - Checked the
--dump-environment-variables=valuebehavior change (exit 1 before, silently skipped now): the flag was never read, so this only affects an error path for a non-functional flag. - Checked the broader completions cleanup (
--dump-limits,--disable-bun-js): neither name appears in any param table in src/, so the completion entries were already stale.
Extended reasoning...
The change deletes an unread CLI param, its DebugOptions field and default, the unreachable early-return in build_command.rs, the Transpiler method it called, and the corresponding shell completion entries, for +2/-45 across six files. It touches no security-sensitive surface. A repo-wide grep at HEAD finds no remaining references to any of the removed identifiers or flag names, and none of the changed paths fall under a CODEOWNERS entry. The only behavioral delta is that passing the flag with a value no longer errors, which is an acceptable consequence of removing an inert flag.
Behaviour change: none
One error path differs. See Downsides.
Problem
bun build --dump-environment-variablesdoes nothing.BUILD_ONLY_PARAMSdeclares the flag (src/runtime/cli/Arguments.rs:538), but no code reads it.DebugOptions.dump_environment_variablesis alwaysfalse, so the branch atsrc/runtime/cli/build_command.rs:604andTranspiler::dump_environment_variablescannot run.--dump-limitsand--disable-bun-js, whose fields Narrow crate-internal Rust visibility across all targets and delete the code it proves dead #36184 deleted.Fix
src/clap/streaming.rs:157). The bare flag gives the same bundle as before.bun bd,test/bundler/cli.test.ts,bundler_env.test.ts,test/cli/bun.test.ts,test/cli/run/env.test.ts. No new test:REVIEW.mdsays "Do not add tests to check dead code stays dead".Background
bun buildprinted valid JSON up to bun 0.8.0, a malformed stream up to 1.0.11, and nothing since 1.0.12 (CLI flags #6395)..envvalue without--env. The docs givebun --print process.env.Downsides
bun build --dump-environment-variables=valueexited 1 with "does not take a value". It is now skipped.Notes
Deleted, one line each
parse_param!("--dump-environment-variables")inBUILD_ONLY_PARAMS(src/runtime/cli/Arguments.rs).DebugOptions.dump_environment_variablesand its default (src/options_types/context.rs).if ctx.debug.dump_environment_variables { ... }branch inBuildCommand::exec, and the stale comment above it (src/runtime/cli/build_command.rs).Transpiler::dump_environment_variables(src/bundler/transpiler.rs).write_json_stringand the iterator ofbun_dotenv::Maphave other callers.completions/bun.zsh: the--dump-environment-variablesand--dump-limitsspecs in_bun_run_completion.completions/bun.bash:--dump-environment-variables --dump-limits --disable-bun-jsinGLOBAL_OPTIONS.Behaviour, measured
Released bun 1.4.3 against a debug build of this branch. "Same bundle" means the same bytes on stdout as
bun build x.js.bun build --dump-environment-variables x.jsbun build --dump-environment-variables=1 x.jsbun build --dump-environment-variables= x.jsBUN_OPTIONS=--dump-environment-variables=1 bun build x.jsbun build --no-such-flag=1 x.jsbun build --helpnever listed the flag.WARN_ON_UNRECOGNIZED_FLAG(src/clap/streaming.rs:10) is never set to true, so the skip prints nothing. This PR does not touch it. #40560 removes it.History
38f83c50c4and5691bf385b(October 2021) added the flag for the oldbun bunandbun devcommands.bun build --dump-environment-variables x.js: 0.8.0 prints valid JSON. 1.0.1 and 1.0.11 print one JSON string per fragment ("{",then"\n ",then"PATH",), which is not valid JSON. The cause is19aa9d93de(update zig to 0.11.0 #4233, first in 0.8.1):Map.jsonStringifychanged fromwriter.writeAlltowriter.writeon the JSON stream.debug_params, andsrc/cli.zig:593read it for every command. CLI flags #6395 deleted that list and added a new declaration, with no help text, tobuild_only_paramsonly.5bf8ad8fb9commented out the line that reads the flag, under a source comment that says the two dump flags were "only used in bun dev".fbbb5e7e38("Remove comments") deleted the commented lines. The merged commit does not contain that comment.bun buildhad its own consumer (src/cli/build_command.zig:220), and it stayed.bun runon bun 1.0.2, and it ended with "This flag appears to have been removed".DebugOptions.dump_limitsandDebugOptions.fallback_only, the fields of the other two debug flags. jsx: remove the dead Solid runtime from bunfig, the CLI, and tsconfig #41699 removed the dead Solid JSX runtime in the same way.If the flag is wanted back
The one-line reader is not enough.
bun build --appreturns intobake::production::build_commandbefore the branch. The branch runs after the compile target lookup, so it must move to directly afterconfigure_defines().--watchneeds a decision. The flag needs help text and docs.Completions: what stays
DebugOptionsdebug flags. After this PR none of their fields exists.completions/bun.bashGLOBAL_OPTIONSstill has nine names that no param table declares:--use --bunfile --server-bunfile --disable-react-fast-refresh --disable-hmr --jsx-production --platform --public-dir --inject. completions: generate fish/bash from bun-cli.json #35443 generates that file frombun-cli.json. completions: optimize parsing, add --cwd support, and fix bash 3.2 compatibility #42275 rewrites the same line._bun_run_completionincompletions/bun.zshstill has eleven specs thatbun rundoes not declare:--no-summary --version -v --revision --external --packages --minify --minify-syntax --minify-whitespace --minify-identifiers --target. Other commands declare them.DebugOptionsfields with no reader that stayoutput_file: Remove dead code: 17 log scopes nothing logs to and four fields nothing reads, in 10 crates #44050 removes it.editor: Bun.openInEditor: use the [debug] editor setting from bunfig.toml #38059 connects it.package_bundle_map:[bundle.packages]inbunfig.tomlfills it (src/bunfig/bunfig.rs:910) and nothing reads it. No open PR covers it.Other params with no reader
207 long flags are declared in
Arguments.rs. 13 have nob"--name"reader insrc/**/*.rs:--tls-min-v1.0,--tls-min-v1.1,--tls-min-v1.2,--tls-max-v1.3:src/js/node/tls.tsreads them fromprocess.execArgv.--trace-events-enabledand six more--trace-*flags: declared on purpose, see the comment above them.--grep: an alias of--test-name-pattern.--dump-environment-variables: the only one with no reader and no stated reason.Checks
test/bundler/cli.test.ts: 38 pass, 0 fail.test/bundler/bundler_env.test.ts: 7 pass, 0 fail.test/cli/bun.test.ts: 39 pass, 0 fail.test/cli/run/env.test.ts: 107 pass, 0 fail. All with--timeout 120000, because the machine was under heavy load.bash -n completions/bun.bashpasses. zsh was not available, socompletions/bun.zshwas checked by reading. The new last line of the_argumentscall ends with' &&, like the other calls in the file.dump-environment-variablesstring. The completion scripts are embedded withinclude_bytes!, sobun completionsinstalls the edited scripts.main: no target grows. Four of the twelve targets are 2 to 4 KB smaller (FreeBSD x64 and aarch64, Windows x64 and aarch64).bun --print process.envprints the process variables and the.envvalues.logLevel = "debug"inbunfig.tomlmakesbun build --env inlineprint the.envfiles that it loads.no test proof · iteration 1 · the description declares no behaviour change, so there is no failing test to prove; the existing suite in CI is the check