[robobun]

Fixes #12067

Problem

• With a macro hello, let s = hello(); s += " World" fails under bun run and --minify-syntax: SyntaxError: Left side of assignment is not a reference.
• visit_decl (src/js_parser/visit/mod.rs:740) put a macro result into the inliner table for let and var too, and for an undefined pattern leaf that has a default.
• The destructuring prune (mod.rs:655) edited the macro's own object. const { og: { title }, og } = meta() left og.image undefined in every build mode.

Fix

• const_values holds a ConstValue. Its one constructor admits only a const, and no undefined leaf that has a default. admit_const_value is the only writer.
• The prune returns a new node. It keeps the value of a key that the pattern binds again, and each property when a key is computed.
• Verified: test/bundler/transpiler/macro-test.test.ts (7 new tests, all fail on main).
• Self-reviewed: 21 concerns raised, 14 addressed here, 7 moved to js_parser: keep a const macro object or array as one value, and fold macro().prop only for a read #46924 (Notes).

Background

• const_values is the inliner table. handle_identifier prints the stored value in place of each read.
• A macro call becomes a literal. A pattern on it drops the properties that it does not bind.
• Not here: a const macro object is still copied at each read ([macro] minify breaks object equality #7196). That removal changes documented output. It is js_parser: keep a const macro object or array as one value, and fold macro().prop only for a read #46924.

Downsides

• A let or var macro result is read at run time. var DEV = isDev(); if (DEV) require("./dev.ts") is 1,576 B under --minify-syntax (main: 8 B). import(home) of a let is not bundled.
• let h = hello(); id(h) is a build error under bun run too.
• 0.02 to 0.7 more instructions per declarator. Text: +1,280 B.

Notes

All numbers come from release builds of one base: main 179664ea89 and this branch. Linux x64.

What each mechanism broke, and where.

Mechanism on main	Example	Wrong under
A let or var macro result is printed at each read	let s = hello(); s += " World"	bun run, --minify-syntax, --minify (the module does not load)
A pattern leaf with a default takes the undefined leaf	const [x = 5] = macro() with [undefined] gives undefined	the same three modes
A computed, joined or number pattern key loses its property	const { [key()]: v } = macro() gives undefined	every mode, plain bun build included
A key that the pattern binds two times is narrowed for both	const { og: { title }, og } = meta(): og.image is undefined	every mode
One object that the macro returns at two places is narrowed for both	const { p: { a }, q } = twice(): q is { a: 1 }	every mode

Rows. 118 one-line programs ran with and without with { type: "macro" } at three doors: bun run, bun build --target=bun, bun build --target=bun --minify-syntax. A cell is right when stdout and the exit code equal the plain import on the same binary.

	bun run	bun build	--minify-syntax	cells
main	44	104	44	192 of 354
this PR	69	105	69	243 of 354

The rows that still differ are the const object and array rows of #7196, 3 rows that read a const in its temporal dead zone, and the rows that pass a name to a macro. 4 cells are right on main and wrong here: let h = hello(); id(h) and the same with var, under bun run and --minify-syntax.

What changes for a program that had no problem.

Case	main	this PR
let n = num(); console.log(n + n, n * 2), --minify	28 B, folded	39 B
let t = table(); console.log(t[0], t[1]) with a 1,000-row table, --minify	38 B	9,930 B
var DEV = isDev(); if (DEV) console.log(require("./devonly.ts")), --minify-syntax	8 B	1,576 B
the same with const DEV	8 B	8 B
let home = homePath(); await import(home), --minify	2 modules bundled	1 module bundled, exit 0, Cannot find module at run time from another directory
the same with const home	2 modules	2 modules
let h = hello(); id(h): exit at bun run / bun build / --minify-syntax	0 / 1 / 0	1 / 1 / 1
the same with const h	0 / 1 / 0	0 / 1 / 0

--reject-unresolved makes the import(home) case a build error. A const keeps each fold.

Cost for a file with no macro.

• handle_identifier (2 instantiations): the disassembly is equal on both builds. 1,506 B and 2,188 B on both.
• s_local plus visit_decl, executed instructions under bun build --no-bundle --target=bun --minify-syntax. Counted with qemu-user 10.0.13 (-one-insn-per-tb -d exec,nochain):

File	declarators	main	this PR	per declarator
react-dom.development.js (1.0 MB)	2,660	647,044	647,095	+0.02
src/js/node/http2.ts	519	121,920	122,086	+0.3
typescript.js (9.1 MB)	20,664	5,073,684	5,087,526	+0.7

• The outputs of those three runs are byte-equal on both builds. Bun.Transpiler#transformSync over the 220 files of src/js with default options, inline: true and minify: { syntax: true } gives the same SHA-256 for 660 of 660 outputs.
• Binary text (size of bun-profile): 107,582,646 B to 107,583,926 B. visit_decl 242 B to 396 B, visit_binding_and_expr_for_macro 942 B to 2,114 B, s_local 7,689 B to 7,614 B.
• A destructured macro result allocates one node and one list for each pattern level. Nothing else allocates. There is no new parser field and no transpiler cache version change: a file in which a macro ran is never cached (src/js_parser/parse/parse_entry.rs:2479).
• valgrind, perf and bloaty are not in the container.

Which lines are the fix.

• ConstValue::new in src/ast/ast_result.rs: kind == LocalKind::KConst, and EUndefined => !has_default.
• visit_decl: kind == LocalKind::KConst && (is_macro || !is_after_const_local_prefix). A declaration with a macro call stays exempt from the const-local prefix, as on main. So let i = macro(); const n = 1 still inlines n.
• visit_binding_and_expr_for_macro: it builds kept and returns a new E::Object or E::Array. visit_decl stores the result in decl.value. With a computed key (narrow is false) it returns the object as it is, and still admits the other leaves.
• The rest is the type: can_be_const_value moved into the constructor, and three const_values.put calls became one.

What stays as on main.

• A const that holds a macro object or array is copied at each read ([macro] minify breaks object equality #7196). ConstValue::new keeps the two was_originally_macro arms. js_parser: keep a const macro object or array as one value, and fold macro().prop only for a read #46924 removes them, adds a guard for macro().prop = v, and rewrites the example in docs/bundler/macros.mdx. It needs a maintainer's decision, because 660eb46 added the fold on purpose and the docs teach it.
• console.log(getV()); const v = num(); function getV() { return v } prints 21 where the plain import throws a ReferenceError.
• A macro argument can name a const only under bun run and --minify-syntax (Macros cannot use constant under certain circumstances #16969, open PR js_parser: resolve a literal or macro-result const in a macro argument at any position, in every build mode #44620).

Tests. 16 written modules have a stored value. 208 more come from cross products of the declaration kind, the value and the use, and the plain import is their oracle. Each module runs at bun run, bun build, bun build --no-bundle and bun build --minify, one door at a time and with no time limit. A debug build runs each sixteenth generated module. The build rows pass --entry-naming, because 9 or more entry points change the output layout (#38732). Four mutations of the fix were built and each fails a test: no exemption for a macro const below the prefix, no number and string arm in the constructor, a macro let that ends the prefix, and no whole value for a key that the pattern binds again.

Self-review. 21 concerns.

• Addressed here: the split from the [macro] minify breaks object equality #7196 change, one bullet for each mechanism with its mode, the prune that wrote through the macro's node, the comment of ConstValue, the let that ended the const-local prefix (let counter = num(); const name = "world"; greet(name) was a build error in an earlier head), the losses of let and var listed and pinned, tests that pin an admission, the generated table, rows for each recursive call that forwards the kind, a --no-bundle door, and the comment for --entry-naming.
• Addressed in part: the position test stays in visit_decl and is not a parameter of the writer, because visit_decl also writes the prefix flag. Bun.Transpiler({ inline: true }) has no door of its own. This PR adds no docs sentence for import() of a let.
• Moved to js_parser: keep a const macro object or array as one value, and fold macro().prop only for a read #46924: the numbers for the lost object fold, the docs scope, the properties that ship with a bound object, the delete and compound-operator rows of the write guard, the JSX-spread test, the define table and an opt-back flag.

Review after the first push. 3 findings.

• A debug build panicked on new Bun.Transpiler({ exports: { replace: { a: ["N", true] } } }) for export const a: admit_const_value asserted a constant symbol, and the injected binding has another kind. The assertion is removed.
• A pattern with a computed key admitted no leaf. The leaves with a flat key are admitted again, as on main.
• import() of a let is not bundled. Not changed: the program now builds like the same program with a plain import, and a const keeps the fold.

Suites run on a debug build. macro-test.test.ts (41 pass), transpiler.test.js (258), runtime-transpiler.test.ts (15), scope-mismatch-panic.test.ts (21), bundler_minify (67), esbuild/dce (87), esbuild/ts (59), bundler_barrel (70), bundler_jsx (65), bundler_string (59), plugins.test.ts (111), isolation.test.ts (50), coverage.test.ts (43), transpiler-cache.test.ts (24), build-error.test.ts (9), and the regression tests 03830, 22656, 26360 and 39900.

Related. #26362 reports the lost write under bun run and was closed as a duplicate of #7196. #26363 and #34089 were earlier attempts that the stale sweep closed. Open PR #44620 gives macro arguments a table of their own and edits the same lines: it rebases on this and routes its inliner write through admit_const_value. Open PR #45581 edits the hit arm of handle_identifier. Open PR #45658 keeps assignment and delete targets as references at 12 other fold sites.

The earlier description of this PR. It removed the object and array arms too, with a guard at each write site.

Repro

// macro.ts
export const obj = () => ({});
export const hello = () => "Hello";

import { obj, hello } from "./macro" with { type: "macro" };
const a = obj();
console.log(a === a);        // #7196
let s = hello();
s += " World";               // #12067

$ bun build entry.ts --target bun --minify
console.log({}==={});        // false
"Hello" += " World";         // SyntaxError

Cause

visit_decl puts the macro result into const_values when could_be_macro is set, and handle_identifier then substitutes the stored expression at every use of the binding. Three checks that gate ordinary const-inlining were missing on that path:

• can_be_const_value() returned true for EObject/EArray whenever was_originally_macro was set. Object/array literals allocate a fresh value on each evaluation, so duplicating one at every use turns a === a into ({}) === ({}) ([macro] minify breaks object equality #7196). The BIdentifier leaf of visit_binding_and_expr_for_macro had the same hole for a nested object reached through destructuring.
• could_be_macro was computed from the macro-call count alone, without was_const, so a let/var bound to a macro result was still substituted, turning s += x into "Hello" += x (Bug with --minify-syntax and macro #12067).
• The destructuring leaves recorded the raw macro value without looking at the pattern's default_value, so const [x = 5] = macroReturning([undefined]) inlined x as undefined instead of leaving the destructuring to apply the default at runtime.

Fix

• Drop the EArray/EObject arms from can_be_const_value().
• Require was_const when computing could_be_macro in visit_decls.
• Factor the leaf insert into maybe_record_macro_const_value, which also skips when the pattern has a default and the value is EUndefined. Thread has_default through the destructuring recursion.

Primitive leaves from const { x } = macro() and direct macro().prop still fold; only whole-object substitution, let/var bindings, and undefined-with-default leaves are kept as bindings.

Verification

test/bundler/transpiler/macro-identity.test.ts covers bun run, bun build, and bun build --minify for:

• direct object/array bindings (o === o, a === a)
• nested object/array reached through destructuring (inner, list)
• destructured leaves with defaults over undefined and over a defined value
• a let string binding that is reassigned
• a let destructured binding that is reassigned

and asserts primitive-leaf / direct-property folding are kept. Five of the six cases fail on the released bun and all pass with this change. macro-test.test.ts and bundler_minify.test.ts remain green.

#9613 (destructuring with reordered property names) has a different root cause (the in-place property compaction overwrites entries before later lookups) and is not addressed here.

---

[human-review] gate passed · iteration 0 · 7 files touched

fails on main (without fix)

ASAN without fix: 7 FAILED
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/bundler/transpiler/macro-test.test.ts
bun test v1.4.4 (367d939d9)

test/bundler/transpiler/macro-test.test.ts:
[macro] call escapeHTML
[macro] call identity
[macro] call identity
[macro] call identity
[macro] call identity
[macro] call identity
[macro] call identity
[macro] call identity
[macro] call identity
[macro] call identity
[macro] call symbolKeys
[macro] call identity
[macro] call identity
[macro] call identity
[macro] call identity
[macro] call identity
[macro] call identity
[macro] call identity
[macro] call identity
[macro] call identity
[macro] call identity
[macro] call identity
[macro] call identity
[macro] call identity
[macro] call identity
[macro] call identity
[macro] call identity
[macro] call identity
[macro] call identity
[macro] call escape
[macro] call addStrings
[macro] call addStrings
[macro] call addStrings
[macro] call addStrings
[macro] call addStrings
[macro] call addStrings
[macro] call addStrings
[macro] call addStrings
[macro] call addStrings
[macro] call addStrings
[macro] call addStrings
[macro] ca
... (truncated)

release without fix: 13 FAILED
bun test v1.4.3-canary.1 (367d939d9)

test/bundler/transpiler/macro-test.test.ts:
(pass) bun builtins can be used in macros [0.06ms]
(pass) latin1 string [0.05ms]
(pass) ascii string [0.02ms]
(pass) type coercion [1.23ms]
(pass) object with Symbol keys [0.06ms]
(pass) escaping [0.14ms]
(pass) utf16 string
(pass) import aliases [0.02ms]
(pass) default import
(pass) namespace import [0.01ms]
(pass) ireturnapromise [0.09ms]
(pass) the `assert { type: 'macro' }` spelling works for every form of import [13.86ms]
(pass) object argument with a sparse numeric key [10.35ms]
(pass) object destructuring of a macro result keeps every bound property regardless of key order or repeated keys [7.48ms]
243 |     stderr: "pipe",
244 |   });
245 |   const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
246 |   expect(stderr).toBe("");
247 |   // Debug builds print "[macro] call <name>" to stdout before the script's own output.
248 |   expect(stdout.trim().split("\n").pop()).toBe(
                                                ^
error: expect(received).toBe(expected)

Expected: "[{"a":1,"b":[2]},{"a":1,"b":[2],"children":"kid"},{"
... (truncated)

passes on PR (with fix)

ASAN with fix: all passed
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/bundler/transpiler/macro-test.test.ts
bun test v1.4.4 (367d939d9)

test/bundler/transpiler/macro-test.test.ts:
[macro] call escapeHTML
[macro] call identity
[macro] call identity
[macro] call identity
[macro] call identity
[macro] call identity
[macro] call identity
[macro] call identity
[macro] call identity
[macro] call identity
[macro] call symbolKeys
[macro] call identity
[macro] call identity
[macro] call identity
[macro] call identity
[macro] call identity
[macro] call identity
[macro] call identity
[macro] call identity
[macro] call identity
[macro] call identity
[macro] call identity
[macro] call identity
[macro] call identity
[macro] call identity
[macro] call identity
[macro] call identity
[macro] call identity
[macro] call identity
[macro] call escape
[macro] call addStrings
[macro] call addStrings
[macro] call addStrings
[macro] call addStrings
[macro] call addStrings
[macro] call addStrings
[macro] call addStrings
[macro] call addStrings
[macro] call addStrings
[macro] call addStrings
[macro] call addStrings
[macro] ca
... (truncated)

release with fix: all passed
$ bun scripts/build.ts --profile=release
[configured] bun-profile → bun (stripped)
  target       linux-x64-gnu
  build type   Release
  build dir    ./build/release
  revision     22b20d8f6b
  features     lto, baseline

23 deps, 394 codegen, 1302 objects in 2743ms

ninja: Entering directory `/workspace/bun/build/release'
[1/4] fetch lolhtml
[lolhtml] up to date
[2/4] fetch rust-argon2
[rust-argon2] up to date
[2/4] cargo plan → /workspace/bun/build/release/rust-target/plan.json
282 units: 194 lib, 16 proc-macro (host), 27 custom-build (host), 23 run custom-build, 17 lib (host), 4 run custom-build (host), 1 rlib
[3/4] reconfigure
[1/1671] mkdir stamps
[2/1671] mkdir codegen
[3/1671] gen bake.{client,server,error}.js
-> bake.client.js, bake.server.js, bake.error.js
[4/1671] install /workspace/bun
bun install v1.4.3-canary.1 (367d939d9)

Checked 22 installs across 44 packages (no changes) [871.00ms]
[5/1671] install /workspace/bun/packages/bun-error
bun install v1.4.3-canary.1 (367d939d9)

Checked 1 install across 2 packages (no changes) [37.00ms]
[6/1671] install /workspace/bun/src/node-fallbacks
bun install v1.4.3-canary.1 (367d939d9)

Checked 111 installs acro
... (truncated)

diff hotspot

src/ast/ast_result.rs                      |  37 +++-
 src/ast/expr.rs                            |  22 --
 src/ast/lib.rs                             |   4 +-
 src/js_parser/p.rs                         |   2 +-
 src/js_parser/parser.rs                    |   2 +-
 src/js_parser/visit/mod.rs                 | 200 +++++++++++-------
 test/bundler/transpiler/macro-test.test.ts | 326 ++++++++++++++++++++++++++++-
 7 files changed, 482 insertions(+), 111 deletions(-)

gate history · 1 passed · 0 rejected · iteration 0

evidence per changed file

file                                        reads  edits  tests
src/ast/ast_result.rs                           0      0      6
src/ast/expr.rs                                 0      0      5
src/ast/lib.rs                                  0      0      5
src/js_parser/p.rs                              1      0      5
src/js_parser/parser.rs                         0      0      5
src/js_parser/visit/mod.rs                      2      2      6
test/bundler/transpiler/macro-test.test.ts      0      0      5

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: oven-sh/bun
• Branch: farm/7594ecaa/macro-object-identity
• File: .coderabbit.yaml
• Commit: 661da07
• Layers: Repository YAML
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: a3f06d51-e9e9-44f1-b258-1fa0b5ef9c42

Commits

Reviewing files that changed from the base of the PR and between 179664e and 661da07.

Files selected for processing: 7

• src/ast/ast_result.rs
• src/ast/expr.rs
• src/ast/lib.rs
• src/js_parser/p.rs
• src/js_parser/parser.rs
• src/js_parser/visit/mod.rs
• test/bundler/transpiler/macro-test.test.ts

Files with no reviewable changes: 1

• src/ast/expr.rs

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review.

Walkthrough

The parser now records eligible const values through a ConstValue wrapper. Macro-produced objects and arrays are recursively processed during destructuring. Added tests cover declaration behavior, inlining, macro arguments, and destructuring.

Changes

Const macro values

Layer / File(s)	Summary
Const-value contract and declaration admission src/ast/ast_result.rs, src/ast/expr.rs, src/ast/lib.rs, src/js_parser/parser.rs, src/js_parser/visit/mod.rs	ConstValuesMap now stores ConstValue. Declaration visiting passes the local kind and admits values through ConstValue::new, which checks the binding kind, default status, and supported expression types.
Macro destructuring and replacement access src/js_parser/visit/mod.rs, src/js_parser/p.rs	Macro-produced objects and arrays are recursively processed for destructuring. Eligible identifier leaves can record const values, and identifier replacement reads the expression through expr().
Macro binding and inlining tests test/bundler/transpiler/macro-test.test.ts	Tests compare macro-bound modules with plain imports and cover declaration kinds, reads, writes, destructuring, inlining, and macro arguments. Debug and ASAN builds sample generated cases every 16th case.

Suggested reviewers: jarred-sumner, alii

Priority: Normal

Severity of issue fixed: Medium

Merge Risk: Minimal · up to 661da

The change limits macro-result inlining to const bindings and rebuilds destructured results without mutating the macro's value. No concrete merge-blocking risk remains after review.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title clearly summarizes the main changes: restricting macro-result admission to const bindings and safely narrowing destructured results.
Linked Issues check		[#12067] requires a macro result assigned to let to remain a valid assignment reference. visit_decls now passes LocalKind to declaration handling, and ConstValue::new admits values only for `K…
Out of Scope Changes check		The changes stay within the macro inlining and destructuring path used by [#12067]. The new ConstValue admission check, non-mutating destructuring prune, and related tests support safe handling of m…
Description check		The pull request description clearly explains the problem, implementation, scope, trade-offs, and verification. It does not use the template headings exactly, but it provides equivalent and substantia…

Full details: Linked Issues check

Explanation

[#12067] requires a macro result assigned to let to remain a valid assignment reference. visit_decls now passes LocalKind to declaration handling, and ConstValue::new admits values only for KConst. Macro results for let and var therefore remain runtime bindings. The reported regression tests cover reassignment and pass with the change.

Full details: Out of Scope Changes check

Explanation

The changes stay within the macro inlining and destructuring path used by [#12067]. The new ConstValue admission check, non-mutating destructuring prune, and related tests support safe handling of macro results. The separate behavior that copies macro-returned objects and arrays at each const read remains unchanged and is explicitly excluded.

Full details: Description check

Explanation

The pull request description clearly explains the problem, implementation, scope, trade-offs, and verification. It does not use the template headings exactly, but it provides equivalent and substantially more detailed information for both required sections.

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[github-actions]

Found 2 issues this PR may fix:

1. Bug with --minify-syntax and macro #12067 - Same const-inlining bug: macro string return value is duplicated at every use site, causing "Hello" += " " instead of proper variable assignment
2. Destructuring assignment from macro function fails #9613 - Destructuring from macro-returned objects produces wrong bindings when property order differs, caused by the object literal being const-inlined instead of assigned to a variable first

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

Fixes #12067
Fixes #9613

🤖 Generated with Claude Code

[robobun]

^{Updated 2:11 PM PT - Oct 25th, 2026}

@robobun, your commit 22b20d8 is building: #132782

[claude]

The follow-up commit addresses the default_value gap I flagged — maybe_record_macro_const_value now skips EUndefined when the pattern has a default, and has_default is threaded through both the object and array recursion. All three changes are strictly conservative (they only remove entries from const_values, never add), so the worst case is a missed inline rather than a new miscompile. Deferring to a maintainer to confirm the optimization tradeoff of dropping the EArray/EObject arms from the shared can_be_const_value() predicate is acceptable.

What was reviewed:

• can_be_const_value() has only two callers (visit_decl BIdentifier and the new helper); can_be_moved() retains its was_originally_macro arms so single-use substitution is unaffected.
• was_const gate on could_be_macro now makes let s = macro() set is_after_const_local_prefix like any other let — consistent, not a regression.
• has_default + EUndefined check: traced null (does not trigger default → correctly still inlined) and short-array / missing-property cases (loop doesn't reach them → correctly left as bindings).
• #9613 (in-place property compaction) confirmed pre-existing and orthogonal; scoped out per author's reply.

Extended reasoning...

Overview

Three files: src/ast/expr.rs drops the EArray/EObject arms from can_be_const_value(); src/js_parser/visit/mod.rs gates could_be_macro on was_const, threads a has_default flag through visit_binding_and_expr_for_macro, and factors the leaf insert into maybe_record_macro_const_value which additionally skips when has_default && EUndefined; a new test file exercises bun run, bun build, and bun build --minify across direct object/array bindings, nested destructuring, defaults over defined and undefined slots, and let reassignment.

Delta since my last review

My prior review flagged two pre-existing issues. The author pushed 3dad298 which fully addresses the default_value one (new helper + has_default param + holes/holesArr test cases) and explicitly scoped out the property-compaction reorder bug as #9613 — a separate defect in the lookup-after-overwrite logic, not in the "should this go into const_values" decision this PR tightens. Both threads are resolved.

Security risks

None. Build-time transpiler const-folding; no untrusted-input parsing surface changes, no allocation logic touched.

Level of scrutiny

Moderate-high — the visit pass is a hot, load-bearing code path and can_be_const_value() is a shared predicate. That said, every change here is a strict tightening: fewer expressions qualify for const_values, and could_be_macro requires more to be true. There is no path by which this can introduce a new wrong-value substitution; the only possible downside is that const o = macro(); o.x no longer folds through the identifier (direct macro().x and destructured leaves still do, per the tests). A maintainer should sign off on that tradeoff.

Other factors

• Callers of can_be_const_value(): only two, both in this diff's blast radius. can_be_moved() (single-use-var substitution) still returns true for macro objects/arrays at expr.rs:2406-2407, so the separate move optimization is unchanged.
• The is_after_const_local_prefix side effect of the was_const gate now fires for let x = macro() where it previously didn't — this matches ordinary let behavior and is more consistent.
• Test file is new rather than appended to macro-test.test.ts; the PR body states that file currently aborts the debug build on an unrelated assertion, which is a reasonable justification.
• Test structure follows harness conventions: tempDir, bunEnv/bunExe, concurrent draining of stdout/stderr/exited, test.concurrent, combined-object assertions.

[claude]

No issues found, but this is transpiler-correctness logic (macro const-inlining) rather than a mechanical change, so it's worth a maintainer glance before merge. The robobun status is also still red on 3dad298a (build-bun step) — the two later commits look like they'd fix whatever broke there, but CI hasn't reported on HEAD yet.

What was reviewed:

• can_be_const_value() callers — only the two sites in visit/mod.rs; can_be_moved() still keeps the was_originally_macro arms, so tree-shaking/movability of macro objects is unchanged.
• is_after_const_local_prefix — let s = macro() now falls into the else and sets it (previously didn't); this matches the non-macro let path and is strictly more correct.
• The has_default && EUndefined gate correctly leaves null alone (JS defaults trigger only on undefined); nested EObject/EArray leaves reached via BIdentifier now bail on can_be_const_value() so inner === inner holds.

Extended reasoning...

Overview

The PR removes the EArray/EObject arms from Data::can_be_const_value() and threads two new gates through the macro-destructuring path in visit/mod.rs: was_const (the declaration keyword) and has_default (the binding pattern has a default initializer). A new maybe_record_macro_const_value helper centralizes the decision to insert into const_values. visit_decl gains a was_const parameter and now requires was_const && could_be_macro for the macro branch. A new test file exercises bun run, bun build, and bun build --minify across identity, destructured leaves with defaults, and let reassignment.

Security risks

None. This is AST-shape logic in the transpiler; no I/O, no untrusted input parsing beyond what the parser already handles.

Level of scrutiny

Medium-high. Const-inlining decides whether an expression is duplicated at every use site, so a wrong true here silently miscompiles user code (as #7196 and #12067 demonstrate). The fix is a strict tightening — every path that previously inserted into const_values now has an additional gate — so the risk profile is "macro output slightly less inlined" rather than "new miscompile", but a maintainer who owns the macro/inlining path should confirm the is_after_const_local_prefix side-effect change for let x = macro() (now sets the flag; previously didn't) is intended.

Other factors

• The two concerns I raised on the earlier revision are resolved: the default_value-over-undefined case is now handled in maybe_record_macro_const_value and covered by the holes/holesArr tests; the reordered-destructure compaction bug is explicitly scoped out as #9613.
• can_be_const_value() has exactly two call sites, both in this file, and can_be_moved() retains the was_originally_macro arms — so no other consumer loses the old behavior.
• CI (robobun) shows build-bun failing on 4 platforms at 3dad298a; two commits landed after (8b45d92f refactor, 62732cb4 autofix) and the current file reads as valid Rust, but the status hasn't updated for HEAD.
• The test file is placed separately from macro-test.test.ts because that file currently aborts the debug build; that's a reasonable justification but worth a maintainer ack.

[robobun]

Rebased onto main at 1d85431. Verified locally on the rebased commit:

• USE_SYSTEM_BUN=1 bun test test/bundler/transpiler/macro-identity.test.ts: 5 fail (including SyntaxError: Left side of assignment is not a reference from the Bug with --minify-syntax and macro #12067 repro).
• bun bd test test/bundler/transpiler/macro-identity.test.ts: 6 pass.
• macro-test.test.ts and bundler_minify.test.ts still pass.
• cargo clippy -p bun_js_parser -p bun_ast is clean.

The previous red checks were unrelated to the diff: the three darwin test lanes expired without running, and the clippy job failed at vendor/lolhtml/Cargo.toml checkout before linting anything. Ready for review.

[claude]

No issues found — the change strictly narrows what enters const_values, so it can only reduce inlining, not introduce new incorrect substitutions. My earlier note on default_value + EUndefined is now handled by maybe_record_macro_const_value, and #9613 is reasonably left out of scope. Deferring to a human only because this is the transpiler hot path that runs on every file.

What was reviewed

• can_be_const_value() has just two callers, both in visit/mod.rs; dropping the EArray/EObject arms only affects the macro const-inlining paths this PR is fixing.
• Macro deserializer emits only primitives, EObject, and EArray — the _ arm in BObject still folds every primitive it did before after the new can_be_const_value() gate.
• let x = macro() now falls into the else and sets is_after_const_local_prefix = true; that matches ordinary let behavior and is at worst a de-opt for later consts in the same prefix.

Extended reasoning...

Overview

Two-line removal from Data::can_be_const_value() in src/ast/expr.rs (drops the EArray/EObject arms whose "no observable difference if duplicated" contract they violated), plus a focused refactor in src/js_parser/visit/mod.rs: visit_decl gains a was_const parameter, the macro-branch condition becomes was_const && could_be_macro, and the destructuring leaf insert is factored into maybe_record_macro_const_value which also checks has_default && EUndefined. has_default is threaded through the BObject/BArray recursion. A new 200-line test file exercises bun run, bun build, and bun build --minify across direct bindings, nested destructuring, defaults over undefined vs. defined, and let reassignment.

Security risks

None. This is const-folding logic in the AST visitor; no untrusted-input parsing, no allocation sizing, no I/O.

Level of scrutiny

High — js_parser/visit runs on every transpiled file, and can_be_const_value() is a shared predicate. I verified it has exactly two call sites (both in visit/mod.rs, both the paths under repair), and the macro deserializer's output set (ENull/EUndefined/EBoolean/ENumber/EString/EObject/EArray) means the new gate on the BObject _ arm rejects nothing that was previously (correctly) admitted. Every edit narrows the set of refs entering const_values, so the failure mode is a missed optimization, not a new miscompile.

Other factors

My earlier inline comment about default_value being ignored was addressed in a follow-up commit (now the has_default/EUndefined guard, covered by the holes/holesArr cases). The other pre-existing note (#9613, in-place property compaction) is a distinct algorithm bug in the same function and the author's decision to keep it out of scope is sound. Tests follow harness conventions (tempDir, bunEnv, concurrent draining of stdout/stderr/exited, test.concurrent). CI on the completed lanes is green; remaining red is build-infra unrelated to the parser. Deferring solely because a human should sign off on any change to the transpiler's const-inlining decision path.

[claude]

LGTM — the follow-up commit addressed the default_value/EUndefined gap I flagged, and #9613 is reasonably scoped out.

What was reviewed:

• can_be_const_value() has only the two call sites in visit/mod.rs; dropping the EArray/EObject arms only affects was_originally_macro values, so non-macro const-inlining is unchanged.
• Checked that not-inlining is always safe: the decl-removal loop only skips a decl when its ref is in const_values, so const o = macroObj() now stays emitted; is_after_const_local_prefix for let macro decls now matches non-macro let.
• Traced has_default through nested BObject/BArray recursion including the BIdentifier-binding-with-EObject-value case and the EUndefined-with-nested-pattern case — all fall through to "skip insert".
• Property/item truncation still runs for let destructures (only the const_values insert is gated), which the last test asserts.

Extended reasoning...

Overview

Three-file change fixing #7196 and #12067: src/ast/expr.rs drops the EArray/EObject arms from can_be_const_value(), src/js_parser/visit/mod.rs threads was_const into visit_decl and has_default through visit_binding_and_expr_for_macro, factoring the leaf insert into maybe_record_macro_const_value, and a new 6-case test file covers bun run / bun build / bun build --minify for object/array identity, nested destructuring, destructure defaults over undefined, and let reassignment.

Security risks

None. Transpiler const-inlining correctness only; no untrusted-input parsing changes, no I/O, no allocation-size arithmetic touched.

Level of scrutiny

Medium — parser/visit-pass code is core, but the change is strictly narrowing: it only removes refs from const_values that were previously (incorrectly) inlined. Not-inlining is always the safe fallback because handle_identifier leaves the ref alone when it is not in the map, and the decl-removal loop only drops a decl whose ref is in const_values with zero uses. I confirmed can_be_const_value() has exactly two callers (both in this file) and visit_decl has exactly two callers (both updated), so there is no missed call site. The was_originally_macro guard on the removed arms means non-macro const x = [...] was already excluded, so ordinary const-inlining is untouched.

Other factors

• My earlier inline comment about default_value + EUndefined was addressed in 3dad298 with the has_default threading and covered by the da/dx assertions in the new test. The other inline comment (in-place property compaction, #9613) is a separate lookup-after-overwrite defect that predates this PR; the author scoped it out with a stated reason and the PR description calls it out explicitly.
• Test quality is good per REVIEW.md: test.concurrent, tempDir/bunEnv/bunExe, concurrent pipe drain, combined {out, stderr, exitCode} assertion, and a negative-contract check that let { a } = big() still truncates unused properties. The PR body evidence block shows 5 failing on the released bun and all 6 passing on both debug-ASAN and release with the fix.
• No CODEOWNERS on src/js_parser/ or src/ast/.

[robobun]

Reworked on main 179664ea89 and force-pushed. The conflict in src/js_parser/visit/mod.rs is gone.

How it was reproduced. m.ts exports hello = () => "Hello". The entry imports it with { type: "macro" } and runs let s = hello(); s += " World"; console.log(s);.

• bun entry.ts on 1.4.3 and on a release build of main: SyntaxError: Left side of assignment is not a reference.
• This branch: Hello World under bun run, bun build and bun build --minify.
• The 7 new tests in test/bundler/transpiler/macro-test.test.ts fail on a release build of main and pass on this branch (41 of 41 in the file).

Scope. This PR no longer removes the fold for a const that holds a macro object or array. 660eb46 added that fold on purpose and docs/bundler/macros.mdx teaches it, so its removal needs a maintainer's decision. It is #46924. This PR keeps the parts that need no decision: let and var, pattern defaults, and the destructuring prune.

[claude]

Findings marked 🟡 are optional suggestions and need no follow-up push.

[claude]

Still open from earlier reviews (3):

• 🔴 src/ast/ast_result.rs:274 — Users who bind a macro-returned path with let or var and pass it to import() get a --minify build that exits 0 but thro…
• 🔴 src/js_parser/visit/mod.rs:762 — Debug and ASAN builds panic when Bun.Transpiler injects a replacement export into a const declaration, where the base b…
• Also unresolved: 1 minor or pre-existing.

If you have decided not to act on one of these findings, resolve its thread (a reply alone leaves it open) and the next review stops counting it. To review this commit again now, use Re-run on its "Claude Code Review" check.

[claude]

Findings marked 🟡 are optional suggestions and need no follow-up push.

[claude]

I re-reviewed the latest push and found no bugs; the two nits from the previous round (per-test timeouts, and twice-bound keys only tested with the nested pattern first) are addressed in 22b20d8. A maintainer should still weigh the intentional trade-off this PR makes: let/var macro results are no longer inlined, which the description documents as costing import(home) bundling and the var DEV = isDev(); if (DEV) require(...) dead-code fold.

What was reviewed:

• visit_decl prefix logic old vs new: the four (kind, is_macro, is_after) cases produce the same is_after_const_local_prefix writes; only let/var macro admission is dropped.
• visit_binding_and_expr_for_macro rewrite: no in-place mutation of the macro node remains; !narrow still admits flat-key leaves; spread returns early as before; array holes and defaults route through has_default.
• ConstValue is the only type in ConstValuesMap; the one reader in p.rs and the using skip at mod.rs:1899 were checked against the new constructor (a using ref can no longer be admitted, and the continue there is still live).

Extended reasoning...

The change touches the JS parser's const-inlining table (src/ast/ast_result.rs, src/js_parser/visit/mod.rs, p.rs, parser.rs) and adds a ~320-line cross-product test in test/bundler/transpiler/macro-test.test.ts; it touches no auth, crypto, or injection surface. The latest commit addressed both of this reviewer's prior nits and no new defects were found in the diff. Approval is withheld because the PR deliberately changes user-visible output for let/var macro bindings (documented downsides: unresolved import() of a let path under --minify, and a dev-only require no longer folded away), and the earlier inline objection on that point was resolved only by the author, so a maintainer decision on that trade-off is the remaining open item.