Repository navigation
Conversation
…ts statement
A module that is loaded with require(), or with import() without
splitting, is printed as `var init_x = __esm(() => { ... })`. The linker
printed a declaration with a side-effect-free value above that wrapper,
with the value. For a `var` that store has a place in the module: the
binding reads undefined before it, and a second declaration assigns
again. So `var mode = "dev"; log(mode); var mode = "prod"; log(mode)`
printed prod twice.
One predicate, Local::value_can_leave_esm_wrapper, now answers for the
linker and for needs_wrapper_ref in the parser. A var, a var merged with
a function and a lifted `exports.x = value` keep their store inside the
wrapper. let, const and class values still leave it: a read before the
statement throws in the source.
…epend on their order
… a var-style library
|
Updated 1:05 AM PT - Oct 19th, 2026
❌ @robobun, your commit 804c58d has 1 failures in
🧪 To try this PR locally: bunx bun-pr 45529That installs a local version of the PR into your bun-45529 --bun |
|
Status How I reproduced it:
The tests: This PR is a draft. It is stacked on #41601, and it also waits for #45705, #44001 and a fix for an import through CI on 804c58d (build 128766): no failure comes from this diff. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Additional findings (outside the current diff — GitHub can't attach inline comments there):
-
🔴
src/js_parser/p.rs— A lazily loaded module whose only export isexport default (function foo() {})now fails a debug assert, and in release bundles a wrapper under a bogus symbol so the default export stays undefined. The parser's SExportDefault arm at src/js_parser/p.rs:9655 still answers withed.can_be_moved()alone, but the linker turns that statement intovar foo = function foo() {}whose binding is the function's own HoistedFunction symbol, sovalue_can_leave_esm_wrapperat src/bundler/linker_context/generateCodeForFileInChunkJS.rs:762 keeps it inside the wrapper while nowrapper_refwas minted. Fix: make both sides agree for a default export of a named function (or async/generator function) expression. [also at: src/ast/s.rs:291 - A lazily loaded module whose default export is a named function expression now tripsdebug_assert!(!ast.wrapper_ref.is_empty())and, in release, its consumers readundefined.]Why this was flagged
Input:
entry.jsdoesrequire("./lib.js")(orimport()without--splitting) andlib.jsisexport default (function foo() {}).needs_wrapper_refonly checksed.can_be_moved()for SExportDefault (src/js_parser/p.rs:9656), EFunction is movable, sowrapper_refstaysRef::NONE(src/js_parser/p.rs:9404).convert_stmts_for_chunkrewrites the statement intovar foo = function foo(){}with bindings.default_name.ref_(src/bundler/linker_context/convertStmtsForChunk.rs:619). The newvalue_can_leave_esm_wrapperseessymbol.kind.is_hoisted_or_function()true (src/ast/s.rs:290) and returns false, so the store goes inside the closure andinner_len > 0, trippingdebug_assert!(!ast.wrapper_ref.is_empty())(src/bundler/linker_context/generateCodeForFileInChunkJS.rs:857). On the base branch the old testcan_be_moved && BIdentifierhoistedvar foo = function foo(){}outside the closure, matching the parser, and the bundle ran correctly.Verification: Triggered when a lazily loaded ES module's default export is a named function expression, e.g.
export default (function foo() {}). The parser'sneeds_wrapper_refSExportDefault arm (src/js_parser/p.rs:9655-9659) still only testsed.can_be_moved(), so nowrapper_refis minted.local.value_can_leave_esm_wrapper(generateCodeForFileInChunkJS.rs:762) returnsfalse. On the base both sides agreed, so this is a regression introduced by the PR.
|
Cross-reference: #45705 relocates a module-level With #45705 and without this PR, these orders give the value that the braced form gives on main. Each is a module that
Main is right there only because the nested |
…it binds a function's name
`export default (function foo() {})` reaches the linker as a local that binds
the symbol of `foo`. The predicate took that symbol for a `var` and kept the
store inside the `__esm` wrapper, but the parser had judged the statement by
its value and made no wrapper symbol. Test the kind of a `var` only.
Tests: the default export forms, a `var` merged with a function in both
orders, and a `var` store after a store in a block, in a body without braces
or after a `for` head without an initializer.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
The finding about Cause: the predicate tested 1cf570b tests the kind of a Checked:
|
|
Results for the orders in the comment above. Each module is loaded with
"In the body" is With both changes every value is right, and no case fails. The 20 tests that #45705 adds to This branch alone is not enough for a reader outside the module. The The tests are in
|
| !self.origin.is_commonjs_export() | ||
| // The kind of a `var` only, not of a function's name: the linker turns | ||
| // `export default (function f() {})` into a local that binds `f`. | ||
| && symbol_of(id.get().r#ref).is_none_or(|bound| bound.kind != symbol::Kind::Hoisted) |
There was a problem hiding this comment.
🔴 If this lands before #45705, a lazily loaded module whose var x is also declared in a braceless body (if (c) var x = 1;, for (var i; ...)) now exports undefined for it; the base exports the value. With the Hoisted check at src/ast/s.rs:293 the top-level var x = 2 becomes x = 2 inside the __esm closure, but the braceless var x stays a closure-local declaration, so the store writes that local and the outer x read by export getters and hoisted functions is never written. Fix: keep the store inside only once every nested var in the module body is turned into an assignment of the outer binding (what #45705 does), or land #45705 first. The author's own ReadFromOutside tests in test/bundler/bundler_edgecase.test.ts:3098 are todo for exactly this. [also at: src/bundler/linker_context/generateCodeForFileInChunkJS.rs:764 - A lazily loaded ES module that declares a var both in a braceless body and at top level now exports undefined for it, where the base bundle exported the right value. generateCodeForFileInChunkJS.rs:762 keeps x = 2 inside the __esm closure, but if (c) var x = 1; is never relocated and stays a closure-local var x, so the store never reaches the outer x.]
Why this was flagged
Input: a module such as if (globalThis) var x = 1; export var x = 2; loaded through require() or import() without splitting, so the linker gives it WrapKind::Esm, and read from outside the closure. maybe_relocate_vars_to_top_level (src/js_parser/fold.rs:59) does nothing for a braceless body because current_scope == module_scope, so the if/for keeps a real var x declaration and that statement goes inside the arrow at generateCodeForFileInChunkJS.rs:826. The top-level var x = 2 has a Hoisted symbol, so value_can_leave_esm_wrapper (src/ast/s.rs:293) returns false and the decl becomes var x; outside plus x = 2 inside. Inside the arrow the nested var x shadows the outer one, so x = 2 writes the closure-local and the outer x stays undefined. On the base the same decl is printed as var x = 2 outside the closure, so outside readers see 2. The PR's tests (test/bundler/bundler_edgecase.test.ts:3098-3103) are marked todo: true; the mitigation is landing #45705 first, which this PR does not enforce.
Verification: test/bundler/bundler_edgecase.test.ts:3098-3107 marks EsmWrapVarStoreAfterBracelessBodyReadFromOutside as todo: true "until #45705"; #45705 is not in this checkout. An SIf/SFor falls to _ => stmt (generateCodeForFileInChunkJS.rs:826) and stays inside the __esm arrow. value_can_leave_esm_wrapper (src/ast/s.rs:293) returns false, so x = 2 writes the local; the outer x stays undefined. On the base, the outside reader got 2.
There was a problem hiding this comment.
Right. On this branch alone the reader outside the module gets undefined for these modules, and main gives the moved value.
This PR is a draft now, and it stays one until the relocation is in its base. It adds no guard. While the second var is inside the wrapper, a guard only chooses which reader is wrong. The old output for such a file is wrong for the reader in the module body (export const inBody = x gives 1 on main, node gives 2). The output of this PR is wrong for the reader outside. The parser is the place that leaves the second var, and #45705 fixes it there.
#45705 keeps an exception for a file with a top-level function f and var f (#44001). With both PRs such a script still goes from 2 to undefined for the reader outside. So the base also needs #44001 and the end of that exception.
The description lists the prerequisites, the measured values, and the todo tests that become active then.
The same test as before. The condition has a name now, and each comment is one line.
using out of its try
#38284
|
Update, 2026-10-23. A fuzz report hit this bug again on main 5c6d4e2. I ran a second review of the design against main. This comment records the results. The reported input // cfg.mjs
var url = "http://localhost"; export const first = url; var url = "https://prod.example"; export const second = url;
// lazy.mjs
const m = await import("./cfg.mjs"); console.log(m.first, m.second);
The design stays The review compared the rule of this PR with five other designs: the rule of esbuild (#34934), a position rule, two rules that search for an early read, and no change. It kept the rule of this PR. A position rule ("a
So each rule that keeps a store in the wrapper shows the two bugs of main that the body lists. Main is wrong for them today if the value is computed: The import through // pkg/package.json: { "name": "pkg", "sideEffects": false, "type": "module" }
// pkg/barrel.mjs
export * from "./lib.mjs";
// pkg/lib.mjs
export var movable = 200;
export var computed = Math.max(500, 1);
export function get() { return computed; }
// entry.mjs
import { movable, computed, get } from "./pkg/barrel.mjs";
console.log(movable, computed, get());
await import("./pkg/lib.mjs");
The import binds to What this PR needs, in order
|
|
The base branch of this PR (#41601) is merged with main |
|
The fix for the import through One result of that work applies here too. With |
|
The base branch of this PR (#41601) moved to Two changes on the base matter here:
This PR needs a merge of its base to pick that up. |
Draft, stacked on #41601. Not ready: see Downsides.
Problem
require(), orimport()without--splitting) prints wrong values from a bundle:var mode = "dev"; log(mode); var mode = "prod"; log(mode)printsprodtwice.var x = <value>above the__esmwrapper if the value is side-effect-free (generateCodeForFileInChunkJS.rs:761). The store then runs at chunk load.Fix
var, and a liftedexports.x = value, keep their store inside the wrapper. One predicate (Local::value_can_leave_esm_wrapper) serves the linker andneeds_wrapper_ref.let,const,classandexport defaultvalues still leave it: the source throws on a read before the statement.test/bundler/bundler_edgecase.test.ts(23 new tests, 19 fail on the base). Self-reviewed: 18 concerns raised, 16 addressed, 2 kept (control tests, a TypeScript alias: Notes).Background
var init_x = __esm(() => {...})holds the body of a lazily loaded module. Its top-level names are declared outside. fix(bundler): become smarter with__esmwrappers #12729 moved side-effect-free values outside too.init_x()call. This change shows them for everyvar. bundler: evaluate a wrapped module's dependencies in source order and await every async one #41601 fixes the order bugs.Downsides
import { OK } from "http-status-codes"besideimport("http-status-codes/build/es/legacy.js")printsundefined(main: 200): an import throughexport *of a side-effect-free file runs noinit_x(). No PR yet.if (c) var x = 1;(no braces) the module exportsundefinedforx. Needs js_parser: relocate module-level vars from brace-less bodies for the bundler and the REPL #45705, and js_parser: print a var merged with a top-level function declaration as an assignment #44001 for a script withfunction fandvar f.varmodules: six packages behindimport(), minified, +1,060 B of 463,287 B (+0.23%).Notes
No GitHub issue reports this. It replaces #34934, which kept every initializer inside the wrapper and was closed by the stale sweep.
Not ready: what has to be in the base first
The order by ancestry is main, then the branch of #41601, then this branch. Three changes have to be in the base before this PR leaves draft:
init_x()calls). It is the base of this PR.var: js_parser: relocate module-level vars from brace-less bodies for the bundler and the REPL #45705, and js_parser: print a var merged with a top-level function declaration as an assignment #44001 together with the end of the exception that js_parser: relocate module-level vars from brace-less bodies for the bundler and the REPL #45705 keeps for a file withfunction fandvar f(has_top_level_function_merged_with_varinrelocates_single_stmt_body_decls). See "Nestedvar".export *of a side-effect-free file. See "Import throughexport *". No PR has it.After that: merge the base, enable the
todotests, add a test for case 3, and delete the symbol mark of #45705 that this predicate makes redundant (VAR_STORES_STAY_IN_WRAPPER).Repro
bun build ./entry.cjs --outfile=out.js && bun out.jstwice dev,twice prod,guard starting,first0 first1 second0 second1twice prod,twice prod,guard skipped,first0 first1The bundle from main has
var mode = "dev", mode = "prod";,var started = true;andvar out, i = 0, i = 0;above the wrappers.What keeps its place, what does not
value_can_leave_esm_wrapperis true for a declarator only if all of these hold:exports.x = value.var(Kind::Hoisted). Avarthat shares its name with a function has a symbol of its own with this kind, so it keeps its place too. The name of a function expression is not avar:export default (function foo() {})binds it, and that value leaves the wrapper as it does on the base.can_be_moved(). It is tested last, so avardoes not pay the walk of the initializer.can_be_moved()is not changed. The class arm of the linker loop is not changed. The parser asks the same predicate with its own symbol table. For a file that the parser classifies as CommonJS thevarclause is off, because such a file never gets an__esmwrapper. That keeps the output of those files as it is.Measurements (the base 1aefaeb against this branch, x64 Linux. The bundle bytes are from 1cf570b. The instruction counts, the wall clock and the binary size are from release builds of 0016a27: the commit after it replaces a test of three symbol kinds by a test of one.)
const m = await import(pkg); console.log(Object.keys(m).length);) for hono 4.7.2, lodash-es 4.17.21, rxjs 7.8.1, graphql 16.8.2, zod 3.24.2, uuid 11.1.0. With--minify: 463,287 to 464,347 B (+1,060 B, +0.23%),gzip -9+779 B (+0.57%). Not minified: 1,148,826 to 1,157,383 B (+8,557 B, +0.74%). Per package, minified: lodash-es +903 B, hono +109 B, rxjs +48 B, the other three 0 B.reactandreact-dom/client) with--production: 134,943 to 135,226 B (+283 B, +0.21%),gzip -9+261 B. The production build of react-dom is a required file with top-levelvars.--minify: 14 of 14 byte-identical (cmp).varstores in the six wrapped bundles: 338. Of them 37 are function or class values that are no longer created at chunk load. Modules that gain a wrapper: 1 (in rxjs).__esm(982 to 986, empty wrappers 12 to 9,init_x()calls 3,134 to 3,137.needs_wrapper_ref, instructions per bundled file (gdb single-step, callees included): graphql 7,129 to 8,702 over 125 files, lodash-es 36,285 to 41,963 over 642 files, rxjs 11,834 to 13,774 over 226 files. That is +9 to +13 per file (three.js: 355 to 459 over 4 files). No allocation is added. valgrind and perf are not available in the build container, so the linker function has no instruction count.import("lodash-es")64.4 to 63.8 ms (base against base: 0.9 ms). The three.js static bundle moved by +1.8 and +2.7 ms in two runs, with a base against base spread of 0.1 to 2.5 ms, and--no-bundleof the same file did not move (16.6 ms both).sizetext +512 bytes. The stripped file has the same size.test/bundler/bundler_bytecode_portable.test.ts: three hashes change (the bundles thatrequire()libraries).libraries.js(2,924 modules) grows by 751 B of 16.7 MB. 1cf570b gives the same three hashes as 0016a27. The commit after it (804c58d) only names the condition: the six minified bundles are byte-identical.varand function declarations gains a wrapper and aninit_x()call (+57 B minified), though nothing can read it.Random module graphs (sources that never throw:
varexports and function declarations. Each bundle runs under node and is compared with node on the source. "var alone" is main with eachexport var v = xwritten asvar v; v = x; export { v }.)require()entryimport()entryrequire()an ES moduleexport *, top-level awaitThe numbers are graphs where every printed value is right. Right on main and wrong on this branch: 0 of 2,611. The generator has no
varin a body without braces. That shape is right on main and wrong on this branch alone for a reader outside the module (see "With #45705"). With thevarchange alone on main, 8 graphs of the last family go from right toundefined, because anexport *of a module with top-level await is not awaited there. The self-review of the first version also reported constructed cyclic graphs (CommonJS, top-level await) that go from right toundefinedor to aTypeErrorwith thevarchange alone. That is why this PR is on #41601 and holds no order fix of its own.Nested
varThe rewrite
var x = vtovar x;outside andx = vinside is only right ifxinside the wrapper is the outer variable. The parser leaves two kinds of module-levelvardeclaration inside the module body, and so inside the wrapper, where they declare a secondx: avarin anif,else,whileordobody without braces, andfor (var i; ...)without an initializer. Module:if (globalThis) var x = 1; var x = 2;(node gives 2), loaded withimport():xexport const inBody = x)export function read() { return x; })undefinedMain is right for the reader outside only because the moved
var x = 2writes the outer variable and the store in theifbody writes the inner one. This branch keepsx = 2in the wrapper, where it writes the inner variable too, so the outer one staysundefined.#45705 makes it one variable in the parser. Its head has moved since that measurement (76f5a6d): it now also keeps the stores of a
varthat it relocates inside the wrapper, with a mark on the symbol. This predicate makes that mark redundant.#45705 does not relocate in a file that has a top-level
function fandvar f(#44001). Such a script, loaded withrequire()and--format=cjs:function f() { return 1; } var f = 2; if (globalThis) var x = 1; var x = 2; function read() { return x; }. In the body and from outside: node 2 and 2, main and the base 1 and 2, this branch 2 andundefined, this branch with #45705 (0e5df92) 2 andundefined.The six wrapped package bundles are byte-identical on this branch with and without the source diff of #45705 (0e5df92): none of their 986 wrapped modules has such a
var.Tests: eight orders (the body of
if,else,whileanddo, an exportedvar, two names, aforhead, a read above both stores), each plain and withminifySyntax. The reads in the module body run now (EsmWrapVarStoreAfterBracelessBody,...MinifySyntax) and fail on the base. The reads from outside aretodo(...ReadFromOutside,...ReadFromOutsideMinifySyntax). With the source diff of #45705 (0e5df92) applied to this branch they pass, and so do the 20 tests that it adds tobundler_edgecase.test.ts.Import through
export *of a side-effect-free fileundefinedexport var x = [2][0](a value that cannot move)undefinedundefinedThe bundle has no
init_lib()call before the code of the entry. The call belongs to theexport *statement ofbarrel.mjs. Tree shaking drops that statement, because nothing depends on it: an import that binds throughexport *depends only on the part that declares the name (theimports_to_bindloop inscanImportsAndExports.rs). A named re-export (export { x } from) is recorded as a dependency (re_exports), and that form is right on every build. So this bug is on main, and the moved value hides it there.Real packages on this branch:
import { OK, NOT_FOUND } from "http-status-codes"besideimport("http-status-codes/build/es/legacy.js")printsundefined undefined(main200 404).import { Registry } from "shiki/textmate"besideimport("@shikijs/core")givesundefined(main: a function). A review found the same in more graph shapes:require()from a CommonJS file,import * as ns, twoexport *hops, a barrel in another package,"sideEffects": ["*.css"], an entry point that re-exports the barrel,--splitting,--format=cjsand--minify.Why no guard in this PR
A design review weighed a flag from the parser for a file with a nested
var, a mark on the symbol, a scan of the wrapper body in the linker, and a copy of #45705 in this branch. While the secondvaris inside the wrapper, a guard only chooses which reader is wrong: with the old output for such a file the reader in the module body is wrong, with the output of this PR the reader outside is wrong. The parser is the place that leaves the secondvar, and #45705 fixes it there. A copy would fork an open PR that still changes. So this PR waits as a draft, and a guard is not built.Self-review
The first version of this change also reordered
export * fromand declared__promiseAllfor an unwrapped file. The review asked for a smaller change: those parts copy #41601 and #42046, the partial order fix could printawaitin a wrapper that is not async, and thevarchange needs the full order fix below it. The points about the copied hunks, the broken graphs and the parse failure are addressed by the removal of those hunks and by the new base. The other points are stated here: no user reported the symptom, bundles that wrap avarmodule get new bytes and content hashes, and the question below is open.A second review covered the change of the predicate to
Kind::Hoistedand the new tests. It bundled 2,296 more modules in up to six load modes and several flag sets (50,528 results) and found no module for which the parser and the linker disagree. The only values that this branch moves away from node are the reads from outside in "With #45705". It raised six points. Four are addressed: thetodotests now state what this branch alone prints, the reads from outside are in Downsides, a test comment claimed node output for entries that node cannot run as written, and this description was stale. Two are kept as they are:EsmWrapLexicalValuesNeedNoWrapperis the control against a wider predicate.EsmWrapDefaultExportValueNeedsNoWrapperfails on 0016a27, the commit before the last one. The twoexport *tests pin the order that avarstore at its statement depends on.import x = E.Awith a const enum value still leaves the wrapper. Bun prints it as aconst, so a read before the statement throws underbun run. tsc prints avar.A third review, of the design, looked at the nested
var. It kept the design, ruled out a guard (above), and found the import throughexport *. Both are in Downsides, and the PR is a draft because of them.edgecase/EsmWrapperElimination1(from #12729) still passes, but its module declares its exports withvar, so it keeps its wrapper now.EsmWrapLexicalValuesNeedNoWrapperasserts the elision forlet,constandclass.Question for a maintainer
esbuild keeps every initializer inside
__esm. This PR moves only the stores that a running program can read early.let,const,classandexport defaultvalues keep the #12729 hoist and the wrapper elision. So a CommonJS module that requires its ES importer during an import cycle still seesexport constvalues before the ES module body runs (node throwsERR_REQUIRE_CYCLE_MODULEthere, andbun runevaluates the ES module inside therequire()). If those should follow esbuild too, the predicate returns false for every initialized local and the class arm goes. That moves every movablelet,constand class of a wrapped module, and a module with only such declarations gets a wrapper and aninit_x()call.A narrower rule is possible too, and it is not built: keep the value outside for a
varthat is declared above the first statement that stays in the wrapper. It fixes each module of the repro, and a module that holds only declarations (the constants ofhttp-status-codes) stays as it is on main. But a module that an import cycle reads early still shows such avarwith its value.One more effect of a value above the wrapper, for the same decision: in cjs and iife output a function value of a
"use strict"ES module that leaves the wrapper runs as sloppy code (export const f = function () { return this === undefined; }givesfalse, node givestrue). #46578 prints the directive inside the wrapper and lists this case as still sloppy. With every initializer inside, that case is strict too. Avarwith such a value is already inside with this PR.Not changed here
export defaultup (parse_entry.rs), and the dev server moves export values. Both sharecan_be_moved().require()of a CommonJS file that holds only hoistable statements prints invalid code (bundler: allocate wrapper_ref for CommonJS modules with no non-hoistable statements #33881).function fandvar fbundles to ESM that does not load (js_parser: print a var merged with a top-level function declaration as an assignment #44001).export var a = 1; var a = 2;loses its declaration in every bundle, wrapped or not (js_parser: record the merged symbol for an export that is re-declared later #42409).export default (function foo() {})or(class Foo {})binds the inner name of the expression. With--minify-identifiersit can get the name of another symbol (var o = function o() {...}besidefunction t(o) { return o + o(); }), andexport default (function foo(foo) {})loses its declaration.bun runevaluates three CommonJS imports of one ES module as first, third, second (import "./a.cjs"; import "./b.cjs"; import "./c.cjs";runs a, c, b). node runs them in order.Suites
At 0016a27,
test/bundler/andtest/regression/issue/cyclic-imports-async-bundler.test.jswith release builds: 7,701 pass on the base, 7,721 pass on this branch (the 20 tests of that commit), 0 fail on both.At 1cf570b with debug builds: -
bundler_edgecase: 221 pass, 13 todo. The 20,000-moduleDeepImportDiamondDAGexceeds the 60 s limit of its build step in this container, with the debug build of the base too (77 and 136 s for the base, 90 and 112 s for this branch, identical output).bundler_dynamic_import_dce333 pass,bundler_cjs2esm113 pass,esbuild/importstar99 pass,esbuild/splitting26 pass,cyclic-imports-async-bundler1 pass.esbuild/default: 153 pass.TopLevelAwaitForbiddenRequireSourceMapCLIand...API(40 builds in 5 s) time out with the debug build of the base too.bundler_bytecode_portablethat this PR re-hashed: the same JS and bytecode hashes as in the test..mjs,.js,.cjsand.ts), each loaded withimport()and withrequire(): 0 asserts, and all 744 results equal the results of a release build of main (367d939). On 0016a27 the assert!ast.wrapper_ref.is_empty()fires for 50 of them, each with a named function expression as its default export.no test proof · iteration 4 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/bundler/bundler_bytecode_portable.test.ts