Stop rewriting a literal import("bun") to Promise.resolve(globalThis.Bun)

[robobun]

Problem

• await import("bun") returns two different things depending on how the specifier is spelled. A literal gives the Bun object itself ([object Bun], ns.default === Bun is false); import(s) with a let, or a const declared after any other statement, gives a module namespace ([object Module], ns.default === Bun).
• Reproduced on bun 1.4.0 and main; bun build --target=bun --minify-syntax shows the same split in its output.
• Cause: import("bun") has two implementations. The printer rewrites a literal to Promise.resolve(globalThis.Bun) (from 2022, before a real "bun" module existed); anything the printer cannot see as a literal goes to the loader's "bun" module (make dynamically importing bun work #4055). Which one a file hits is decided by const inlining, an optimisation that is supposed to be unobservable.
• Because the rewrite skipped the loader, mock.module("bun", ...) applied to import(s) but not to import("bun"), and import options were silently dropped.

Fix

• The printer stops rewriting the dynamic-import form: a literal import("bun") is printed as an ordinary external import and the loader resolves it, in the runtime transpiler and in bun build --target=bun. require("bun") and static imports are still inlined to globalThis.Bun, which is also what the loader returns for a require of "bun" (their globalThis shadowing bug is subtle bundler bug, require("bun") in a scope that defines local variable named globalThis will bundle incorrectly #8058 / js_printer: fix require("bun") and other printer literals being captured by same-named locals #35739, untouched here).
• Why the namespace is the right single answer: the loader path exists regardless (computed specifiers, re-exports, mocks), and import() of a module record can only ever produce a namespace, so the literal has to move to it. "bun" then behaves like import("bun:sqlite") or import("node:fs").
• Observable change: (await import("bun")) === Bun is now false for the literal (it already was for every other spelling); .default, named exports and destructuring all work. The literal is now a real module load, cheap only because Declare the "bun" module's ESM exports lazily instead of reifying the whole Bun object #37714 made the module's exports lazy, so this PR is stacked on it. The runtime transpiler cache version is bumped so cached output holding the old rewrite is not reused after an upgrade.
• Verification: new tests in the runtime, bundler and mock.module suites fail without the printer change and pass with it, including four one-file programs (literal, inlined const, non-inlined const, let) that must print identical output. Import options, .cjs callers, --format=cjs, --minify, --compile and the Bake dev server path were checked by hand.

Background

• The "bun" module: since make dynamically importing bun work #4055 the module loader has a native "bun" module, a namespace whose default is the Bun global with one named export per Bun.* property. export * from "bun", mock.module("bun", ...) and every computed import() already go through it.
• Import record tags: an import record whose specifier is the literal "bun" carries a Bun tag, and the printer used that tag to emit globalThis.Bun in place of the import for all three kinds (require, static import, dynamic import). After this PR only the first two are rewritten.
• Const inlining: the transpiler (and --minify-syntax in the bundler) substitutes a const string into import(x) only when the const is in the leading run of const declarations of its scope, the TDZ-safe rule. That is why one extra statement before the const flips which path a file takes.
• Runtime transpiler cache: bun stores transpiled output of source files on disk under a version constant; bumping the constant discards entries whose printed text would now differ.

---

[review] gate passed · iteration 0 · 6 files touched

fails on main (without fix)

ASAN without fix: 7 failed, 1 skipped
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/bundler/bundler_bun.test.ts test/js/bun/resolve/builtin-esm-lazy-exports.test.ts test/js/bun/resolve/import-meta.test.js test/js/bun/test/mock/mock-module.test.ts
bun test v1.4.0 (132085efd)

test/bundler/bundler_bun.test.ts:
16 |         console.log(require("bun") === Bun);
17 |       `,
18 |     },
19 |     run: { stdout: "[object Module] true true\ntrue" },
20 |     onAfterBundle(api) {
21 |       api.expectFile("out.js").toContain('import("bun")');
                                    ^
error: expect(received).toContain(expected)

Expected to contain: "import(\"bun\")"
Received: "// @bun\nvar __require = import.meta.require;\n\n// entry.ts\nvar ns = await Promise.resolve(globalThis.Bun);\nconsole.log(Object.prototype.toString.call(ns), ns.default === Bun, ns.serve === Bun.serve);\nconsole.log(globalThis.Bun === Bun);\n"

      at onAfterBundle (/workspace/bun/test/bundler/bundler_bun.test.ts:21:32)
      at <anonymous> (/workspace/bun/test/bundler/expectBundled.ts:1591:7)
(fail) bundler > bun/dynamic-import-bun-is-module-namesp
... (truncated)

release without fix: 19 failed, 1 skipped
bun test v1.4.0-canary.1 (9008ae7ab)

test/bundler/bundler_bun.test.ts:
16 |         console.log(require("bun") === Bun);
17 |       `,
18 |     },
19 |     run: { stdout: "[object Module] true true\ntrue" },
20 |     onAfterBundle(api) {
21 |       api.expectFile("out.js").toContain('import("bun")');
                                    ^
error: expect(received).toContain(expected)

Expected to contain: "import(\"bun\")"
Received: "// @bun\nvar __require = import.meta.require;\n\n// entry.ts\nvar ns = await Promise.resolve(globalThis.Bun);\nconsole.log(Object.prototype.toString.call(ns), ns.default === Bun, ns.serve === Bun.serve);\nconsole.log(globalThis.Bun === Bun);\n"

      at onAfterBundle (/workspace/bun/test/bundler/bundler_bun.test.ts:21:32)
      at <anonymous> (/workspace/bun/test/bundler/expectBundled.ts:1591:7)
(fail) bundler > bun/dynamic-import-bun-is-module-namespace [15.32ms]
33 |         console.log(Object.prototype.toString.call(ns), ns.default === Bun, ns.serve === Bun.serve);
34 |       `,
35 |     },
36 |     run: { stdout: "[object Module] true true" },
37 |     onAfterBundle(api) {
38 |       api.expectFile("out.js").toContain('import("bun")'
... (truncated)

passes on PR (with fix)

ASAN with fix: 1 skipped
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/bundler/bundler_bun.test.ts test/js/bun/resolve/builtin-esm-lazy-exports.test.ts test/js/bun/resolve/import-meta.test.js test/js/bun/test/mock/mock-module.test.ts
bun test v1.4.0 (132085efd)

test/bundler/bundler_bun.test.ts:
(pass) bundler > bun/dynamic-import-bun-is-module-namespace [924.92ms]
(pass) bundler > bun/dynamic-import-bun-inlined-const [493.41ms]
(pass) bundler > bun/import-bun-format-cjs [548.72ms]
(pass) bundler > bun/embedded-sqlite-file [525.81ms]
(pass) bundler > bun/sqlite-file [544.54ms]
(pass) bundler > bun/TargetBunNoSourcemapMessage [668.32ms]
(pass) bundler > bun/TargetBunSourcemapInline [665.68ms]
(pass) bundler > bun/unicode comment [447.09ms]
(pass) bundler > bun/ExportsConditionsDevelopmentAPI [492.38ms]
(pass) bundler > bun/ExportsConditionsDevelopmentInProductionAPI [499.53ms]
(pass) bundler > bun/ExportsConditionsDevelopmentCLI [1091.81ms]
(pass) bundler > bun/ExportsConditionsDevelopmentInProductionCLI [586.11ms]

test/js/bun/resolve/import-meta.test.js:
(pass) import.meta.require is settable [4.87m
... (truncated)

release with fix: 1 skipped
$ bun scripts/build.ts --profile=release
[configured] bun-profile → bun (stripped) in 771ms (unchanged)
ninja: Entering directory `/workspace/bun/build/release'
[1/5] gen generated_host_exports.rs
generated_host_exports.rs: 93 exports (host=3, lazy=10, generic=80, rust=0); 239 extern-C blocks audited
[1/5] cargo bun_bin → libbun_rust.a (--target x86_64-unknown-linux-gnu)

  nightly-2026-07-20-x86_64-unknown-linux-gnu unchanged - rustc 1.99.0-nightly (9f36de775 2026-07-19)

^[[1m^[[92m   Compiling^[[0m bun_js_printer v0.0.0 (/workspace/bun/src/js_printer)
^[[1m^[[92m   Compiling^[[0m bun_bundler v0.0.0 (/workspace/bun/src/bundler)
^[[1m^[[92m   Compiling^[[0m bun_standalone_graph v0.0.0 (/workspace/bun/src/standalone_graph)
^[[1m^[[92m   Compiling^[[0m bun_transpiler v0.0.0 (/workspace/bun/src/transpiler)
^[[1m^[[92m   Compiling^[[0m bun_bunfig v0.0.0 (/workspace/bun/src/bunfig)
^[[1m^[[92m   Compiling^[[0m bun_install v0.0.0 (/workspace/bun/src/install)
^[[1m^[[92m   Compiling^[[0m bun_jsc v0.0.0 (/workspace/bun/src/jsc)
^[[1m^[[92m   Compiling^[[0m bun_js_parser_jsc v0.0.0 (/workspace/bun/src/js_parser_jsc)
^[[1m^[[92m   Compiling^[[0m bun_ast_jsc v0.0.0 (/workspace/bun/src/ast_jsc)
^[[1m
... (truncated)

diff hotspot

src/js_printer/lib.rs                              | 34 +++++------------
 src/jsc/RuntimeTranspilerCache.rs                  |  4 +-
 test/bundler/bundler_bun.test.ts                   | 35 ++++++++++++++++++
 .../bun/resolve/builtin-esm-lazy-exports.test.ts   | 13 +++----
 test/js/bun/resolve/import-meta.test.js            | 43 ++++++++++++++++++++--
 test/js/bun/test/mock/mock-module.test.ts          | 31 ++++++++++++++++
 6 files changed, 123 insertions(+), 37 deletions(-)

gate history · 1 passed · 0 rejected · iteration 0

evidence per changed file

file                                                  reads  edits  tests
src/js_printer/lib.rs                                     6      4      0
src/jsc/RuntimeTranspilerCache.rs                         3      3      0
test/bundler/bundler_bun.test.ts                          1      1      0
test/js/bun/resolve/builtin-esm-lazy-exports.test.ts      4      5      0
test/js/bun/resolve/import-meta.test.js                   2      2      0
test/js/bun/test/mock/mock-module.test.ts                 2      4      0

Original description

What

import() of "bun" returned two different kinds of object depending on how the specifier reached the transpiler:

// a.mjs
const s = "bun"; const ns = await import(s);
console.log(Object.prototype.toString.call(ns), ns.default === Bun);   // [object Bun] false

// b.mjs: identical, plus any statement before the const (empty.mjs is an empty file)
import "./empty.mjs";
const s = "bun"; const ns = await import(s);
console.log(Object.prototype.toString.call(ns), ns.default === Bun);   // [object Module] true

let s = "bun" behaves like b.mjs; a literal import("bun") behaves like a.mjs. bun build --target=bun --minify-syntax shows the same split in its output (Promise.resolve(globalThis.Bun) for a.mjs, import(s) for b.mjs). Reproduced on bun 1.4.0 and on main.

Cause

Two implementations of import("bun"):

• The printer (print_require_or_import_expr, ImportRecordTag::Bun) rewrites a literal import("bun") into Promise.resolve(globalThis.Bun). This dates from a97914f (2022), when there was no "bun" module in the loader at all and the rewrite was what made require("bun") / import("bun") work.
• make dynamically importing bun work #4055 (2023) added the real "bun" module to the loader (generateNativeModule_BunObject): a namespace whose default is Bun, with one named export per Bun.* property. Every import() whose specifier the printer cannot see as a literal goes there, as do export * from "bun" and mock.module("bun", ...).

The runtime transpiler (and --minify-syntax in the bundler) inlines a const string into import(s) only when the const is in the leading run of const declarations of its scope, which is the standard TDZ-safe rule for const inlining and is working as intended. Inlining is supposed to be unobservable; the printer rewrite is what made it observable, because a folded specifier took the first implementation and an unfolded one took the second.

Fix

Remove the dynamic-import half of the rewrite. A literal import("bun") is now printed as a normal external import("bun") and the loader resolves it, in both the runtime transpiler and bun build --target=bun. require("bun") is still inlined to globalThis.Bun: the loader also returns the Bun object for a require() of "bun", so the two paths already agree there (the shadowed globalThis problem with that inlining is #8058 / #35739, which this PR does not touch). The runtime transpiler cache version is bumped because cached output for files containing a literal import("bun") would otherwise keep the old rewrite after an upgrade.

Making the namespace the single answer is the only option that can be consistent: the loader path exists regardless (computed specifiers, re-exports, mocks), and import() of a module record can only ever produce a namespace object, so the literal has to move to it rather than the other way around. It also makes "bun" behave like every other builtin (await import("bun:sqlite") or await import("node:fs") return namespaces too), and the old Promise.resolve(globalThis.Bun) text was skipping the loader entirely, so mock.module("bun", ...) applied to import(s) but not to import("bun"), and import options were silently dropped. Those now behave the same as for any other module.

Observable changes

• await import("bun") is the module namespace: ns.default === Bun, ns.serve === Bun.serve, Object.keys(ns) lists the Bun properties. (await import("bun")) === Bun is now false; it was already false for every non-literal form. Destructuring or property access keeps working, and .default starts working.
• import("bun") is one more module load instead of a resolved promise. This is built on top of Declare the "bun" module's ESM exports lazily instead of reifying the whole Bun object #37714, which makes the "bun" module's exports lazy, so the load constructs nothing on the Bun object up front (before Declare the "bun" module's ESM exports lazily instead of reifying the whole Bun object #37714 the generator reified every Bun.* property, about 5ms on a release build, and that bulk reification also trips BUN_JSC_validateExceptionChecks on the ASAN lane, which is what the first CI run of this PR hit once the literal form started reaching it). Static import ... from "bun" and require("bun") are unchanged.
• Side effect under --minify-whitespace: return import("bun") was printed as returnPromise.resolve(...) (the dynamic-import case of Fix missing space between preceding identifier and rewritten globalThis.Bun under --minify-whitespace #30673); the external import path prints the separating space, so that case is fixed here. The require("bun") case in Fix missing space between preceding identifier and rewritten globalThis.Bun under --minify-whitespace #30673 still stands.
• Bake dev server code: a literal import("bun") now goes through hmr.dynamicImport("bun") like other builtins do, which falls through to the real import(); checked manually with a route doing both the literal and a computed import and getting the same namespace.

#35739 (open) drops the runtime half of both rewrites for the globalThis shadowing bug; it would fix the runtime side of this as a side effect but leaves the bundler split in place. The two overlap textually in the same hunk and are otherwise compatible: if it lands first, this PR reduces to deleting the dynamic-import branch for the bundler too.

Tests

All of these fail without the src/ change and pass with it:

• test/js/bun/resolve/import-meta.test.js: the literal import("bun") test now asserts the namespace shape and that it is the same record as import(eval("'bun'")); a new concurrent group runs four single-file programs (literal, const that gets inlined, const after an import that does not, let) and expects identical output from each.
• test/bundler/bundler_bun.test.ts: bun build --target=bun of a literal import("bun") keeps import("bun") in the output and prints the namespace when run, with require("bun") === Bun checked alongside; the same through --minify-syntax with the specifier in a const.
• test/js/bun/test/mock/mock-module.test.ts: mock.module("bun", ...) in a child bun test applies to the literal import("bun") and the computed one alike.
• test/js/bun/resolve/builtin-esm-lazy-exports.test.ts (from Declare the "bun" module's ESM exports lazily instead of reifying the whole Bun object #37714 / Declare node:process and node:module ESM exports lazily as well #37726): its comments described the literal import("bun") as being rewritten, and its import() test routed the specifier through the helper module to keep it away from the transpiler. The comments are updated and the test now uses the literal directly, so it exercises the path this PR changes (it fails on the old printer with defaultIsBun: false).

Also run on this build: the rest of import-meta.test.js, bundler_bun.test.ts, mock-module.test.ts and builtin-esm-lazy-exports.test.ts, test/js/bun/util/BunObject.test.ts, test/cli/run/syntax.test.ts (includes await import('bun')), test/cli/run/transpiler-cache.test.ts. Manually checked import("bun", {...}), import("bun") from a .cjs file, --format=cjs output, --minify output, and a bun build --compile binary; all return the namespace.

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 9 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 611827f5-3c03-4e79-a90b-f59607dc8673

📥 Commits

Reviewing files that changed from the base of the PR and between 9a543cc and 132085e.

📒 Files selected for processing (6)

• src/js_printer/lib.rs
• src/jsc/RuntimeTranspilerCache.rs
• test/bundler/bundler_bun.test.ts
• test/js/bun/resolve/builtin-esm-lazy-exports.test.ts
• test/js/bun/resolve/import-meta.test.js
• test/js/bun/test/mock/mock-module.test.ts

---

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

[robobun]

Status: reproduced on bun 1.4.0 and on main with two otherwise identical files, const s = "bun"; await import(s) with and without a preceding import statement (one returns Bun itself, the other the "bun" module namespace), and the same split in bun build --target=bun --minify-syntax output.

Fix in this PR: the printer no longer rewrites a literal import("bun") to Promise.resolve(globalThis.Bun), so every form of import() goes through the loader and returns the same namespace. Tests in test/js/bun/resolve/import-meta.test.js, test/bundler/bundler_bun.test.ts, test/js/bun/test/mock/mock-module.test.ts and the "bun" import() case in test/js/bun/resolve/builtin-esm-lazy-exports.test.ts fail without the src/ change and pass with it. The branch depends on #37714 and is rebased onto current main (past #37726, which touched the same test file).

For the maintainer looking at this: the one deliberate behavior change is that a literal await import("bun") now returns the namespace (ns.default === Bun) instead of Bun itself, which is what every other spelling of the import already returned; the argument for that direction is in the description. All review comments have been addressed.

CI for the current head (build 93159): 178 jobs passed, including every Linux lane and the ASAN lane, which is where the files this PR touches are exercised under exception-check validation. The remaining red is unrelated to the diff: test/bake/deinitialization.test.ts segfaults during dev-server teardown on Windows 2019 x64 (its fixture has no dynamic import at all; reported separately), and the two darwin 26 aarch64 test jobs expired waiting for an agent, as they also did in build 92894 while main builds were getting that lane. I am not going to retrigger further for those; ready for a maintainer.

[claude]

I reviewed this PR and didn't find any bugs. Because it changes the observable result of await import("bun") (now a module namespace rather than Bun itself) and overlaps with #35739 in the same printer hunk, a maintainer sign-off on the behavior change would be worthwhile.

Checked that the removed ImportKind::Dynamic branch falls through cleanly to the external-import() path at src/js_printer/lib.rs:2624 (which calls print_space_before_identifier, so the minify-whitespace fix noted in the description holds). Confirmed ImportRecordTag::Bun handling for static imports (print_global_bun_import_statement at :5625) and the bake dev-server hmr.dynamicImport path are unaffected. The transpiler cache version bump is correct given cached output changes shape. Tests follow harness conventions (tempDir, concurrent pipe drain, using) and cover the literal/inlined-const/non-inlined/let matrix plus bundler and mock.module paths.

Extended reasoning...

Overview

The PR removes one branch from print_require_or_import_expr in src/js_printer/lib.rs: the ImportKind::Dynamic arm under ImportRecordTag::Bun that rewrote a literal import("bun") to Promise.resolve(globalThis.Bun). The Require/Stmt arm is kept unchanged. RuntimeTranspilerCache EXPECTED_VERSION is bumped 25→26. Three test files gain coverage: the runtime transpiler path (four specifier spellings in import-meta.test.js), the bundler path with and without --minify-syntax (bundler_bun.test.ts), and mock.module("bun") now applying to literal dynamic imports (mock-module.test.ts).

Security risks

None. No untrusted input handling, no auth/crypto, no memory-safety surface. Pure printer control-flow simplification plus a cache version bump.

Level of scrutiny

Medium-high. The mechanical change is small and clearly correct — the deleted branch falls through to the existing external-import() path used by every other builtin, and I traced that path to confirm print_space_before_identifier runs and import_options are now honored. But the effect is a user-visible semantic change: (await import("bun")) === Bun flips from true to false for the literal form (it was already false for computed specifiers). The PR description makes a strong case that this is a consistency fix, and property/destructuring access keeps working since the namespace mirrors Bun.*, but it is still an observable break that a maintainer should ratify.

Other factors

• The PR notes textual overlap with open PR #35739 in the same hunk; a human should decide sequencing.
• Test quality is high: hermetic (tempDir, bunEnv, child processes), pipes drained concurrently, exit code asserted last, describe.concurrent/it.each for the variant matrix, and the modified import-meta.test.js case would fail on the old build (the PR states USE_SYSTEM_BUN=1 verification was done).
• Grepped for other ImportRecordTag::Bun sites and Promise.resolve(globalThis.Bun) references — no stragglers. The static-import path at js_printer/lib.rs:5625 and the bake dev-server builtin check at convertStmtsForChunkForDevServer.rs:116 are independent of this change.

[robobun]

^{Updated 11:05 AM PT - Aug 12th, 2026}

❌ @robobun, your commit 132085e has 1 failures in Build #93159 (All Failures):

• test/bake/deinitialization.test.ts - pid 2660 segmentation fault at address 0x0 on 🪟 2019 x64

---

🧪   To try this PR locally:

bunx bun-pr 37730

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

bun-37730 --bun

[claude]

Thanks — 79213a9 addressed the stale-comment note (the builtin-esm-lazy-exports.test.ts comments now match the new behavior, and the import() test uses the literal directly). Re-review found no issues. Not auto-approving only because (await import("bun")) === Bun going from true to false is a user-visible default change you already flagged for maintainer sign-off.

What was reviewed:

• Printer change: ImportKind::Dynamic falls through to the normal external-import path; Require/Stmt still inline to globalThis.Bun; wrap handling preserved.
• Transpiler cache version bump (25→26) so cached Promise.resolve(globalThis.Bun) output is invalidated.
• New tests follow harness conventions (tempDir, concurrent pipe drain, child process for the mock.module("bun") case) and cover literal / inlined-const / non-inlined / let plus the bundler output shape.

Extended reasoning...

Overview

The PR removes one branch from print_require_or_import_expr in src/js_printer/lib.rs: ImportRecordTag::Bun with ImportKind::Dynamic no longer emits Promise.resolve(globalThis.Bun), so a literal import("bun") reaches the loader like any other dynamic import and resolves to the "bun" module namespace. Require and Stmt kinds keep the globalThis.Bun inlining. EXPECTED_VERSION in RuntimeTranspilerCache.rs is bumped 25→26. Four test files are updated/extended: import-meta.test.js (namespace shape + four-spelling matrix in subprocesses), bundler_bun.test.ts (output contains import("bun"), runtime prints [object Module]), mock-module.test.ts (mock applies to literal and computed alike, in a child), and builtin-esm-lazy-exports.test.ts (stale comments fixed, specifier indirection dropped so the test now exercises this path).

Security risks

None. This is a printer/transpiler output change for the "bun" builtin specifier; no untrusted-input parsing, no auth/crypto/permissions surface.

Level of scrutiny

Medium-high. The code change is a ~15-line simplification and the mechanism is well-argued in the description, but it changes the observable result of await import("bun") from Bun itself to a module namespace whose default is Bun. That is the correct direction (it makes the literal agree with every non-literal spelling, with mock.module, and with how every other builtin behaves under import()), but it is still a user-facing default change to a Bun-native API — the kind of decision REVIEW.md routes to a maintainer rather than an automated approval. The author explicitly flagged this in the thread as the one thing needing sign-off.

Other factors

My prior inline note about stale comments in builtin-esm-lazy-exports.test.ts was addressed in 79213a9: the comment at lines 43–44 now correctly says only static import ... from "bun" and require("bun") are rewritten, the specifier indirection and its explanatory comment are deleted, and the affected test uses await import("bun") directly (so it now fails on the old printer with defaultIsBun: false, per the updated PR description). The comment-cop bot threads on lib.rs and RuntimeTranspilerCache.rs are resolved — the surviving comments are three and two lines respectively and match neighboring conventions. Test coverage is thorough (variant matrix, bundler output assertion, mock isolation in a subprocess, cache-version bump), and the description records manual checks of .cjs, --format=cjs, --minify, --compile, and Bake dev-server paths. The only open item is the maintainer call on the behavior direction; there is also an open PR #35739 touching the same hunk that whichever lands second will need to reconcile.

[claude]

I re-reviewed after 79213a9 and found no further issues — the stale-comment note is addressed and the import("bun") case in builtin-esm-lazy-exports.test.ts now exercises the changed path directly. Deferring to a maintainer only because this deliberately changes what a literal await import("bun") returns (namespace instead of Bun itself), which is an API-visible decision the author already flagged for sign-off.

What was reviewed:

• Printer change: ImportKind::Dynamic no longer matches the ImportRecordTag::Bun inline; Require/Stmt still do, and the fall-through prints a normal external import("bun").
• Transpiler cache version bump to 26 so cached Promise.resolve(globalThis.Bun) output is invalidated.
• New tests cover runtime (four specifier spellings agree), bundler --target=bun output with and without --minify-syntax, and mock.module("bun") reaching the literal form.

Extended reasoning...

Overview

The PR drops the ImportKind::Dynamic arm of the ImportRecordTag::Bun rewrite in src/js_printer/lib.rs:2416-2428, so a literal import("bun") is printed as a real dynamic import and resolved by the loader to the "bun" module namespace, matching what a computed specifier already returned. require("bun") and static import ... from "bun" remain inlined to globalThis.Bun. EXPECTED_VERSION in RuntimeTranspilerCache.rs is bumped 25→26 so on-disk .pile entries carrying the old rewrite are discarded. Four test files add coverage: import-meta.test.js (namespace shape + four-spelling parity in subprocesses), bundler_bun.test.ts (bun build --target=bun output and run, plus --minify-syntax const-inlining case), mock-module.test.ts (child bun test verifying mock.module("bun") now applies to the literal), and builtin-esm-lazy-exports.test.ts (comment update + the import() test now uses the literal, which fails on the old printer).

Security risks

None. No untrusted-input parsing, no auth/crypto, no filesystem or network surface. The only runtime effect is that a literal import("bun") now goes through the same loader path a computed one already did.

Level of scrutiny

Moderate. The src/ change is a small, well-scoped deletion in the printer plus a cache version bump, and the test coverage is thorough (each new test fails without the fix per the description). What raises this above auto-approve is that it is a deliberate user-visible behavior change: (await import("bun")) === Bun was previously true for the literal form and is now false (.default === Bun instead). The author explicitly calls this out as the one thing a maintainer should confirm. That is an API-surface decision, not a mechanical fix.

Other factors

My earlier inline note about stale comments in builtin-esm-lazy-exports.test.ts was addressed in 79213a9 — the file now correctly documents that only static import/require are rewritten, and the specifier indirection was removed so the test uses the literal directly. The comment-cop bot flags on comment length were also addressed and resolved. There is a related open PR (#35739) that overlaps textually in the same hunk; the description already explains the interaction. The PR depends on #37714 (lazy "bun" module exports) for the load-cost / exception-checks story; that dependency is stated and the branch is rebased onto it. Given the API decision and the stated dependency, a maintainer sign-off is the right bar here.

[claude]

I reviewed this PR and did not find any bugs. Because it deliberately changes what a literal await import("bun") returns (namespace instead of the Bun object) — a user-facing behavior change the description explicitly flags for a maintainer — a human sign-off on that direction would be worthwhile.

What was reviewed:

• Printer fall-through for ImportKind::Dynamic with ImportRecordTag::Bun reaches the external import() path with print_space_before_identifier(), so import_options and the --minify-whitespace separator are handled.
• ImportKind::Stmt still hits the globalThis.Bun inline (this arm is unreachable via print_require_or_import_expr; static imports are handled elsewhere), so no change there.
• Transpiler cache version bump matches the printed-output change; tests cover literal, inlined-const, non-inlined-const, let, bundler with/without --minify-syntax, and mock.module.

Extended reasoning...

Overview

Drops the ImportKind::Dynamic arm of the ImportRecordTag::Bun rewrite in src/js_printer/lib.rs::print_require_or_import_expr, so a literal import("bun") is printed as an external dynamic import and resolved by the loader instead of being replaced with Promise.resolve(globalThis.Bun). require("bun") and static import ... from "bun" are still inlined. RuntimeTranspilerCache EXPECTED_VERSION is bumped 25 → 26. Four test files add/update coverage: import-meta.test.js (four spellings of the specifier in child processes), bundler_bun.test.ts (bundled output shape + run), mock-module.test.ts (mock applies to the literal), and builtin-esm-lazy-exports.test.ts (uses the literal now that it reaches the loader; stale comments updated per my earlier note).

Security risks

None identified. This is a printer-output change for a hardcoded builtin specifier; no untrusted input reaches new parsing or allocation.

Level of scrutiny

High — the printer is on the hot path for every transpiled file, and the change is a deliberate, observable behavior change to user code: (await import("bun")) === Bun goes from true to false, and the returned object is now a module namespace whose default is Bun. Property/destructuring reads keep working (the namespace has a named export per Bun.* property), and the change makes the literal consistent with every non-literal spelling and with other builtins, so the argument for it is strong. But per the guidelines this is exactly the kind of "large scale design decision a human should weigh in on"; the PR body itself frames it as the one deliberate behavior change for a maintainer to look at.

Other factors

• The description notes textual overlap with open PR #35739 (drops the runtime half of both rewrites for the globalThis shadowing bug); a human can decide the merge order.
• Builds on #37714 (lazy "bun" module exports), already on main, so the extra module load the literal now triggers does not eagerly reify Bun.*.
• Prior review feedback (comment-cop nags on comment length; my note on stale comments in builtin-esm-lazy-exports.test.ts) has been addressed and resolved.
• I traced the fall-through in print_require_or_import_expr: with record.source_index invalid the dynamic case reaches the external-import() block, which calls print_space_before_identifier() and honors import_options, so the minify-whitespace returnimport concern and dropped-options concern do not apply here.