bundler: rename esm top-level vars that collide with host globals

[robobun]

Repro

// helpers.js
export const getComputedStyle = el =>
  el.ownerDocument.defaultView.getComputedStyle(el, null);

// entry.js
import { getComputedStyle } from "./helpers.js";
getComputedStyle(document.body);

$ bun build entry.js --outfile=out.js
$ cat out.js | head -2
// helpers.js
var getComputedStyle = (el) => el.ownerDocument.defaultView.getComputedStyle(el, null);

Loaded via <script src="out.js"> (classic script, no type="module"),
var getComputedStyle at the top level becomes a property of window,
so defaultView.getComputedStyle resolves back to the var and the
function recurses into itself:

Uncaught RangeError: Maximum call stack size exceeded
    at getComputedStyle (out.js:2:45)
    at getComputedStyle (out.js:2:71)
    ...

Chart.js ships exactly this helper; bundling it with Bun's default
--format=esm and loading the result in a classic script tag breaks at
runtime. The same output works when loaded as a module, and esbuild
--format=esm produces the same var and fails the same way (esbuild
only avoids it by defaulting to iife).

Cause

select_local_kind in the parser rewrites module-scope const/let to
var during bundling so the declaration can be hoisted out of an
__esm/__commonJS wrapper. In ESM format the chunk has no wrapper, so
the var sits at the true top level.

The renamer only reserves names for unbound identifier references. Since
.defaultView.getComputedStyle is a property access and no bare
getComputedStyle reference appears anywhere in the bundle, the name is
never reserved and the declared symbol keeps it.

Fix

While building the reserved-name set, walk each module scope once more
and reserve any declared top-level name that is in the pure-global
identifier table (defines_table). The number renamer then suffixes the
binding (var getComputedStyle2) and all references follow; the
.defaultView.getComputedStyle property access is untouched, so the
output works whether loaded as a module or a classic script.

Scope is kept tight so nothing changes for code that cannot hit the
hazard:

• only --format=esm --target=browser (Bun/Node load ESM in module
  scope, where var never reaches globalThis, so --target=bun|node
  and --compile are untouched);
• WrapKind::Cjs modules are skipped because their declarations stay
  inside the __commonJS closure;
• only symbol kinds that print as a VarScoped declaration
  (var/function/async function/function*) are considered;
  class and import bindings stay lexical at the chunk top level and
  cannot clobber globalThis, so class Response {} keeps its .name
  and import { Response } from "pkg" keeps its alias.

Verification

edgecase/EsmTopLevelVarShadowsHostGlobal bundles the Chart.js-shaped
helper and evaluates the output via indirect eval (classic-script
scope). On main it overflows; with the fix it calls the host
getComputedStyle once.

EsmTopLevelVarShadowsHostGlobalScoping pins the gates: class Response {} keeps class Response, an external import { Response }
keeps the bare alias, and a CJS-wrapped module keeps { URL, fetch }
shorthand. EsmTopLevelVarShadowsHostGlobalTarget_{bun,node} confirm
that --target=bun|node leave var getComputedStyle/var name
unchanged, and IifeTopLevelVarShadowsHostGlobal confirms iife output
keeps the name (the IIFE already scopes it).

Fixes #14110

---

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

[coderabbitai]

Walkthrough

Changes

ESM global reservation

Layer / File(s)	Summary
Global lookup and ESM reservation src/js_parser/defines_table.rs, src/bundler/linker_context/renameSymbolsInChunk.rs	Adds pure-global identifier detection and reserves matching renamable module symbols before ESM renamer initialization.
Global shadowing edge cases test/bundler/bundler_edgecase.test.ts	Adds ESM and IIFE tests covering getComputedStyle, requestAnimationFrame, and fetch output and runtime behavior.

Suggested reviewers: jarred-sumner

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The change addresses #14110 by reserving conflicting globals in ESM output and adds regression tests for the failure.
Out of Scope Changes check	✅ Passed	The code and tests are all directly related to the global-name collision fix, with no obvious unrelated changes.
Title check	✅ Passed	The title clearly summarizes the main change: renaming ESM top-level vars that collide with host globals.
Description check	✅ Passed	The description covers what changed and how it was verified, though it doesn't follow the template headings exactly.

---

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

[claude]

I didn't find implementation bugs, but this is a behavioral change to default bundler output that a maintainer should sign off on: every ESM bundle with a module-level binding matching one of the ~640 names in defines_table (export class Response, export const fetch, export function alert, const Map = …, etc.) will now have that binding suffixed. It also intentionally diverges from esbuild's ESM behavior. Whether that trade-off is right — vs. documenting that --format=esm output must be loaded as a module, or gating this behind a flag — is a design call.

What was reviewed:

• The reserved-name loop mirrors compute_reserved_names_for_scope's member walk with the inverted kind predicate, and reserved_names feeds both the number and minify renamers.
• is_pure_global_identifier is a thin wrapper over the existing generated lookup; the pub visibility is needed for the cross-crate call from bun_bundler.
• Searched bundler snapshots for top-level var <global> collisions that this would have flipped — none found.

Extended reasoning...

Overview

The PR reserves any declared top-level name in an ESM chunk that appears in the parser's ~640-entry pure-global table, so the number/minify renamer suffixes the binding (var getComputedStyle2). It adds is_pure_global_identifier as a public wrapper in js_parser::defines_table and three itBundled cases covering the Chart.js repro, an unbound-reference control, and an iife-format control.

Design trade-off

The failing scenario is loading --format=esm output as a classic <script>. That's already only partially supported — an ESM bundle whose entry has exports emits top-level export {…}, which is a syntax error in a classic script — so the fix targets a narrow use case (export-less ESM bundles loaded as classic scripts). The change it makes is broad: any module-level binding named Response, fetch, Map, Error, URL, document, etc. is now renamed in every ESM bundle regardless of how it's loaded. The PR description notes esbuild has the same behavior and does not rename; this deliberately diverges. That's a reasonable choice, but it's a default-output design decision that should get a maintainer's eyes rather than a bot approval.

Level of scrutiny

Bundler renamer output is user-visible and covered by many snapshot tests; a change here touches every ESM build. I grepped bundler .snap files and test/bundler for top-level var <known-global> patterns and found none, so existing snapshots likely don't flip, but the blast radius still warrants human review.

Other factors

Implementation looks correct: the new loop reads scope.members the same way compute_reserved_names_for_scope does, guards on kind != Unbound && !must_not_be_renamed(), and .put(.., 1) is idempotent with the existing reservation for unbound refs of the same name (so the typeof fetch control case works). reserved_names is consumed by both MinifyRenamer::init and NumberRenamer::init, so the fix applies with and without --minify-identifiers. No memory-safety or SB concerns beyond what the surrounding code already documents. No prior review comments on the PR.

[robobun]

^{Updated 12:14 PM PT - Jul 25th, 2026}

✅ @robobun, your commit 8c233459cbf7937bd1ffb81dd387dc2453d97d2f passed in Build #81081! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 35617

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

bun-35617 --bun

[claude]

Prior inline feedback (add TsEnum/TsNamespace to the matches!, trim the bug-history comment) was addressed in 8c23345; no new issues found this pass. Deferring to a maintainer because this is a deliberate bundler-output policy change — it diverges from esbuild by proactively renaming any browser-ESM top-level var/function/const/let whose name is in the ~600-entry pure-global table, which changes .name for those bindings and is a call someone should sign off on.

What was reviewed:

• Reserved-name loop placement (runs before both MinifyRenamer and NumberRenamer init, so both paths honor it) and the WrapKind::Cjs skip.
• Symbol-kind gate against the lowering paths that emit chunk-top-level var; the remaining Class-in-WrapKind::Esm gap is acknowledged with a stated trade-off.
• Test coverage pins the negative contract: target=bun|node, format=iife, CJS-wrapped modules, class, and external import aliases keep their original names.

Extended reasoning...

Overview

The PR adds a pass in rename_symbols_in_chunk that, for --format=esm --target=browser chunks, walks each non-CJS-wrapped module scope and reserves any declared top-level name that appears in the parser's pure-global identifier table (defines_table::is_pure_global_identifier). The number/minify renamer then suffixes the binding so a scope-hoisted var getComputedStyle becomes var getComputedStyle2, preventing it from clobbering window.getComputedStyle when the ESM output is loaded via a classic <script> tag. A one-line is_pure_global_identifier wrapper is added in defines_table.rs, and six itBundled cases are added covering the fix and its gates.

Security risks

None. This is bundler symbol-renaming logic operating on already-parsed ASTs; no new I/O, no untrusted-input parsing, no privilege boundaries.

Level of scrutiny

Medium-high. The mechanism is small and localized, but it changes emitted output for the default --target=browser --format=esm configuration whenever a module-scope binding shares a name with any of the several hundred globals in the pure-global table (Map, URL, fetch, Response, DOM/CSSOM constructors, etc.). That's an intentional divergence from esbuild, which the PR description calls out. The gating (browser-only, ESM-only, VarScoped-kind-only, skip WrapKind::Cjs) is careful and well-tested, but whether this is the right layer for the fix versus, say, recommending --format=iife for classic-script consumers, is a product/design call.

Other factors

• I previously flagged two issues inline; both are resolved (TsEnum | TsNamespace added to the matches!; the Chart.js/#14110 bug-history parenthetical removed and the comment compressed to one line). The author left Class out of the matches! with an explicit trade-off argument — including it would rename movable class Response {} in WrapKind::None modules and break .constructor.name, which the Scoping test now pins.
• The reserved-names loop runs before both renamer constructors, so --minify-identifiers picks up the reservation too (verified by reading MinifyRenamer::init — it takes reserved_names by value at that point).
• Tests exercise the runtime failure via indirect eval (matching classic-script scope), assert the property-access .defaultView.getComputedStyle is untouched, and pin that target=bun|node and format=iife output is byte-identical to before. The negative-contract coverage is solid.
• One user-visible side effect worth a maintainer's eye: export function URL() {} in a browser-ESM bundle now has .name === "URL2". That's inherent to the approach and probably fine, but it's the kind of thing a human should ratify.