Repository navigation
Conversation
…top-level await An import statement in a file that is printed inside a __commonJS wrapper is lowered to a call of the imported file's __esm wrapper. When that file is async (a top-level await is on its chain of static imports), the call was printed as `await init_x()` inside the wrapper, a plain function, and the bundle did not parse. The linker now reports that import as an error, after tree shaking and before code generation. The check and the code generator take the lowering of an import statement from one function.
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 47 seconds. View limit details
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. Comment |
|
Status Reproduced on main (179664e, release build) and on Bun 1.4.3:
With this PR the build exits 1 and reports the PR: #46749 CI (build 132039, c37de90): the diff is green on every job that ran a test. 179 jobs passed. 7 test files failed once and passed on retry, and none of them is a bundler test. Two jobs cannot start: no agent takes |
|
Updated 10:01 AM PT - Oct 24th, 2026
✅ @robobun, your commit c37de907a015724b935a671e0a04dcd3f3d38379 passed in 🧪 To try this PR locally: bunx bun-pr 46749That installs a local version of the PR into your bun-46749 --bun |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.
Beyond the inline finding, I also diffed the import_stmt_lowering extraction against the old should_remove_import_export_stmt arm by arm: each variant emits the same statement with the same loc (external keeps record.range.loc, CJS keeps loc), WrapKind::None still falls through to removal, and the ? on the external arm only changes an expect into error propagation. I also checked the has_any_top_level_await_modules gate in generateChunksInParallel.rs: the validator is skipped when no module is async, which is when Init { is_async: true } cannot occur.
Extended reasoning...
The change refactors the import-statement lowering in src/bundler/LinkerContext.rs into a shared decision enum, adds a pre-codegen validator that fails the build when a __commonJS-wrapped file imports an async __esm module, and adds a debug assertion plus 38 tests. It touches no security-sensitive surface. The one confirmed inline finding (unaliased export * from still slips through) and the overall size of the linker change are why a human look remains worthwhile.
| // The statements that `convert_stmts_for_chunk` gives to | ||
| // `should_remove_import_export_stmt`. It prints `export * from` without an | ||
| // alias as a bare `init_x()`, so that statement needs no `await`. | ||
| let (namespace_ref, import_record_index) = match stmt.data { | ||
| bun_ast::StmtData::SImport(s) => (s.namespace_ref, s.import_record_index), | ||
| bun_ast::StmtData::SExportFrom(s) => { | ||
| (s.namespace_ref, s.import_record_index) | ||
| } | ||
| bun_ast::StmtData::SExportStar(s) if s.alias.is_some() => { | ||
| (s.namespace_ref, s.import_record_index) | ||
| } | ||
| _ => continue, | ||
| }; |
There was a problem hiding this comment.
🟣 pre-existing, not blocking: pre-existing: users whose CommonJS file has export * from a top-level-await module still get a bundle that silently reads exports before they are initialized, where every other import shape now gets the new build error. The validator at src/bundler/LinkerContext.rs:2428 only inspects SExportStar when s.alias.is_some(), and convertStmtsForChunk.rs:276-298 prints the un-aliased form as a bare init_x() with no await inside the __commonJS wrapper. Fix: treat an un-aliased export * (including the lifted module.exports = require(...) form at convertStmtsForChunk.rs:301) whose target is Init { is_async: true } as the same error, so every statement that calls an async __esm wrapper from a __commonJS body is rejected, not only the three awaited shapes. [also at: src/bundler/LinkerContext.rs:2422 - pre-existing: users whose __commonJS-wrapped file re-exports a top-level-await module with an unaliased export * still get a bundle that reads that module's exports before its await resolves, with no error.]
A small fix can ride a push you are already making; otherwise a short reply is enough.
Why this was flagged
Input: c.js is CommonJS for the bundler and contains export * from "./tla.js" where tla.js is await 0; export const t = 1;; entry does import c from "./c.js"; console.log(c.t). bun build runs validate_commonjs_imports_of_async_modules (src/bundler/LinkerContext.rs:2384); the match at LinkerContext.rs:2428 skips SExportStar without alias, so no error is reported. Code generation at src/bundler/linker_context/convertStmtsForChunk.rs:272-298 appends init_tla() via append_non_dependency (no await) inside the __commonJS function, then __reExport(exports, tla_exports). init_tla is an async __esm wrapper: t = 1 runs only after the await 0 microtask, so require_c() returns synchronously and c.t is undefined. The base branch produces the same output, so this is pre-existing; the PR's test cjs/tla_import_export_star (which only reads c.a) does not make the output correct. The lifted form at convertStmtsForChunk.rs:301-323 has the same exposure.
Verification: pre-existing; acknowledged in diff: the PR description says "Not taken: an error for export * (it runs today)". The validator's match at src/bundler/LinkerContext.rs:2428 is SExportStar(s) if s.alias.is_some(), so the un-aliased form falls to _ => continue. src/bundler/linker_context/convertStmtsForChunk.rs:272-298 emits a bare init_x() with no await, so init_x() returns a promise that nobody waits for.
There was a problem hiding this comment.
This is the question that the PR body leaves for a maintainer (Notes, "Same class, not in this PR"). I did not add the error, because these builds run today. Facts for that decision, from release builds of main (179664e) and of this branch (c37de90):
c.jsisexport * from "./tla.js"; exports.a = 1; Object.keys(exports);. Both builds exit 0 and print a bareinit_tla();in the wrapper. A function declaration or a literal oftla.jsis ready at once. A value thattla.jsassigns after itsawaitreadsundefined, and1three microtasks later.- The names are there: the wrapper has
__export(exports, { t: () => t }). The linker prints no__reExportfor this file. - Only the
bun buildbundle runs that source. node andbun runtreat a file withexportas an ES module and throwReferenceError: exports is not defined. esbuild 0.21.5 bundles it as an ES module, and the bundle throws aReferenceErroratexports.a = 1. - The lifted form is not exposed.
module.exports = require("./tla.js")fails the build on main and here:This require call is not allowed because the transitive dependency "tla.js" contains a top-level await. The same holds when therequire()target reachestla.jsthrough anexport *.
If a maintainer wants the error, I add this statement to the check, with a hint of its own (the import() hint does not fit a re-export).
|
#41601 changed at So three lines in the body of this PR do not hold any more. #41601 does not make that statement wait, does not change |
Problem
bun buildexits 0 with a bundle that does not parse:error: "await" can only be used inside an "async" function.await init_tla()inside__commonJS(function(exports, module) {. Trigger: a CommonJS file has animportof a module with top-level await.should_remove_import_export_stmt(src/bundler/LinkerContext.rs:2252) takes theawaitfrom the async flag of the imported file. It ignores the wrapper of the importing file.Fix
validate_commonjs_imports_of_async_modulesreports each suchimportas a build error before code generation. Notes give the import chain to theawait.import_stmt_lowering.test/bundler/bundler_cjs.test.ts(38 new tests, 26 fail on main). Other suites: Notes.export *(it runs today), a narrower async flag (separate change), a tracking issue.Background
__commonJS(function(exports, module) {...}), a plain function. A file that usesmoduleorexportsis CommonJS, also withimportstatements.var init_x = __esm(...). A top-level await on its import chain makes that wrapperasync.await. No user report exists.Downsides
awaitthat tree shaking removed (Notes)..text+3,072 B. With a top-level await: 20,000 record reads for 20,000 CommonJS files, 0 allocations. Otherwise: one bool test.Notes
Repro
bun build entry.js --outfile=out.js && bun out.js. On main (179664e) and on 1.4.3 the build exits 0 and the run fails at line 54 ofout.js:With this PR the build exits 1:
For a chain of imports the headline says
the transitive dependency "tla.js", and one note perimporton the way comes first (The file mid.js imports the file tla.js here:). These are the two note texts of therequire()error. Both errors now take them fromtop_level_await_noteandimport_toward_top_level_await_note.Origin. No user reported this. It was found by a read of the code. I found no issue for an
awaitinside__commonJS. The same symptom in an__esmwrapper was reported and fixed in 2024 (#12508, #13607).The latent form. With
--splitting, or with a second entry point, the file that does not parse can be one that loads later. The build exits 0, the program starts, and theSyntaxErrorcomes when that file is first imported.entry.jswithif (process.argv[2] === "rare") await import("./c.js")runs on main until the argument israre.When the error is reported. Exactly when the linker would print
await init_x()inside a__commonJSwrapper:__commonJSwrapper,import, anexport ... fromor anexport * as ns fromin a part that tree shaking kept,__esmwrapper, and is marked async.import_stmt_loweringholds the conditions on the imported file.should_remove_import_export_stmt(the code generator) and the new check both call it. Adebug_assert!ingenerate_code_for_file_in_chunk_jsfails when any code puts an awaited dependency into a__commonJSwrapper. With the check disabled in a local build, that assertion fired for the repro.The check runs at the start of
generate_chunks_in_parallel, on the bundle thread.bun build,Bun.build,--compile, HTML entry points andbun build --appgo through it. The commands that only link (bun install --analyze,bun add --analyze) return before it: on the repro they exit 0 on main and with this PR.--format=internal_bake_devhas no__commonJSwrapper and is not checked.Which files are CommonJS for the bundler. The parser decides (
src/js_parser/parse/parse_entry.rs). A file is CommonJS when it has noexportand referencesmoduleorexports. That includestypeof module,module.hotand TypeScriptexport =. A directevalcounts as a reference to both. The file name does not matter: a.mjsfile withtypeof moduleis CommonJS too. The linker also puts a liftedexports.x = ...file back into a wrapper when an importer holds its default import as a value.bun run, sourcebun build, mainimportandmodule.exportsawaitin__commonJS, does not parseimportand a directevalawaitin__commonJS, does not parseeval.mjs,importandtypeof moduleFor the direct
evalthe extra note is:A direct eval makes a file without an "export" CommonJS. Add an "export" to "c.js", or call eval as (0, eval)(code). Two tests build the file with each of these two changes. The notes give no reason for the other files: at link time the flags formoduleandexportsare also set forexport =, for a directevaland by the linker, so a reason read from them can be false.Fixtures. 100 builds of small graphs, release builds of main (179664e, md5 105b11a1) and of this PR (c37de90, md5 883bc10c):
awaitinside__commonJSon main now exit 1.A second set of 159 builds: 47 of 47, 0, 96 of 96, 16 of 16.
Builds that keep working (each is a test): a lifted
exports.xfile that importers read by name, an unused import of a"sideEffects": falsepackage (also when another file uses the package), a type-only import, a CommonJS file that is not in the bundle,export *without alias, a sync import beside a top-level-await module,--format=internal_bake_dev.--ignore-dce-annotationsandtreeShaking: falsekeep the package in the bundle, and the same sources then get the error.Measurements. Release builds of main and of this PR, the md5 values above.
size -A:.text82,164,617 to 82,167,689 (+3,072 B). Every other section is the same.size_of::<LinkerContext>()1160 to 1160,size_of::<RequireOrImportMeta>()20 to 20.import_stmt_loweringis inlined into its two callers (#[inline(always)]). With plain#[inline]it stayed a call in the code generator.scan_imports_and_exportsreads 40,000 records on the 20,000 graph. Instructions: 1,986,936,760 to 1,987,809,308 (+0.044%, main against itself -0.089%).__rust_alloc,__rust_realloc,__rust_alloc_zeroed): 0 on a build that passes (2,000 CommonJS files, top-level await in the entry), 19 on the repro.await: 9.09e9 instructions for 5,000 errors, 18.06e9 for 10,000, 35.99e9 for 20,000. Therequire()error on the same graphs: 9.01e9, 17.91e9, 35.72e9. The chain table is built once, for the first error.--sourcemap=inline: 40 of 40bun buildruns and 40 of 40Bun.buildruns report the error (debug + ASAN build).perf,valgrind,heaptrack,straceandbloatyare not in the container. The counts are fromgdband aperf_event_opencounter.Same class, not in this PR.
export * fromwithout alias, in a CommonJS-wrapped file: the linker printsinit_x()and does not wait, so the file reads values before theawaitfinishes. It builds and runs on main and here. Question for a maintainer: refuse it too? bundler: keep source order in a wrapper prefix that has no async dependency #41601 makes that statement wait. It then has to go throughimport_stmt_loweringand this check.exports.a = 1; Object.keys(exports); await 0;, orawait import()at the top level) still builds toawaitinside__commonJS. The parser picks CommonJS before it looks at theawait. js_parser: make the ESM/CJS classification and the module/exports bindings agree #40840 and js_parser: do not bind module or exports in a file whose module type is ESM #46037 change that choice. A check on the keyword would refuse builds whoseawaitis dead code and that run today.awaitthat is not in the bundle:c.jsimportsmid.js, andmid.jshas an unused import of a"sideEffects": falsepackage with a top-level await.mid.jskeeps anasyncwrapper, andc.jsgetsawait init_mid(). On main that build does not parse.require()of such a file is refused for the same reason. A pass that narrows the flag after tree shaking fixes both and needs no change here.import()of a CommonJS file that is marked async: bundler: do not treat a__commonJSwrapper as async inimport()#46748.awaitin a plain__esmwrapper on an import cycle: bundler: compute top-level await as a fixed point #45949, bundler: keep source order in a wrapper prefix that has no async dependency #41601.__promiseAllnot defined: bundler: define __promiseAll when an unwrapped file awaits two async ESM wrappers #42046, bundler: keep source order in a wrapper prefix that has no async dependency #41601.Weighed and not built.
__commonJSwrapper is printed (about 12 lines): it refuses the same builds, after every file is generated, with a file name and no position or chain.await init_x()in the importers of the CommonJS file, above the call of its wrapper:require()of the file has no place to wait, and the imports of the file no longer run in source order.Related open PRs.
should_remove_import_export_stmt: itsappend_dependency(..., is_async)goes into theInitarm. Its awaitedexport *needs thealias.is_some()guard in the check removed, andcjs/tla_import_export_starthen expects the error. Thedebug_assert!readshas_async_dependency, which it removes.validate_tla. The conflict is in the two note helpers only.cjs/tla_import_typeof_module_in_mjs,cjs/tla_import_export_from,cjs/tla_import_export_star_asandcjs/tla_import_export_star.Self-review. 25 concerns. Changes that came from it: the chain is computed once per build and not per error,
validate_tlashares the note text, the hint no longer namesmodule.exportsand no longer suggests animport()that a reader couldawaitat the top level, the note for a directeval, 12 more tests (option twins, chains, files that are CommonJS withoutmodule.exports, two entry points), no docs sentence, and theimport()change moved to #46748. Not taken:export *without alias. It runs today. It is the question above.Tests.
test/bundler/bundler_cjs.test.ts: 100 pass with the debug build of this branch. With a release build of main, the 26 tests that expect the error fail and the 12 tests that expect a build pass. The tests are in this file and not intest/bundler/esbuild/default.test.ts: two 40-build tests there time out under a debug build (#46028).Suites. Release build of this branch: all 71 files of
test/bundler/andtest/bundler/esbuild/,test/regression/issue/cyclic-imports-async-bundler.test.jsandtest/bake/dev-and-prod.test.ts: 3,178 pass, 2 fail. The 2 are inbundler_bytecode_portable.test.tsand fail the same way with the build of main. I cannot run Windows or macOS here.