Repository navigation
bundler: name the namespace of an external export * in a non-entry module - #41220
Jarred-Sumner wants to merge 1 commit into
Conversation
…module `export * from "path"` in a module that is not an entry point becomes `import * as ns from "path"` and `__reExport(exports_x, ns)`. The printer prints `* as ns` only for a record with `CONTAINS_IMPORT_STAR`, and that flag was missing. So the output had a bare `import "path"`, and `__reExport` threw `ReferenceError: path is not defined`. This happened for `import * as ns`, `import()`, and `require()` of such a module, for every external, in every release back to at least 1.3.5. Set the flag where the linker decides the export star runs at run time. The lifted CommonJS path already does this.
|
Updated 7:15 PM PT - Sep 2nd, 2026
⏳ @Jarred-Sumner, your commit df8b062 is still building in
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Essentials Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review. WalkthroughThe linker marks external runtime re-exports as import-star records when ES module syntax is preserved. Bundler tests cover external namespace re-exports, lazy-loaded Node built-in re-exports, and updated bytecode output hashes. ChangesES module re-exports
Suggested reviewers: Merge Risk: ⚪ Minimal · up to The bundler fix preserves the external module namespace needed for runtime export-star re-exports, preventing failures when bundled modules are accessed through namespace imports, dynamic imports, or require. The accompanying coverage and expected bytecode-output update leave no current merge-blocking risk. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| "js": "3cba87d68bb02922e826131fd75b6c1241cc9df5388036e55f26549ac40accd1", | ||
| "jsc": { | ||
| "bytes": 23789264, | ||
| "sha256": "27f1a233c4d2ef984ff6ac2ce67e49328841d371b58027178b0a3d7665d21c03", | ||
| "sha256": "eae4e0752da8ddbfbbdd20089df4b04274fd7063e8163b195b0de9e0623c9ce8", |
There was a problem hiding this comment.
🔴 These snapshot hashes should not have changed: the linker fix only inserts CONTAINS_IMPORT_STAR when output_format.keep_es6_import_export_syntax() (ESM only), but this test's bundle() runs bun build --bytecode --target=bun with no --format, which forces CJS (Arguments.rs:2068; ESM bytecode requires --compile). The CJS output is byte-identical to the base, so this test now fails on every platform against the un-touched base hashes. Fix: revert the libraries.js js and jsc.sha256 edits back to the base values — this PR does not affect that build.
Extended reasoning...
keep_es6_import_export_syntax() returns self == Format::Esm (src/options_types/bundle_enums.rs:41-43). The only functional change in scanImportsAndExports.rs is guarded by that predicate, so for Format::Cjs the emitted import records — and therefore the printed JS and its bytecode — are unchanged from base fd10aef. bundle() at bundler_bytecode_portable.test.ts:350 spawns bun build --bytecode --target=bun --outdir … with no --compile and no --format; Arguments.rs:2067-2070 sets output_format = Format::Cjs for --bytecode, and Arguments.rs:2540 rejects --format=esm without --compile. So the libraries.js build still produces js sha 046e3e45… and jsc sha 27f1a233…, but the inline snapshot now expects 3cba87d6… / eae4e075…. The toMatchInlineSnapshot at line 466 fails on every CI lane. The unchanged bytes: 23789264 alongside two changed sha256 values is the tell that these were not regenerated by --update-snapshots on the current base (the PR description confirms the suite was 'not run after the rebase'). Correct action: drop the two hash edits; nothing…
Verification: normal — acknowledged in diff: the PR description says "Not run after the rebase onto #41213 … will probably need its libraries.js hash regenerated"; the code shows that guess is wrong. The only functional source change is guarded by output_format.keep_es6_import_export_syntax() (src/bundler/linker_context/scanImportsAndExports.rs:1301-1305), which returns self == Format::Esm… | normal —…
df8b062 to
d207238
Compare
|
This PR also fixes #15997. The body can say "Fixes #15997". #36714 makes the same change in The tests in this PR do not have the exact shape of #15997. In that shape, one file has I ran the case against this branch. It fails on the released bun 1.4.1, where the output has a bare // https://github.com/oven-sh/bun/issues/15997
itBundled("importstar/ReExportStarExternalNonEntryWithNamedImportES6", {
files: {
"/entry.js": /* js */ `
import { createHash, convertPrivateKey } from "./crypto-node.js";
console.log(typeof createHash, typeof convertPrivateKey);
`,
"/crypto-node.js": /* js */ `
export * from "node:crypto";
import { createPrivateKey } from "node:crypto";
export function convertPrivateKey(pem) { return createPrivateKey(pem); }
`,
},
target: "bun",
format: "esm",
onAfterBundle(api) {
api.expectFile("/out.js").toMatch(/import \* as \w+ from "(node:)?crypto"/);
},
run: {
stdout: "function function",
},
}); |
|
Two notes from #46786, which carries this commit as its first commit:
|
|
#46903 (the fix for #18008) needs this change first. It keeps a Two kinds of target reach it, both with ES module output:
I applied the source change of this PR on top of #46903. Then each of these shapes loads and prints the right values. The table is under "Merge order" in #46903. |
|
#47449 is open for the second kind of target in the comment above: a target that a browser build disables. It removes the statement in I built main 9fe1cd6 with the source change of this PR alone, with #47449 alone, and with both (release builds,
With both changes, the 13 new cases of #47449 pass and the two new cases of this PR pass. |
With ESM output, a module that is not an entry point and does `export * from "<external>"` printed `__reExport(exports_x, ns)` followed by a bare `import "<external>"`, so loading it threw `ReferenceError: ns is not defined`. `convert_stmts_for_chunk` rewrites the statement to `import * as ns`, but the printer prints `* as ns` only for a record with CONTAINS_IMPORT_STAR, and the linker never set it on this path. Set it where the linker decides the export star runs at run time, as the lifted CommonJS path already does. Same source change as oven-sh#41220. Adds tests for a named import of such a module, a namespace import of it, and a split chunk that runs it on load. Fixes oven-sh#15997 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…JS wrapper A file that assigns `exports` and has `export * from "pkx"` gets a CommonJS wrapper when another file requires it. Its `exports` is an object of the wrapper, so no link-time bind reaches it. In ES module output the statement of such a file, when it is not an entry point, runs in the wrapper as it does on main: `__reExport(exports, pkx)`. The record of the statement gets `CONTAINS_IMPORT_STAR`, so the namespace that the call reads has an import (the source change of #41220). `external_star_member` sets the namespace alias through the safe symbol table API. The SAFETY comment of `match_imports_with_exports_for_file` lists what its loop mutates now. Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>
|
A question that decides how #47685 lands, because the two PRs overlap. #47685 binds a name that is read through an external Should a file that is not an entry point keep the run-time route of this PR, with the bind for entry points only?
This PR alone fixes both repros of #15997. The Notes of #47685 have what each route costs. |
What does this PR do?
Take a module that isn't an entry point and does
export * from "path", or from any external. Bundling it as ESM produced a bareimport "path";, so__reExport(exports_x, path)threwReferenceError: path is not defined. This fails forimport * as ns,import(), andrequire()of such a module, on--target=bunand--target=node. It goes back to at least 1.3.5.convert_stmts_for_chunkturns the statement intoimport * as ns. But the printer prints* as nsonly for a record withCONTAINS_IMPORT_STAR, and this path never set that flag. The lifted CommonJS path already sets it. This PR sets it where the linker decides the export star runs at run time.How did you verify your code works?
test/bundler/esbuild/importstar.test.ts. They fail on the released build and pass with this change.importstar,default,dce,splitting,cjs2esm, andedgecasesuites pass locally, on the base before the rebase.libraries.jsbytecode hash does not change. CI's install of the corpus packages has no externalexport *of this shape.