Repository navigation
Conversation
When a non-entry module contains `export * from "<external>"` and the output format keeps ES6 imports, the linker rewrites the statement into `import * as ns from "<external>"` and emits `__reExport(exports, ns)` so the re-export happens at runtime. The printer decides whether to emit the `* as ns` clause based on the import record's CONTAINS_IMPORT_STAR flag, but the parser never sets that flag for a bare `export * from` record, so the printer produced a bare side-effect import instead and `ns` was left undefined. Set CONTAINS_IMPORT_STAR on the record in scanImportsAndExports alongside CALLS_RUNTIME_RE_EXPORT_FN for external records in ESM output, so the namespace import survives printing. Fixes #15997
Collaborator
Author
|
Status: reproduced, root-caused, fixed, and verified locally. Ready for review. Reproduced with Fix in CI (build 87154): the bundler change and new importstar tests pass on every lane. The reported failures are unrelated to this diff: EC2 agent provisioning for alpine 3.23, |
Contributor
|
Warning Review limit reached
Next review available in: 1 minute Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
Comment |
Collaborator
Author
|
Updated 4:19 AM PT - Aug 1st, 2026
⏳ @robobun, your commit f19ea13 is still building in
UnsupportedThe requested configuration is currently not supported. Please check the documentation for supported configurations.9c2710dd-ca53-4dba-92c2-3231c75c4a84
UnsupportedThe requested configuration is currently not supported. Please check the documentation for supported configurations.87b8fc06-c734-4333-bcc9-6b289f5b29d5 |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
LGTM — targeted one-flag fix so the synthesized import * as ns for a runtime __reExport actually prints its namespace binding.
What was reviewed:
- Traced
convertStmtsForChunk.rs:165-181andjs_printer/lib.rs:5691— the new gate (is_invalid() && keep_es6_import_export_syntax()) matches exactly where theS::Importis synthesized and where the printer requiresCONTAINS_IMPORT_STAR. - Checked other
CONTAINS_IMPORT_STARconsumers (Step 1 CJS-wrap, Step 6__toESMpath) — both are gated on a validsource_indexor on!keep_es6, so unaffected. - Two new
itBundledcases sit next toReExportStarExternalES6and cover the plain non-entry re-export plus the issue's exactnode:cryptoshape, with both a shape assertion and a runtime run.
Extended reasoning...
Overview
The PR fixes #15997: bundling a non-entry module that does export * from "<external>" under an ESM output format produced __reExport(exports, ns) referencing an unbound ns, because the printer only emits the * as ns clause when the import record carries CONTAINS_IMPORT_STAR, and the parser never sets that flag for a bare export-star record. The change is ~10 lines in src/bundler/linker_context/scanImportsAndExports.rs (Step 6 export-star loop): when marking the record with CALLS_RUNTIME_RE_EXPORT_FN, also set CONTAINS_IMPORT_STAR under rec_source_index.is_invalid() && output_format.keep_es6_import_export_syntax(). Two itBundled tests are added to test/bundler/esbuild/importstar.test.ts.
Security risks
None. This is bundler output-shape correctness; node:crypto in the test is only an example external module.
Level of scrutiny
Medium — bundler linker code is subtle, but the change is a single flag insert whose gate mirrors the exact condition in convertStmtsForChunk.rs where the S::Import { namespace_ref, star_name_loc } statement is synthesized (verified at lines 165-181), and the consumer in js_printer/lib.rs:5689-5701 confirms CONTAINS_IMPORT_STAR is what gates the * as clause. I audited the other readers of CONTAINS_IMPORT_STAR: the Step 1 CJS-wrap decision only runs when record.source_index.is_valid(), and the earlier Step 6 __toESM decision short-circuits on kind == Stmt && keep_es6_import_export_syntax() before reaching the flag check — so setting the flag here cannot leak into either. The CJS/require path (which emits __reExport(exports, require(path))) is untouched because the gate requires keep_es6_import_export_syntax().
Other factors
- The comment-cop bot flagged an over-long comment in an earlier commit; that thread is resolved and the current diff has a single-line comment.
- Tests are placed alongside the existing
ReExportStarExternalES6sibling and follow theitBundledconventions (issue URL comment, runtime files for the external, anonAfterBundleshape check plus arunstdout check so the assertion cannot be vacuous). - No CODEOWNERS entry for these paths. No prior human review comments outstanding.
Jarred-Sumner
pushed a commit
that referenced
this pull request
Sep 1, 2026
…n its assertions (#41028) ### Problem - `test/bundler/esbuild/importstar.test.ts` takes 11s on the debian 13 x64-asan lane (build 108747). The default `itBundled` backend is `api`, which the harness registers with `it.serial` (`expectBundled.ts:1962`), so its 75 cases run one at a time. 64 of them spawn a subprocess for `run`. - Three cases only check that the build succeeds. `ImportStarMangleNoBundleNoCapture` is a copy of the Capture case. ### Fix - Default the file to the CLI backend (the wrapper from `bundler_compile.test.ts`) and use `describe.concurrent`. Error and warning cases pass on it unchanged. - New checks: `ImportStarAndCommonJS` runs the bundle, no `__commonJS` wrapper. `ExportSelfIIFE` runs the bundle, no `__reExport` helper. `ImportExportOtherAsNamespaceCommonJS` requires the bundle and checks the JSON. `ImportStarNoCapture` and `ImportStarExportStarNoCapture`: no namespace object (`__export`). `ImportStarMangleNoBundleNoCapture` gets the upstream no-capture entry point. - Verified: `bun bd test test/bundler/esbuild/importstar.test.ts` (debug, ASAN): 35.3s and 35.4s before, 12.4s to 14.5s after (six runs). Released bun: 0.7s before, 0.25s after. 75 pass, 4 todo, every run. expect() calls: 64 before, 71 after. ### Background - The `api` backend calls `Bun.build()` in the test process with `process.chdir`, so the harness makes it serial. The `cli` backend spawns `bun build` in the case's own directory and can overlap. - `bun test` runs consecutive concurrent cases as one group of up to 20. A serial case ends the group, so the no-`run` cases also use `cli` (on `api` they measured 15.4s). - `__export` builds a namespace object. `__commonJS` wraps a CommonJS module. `__reExport` copies exports at runtime for `export *` of an external module. <details><summary>Notes</summary> What was not changed, and why: - `ImportStarExportStarAsNoCapture` and `ImportStarExportImportStarNoCapture` get no `__export` check. Bun still materializes the namespace for `ns.foo` through a re-export (esbuild binds `foo` directly). #41009 changes that and adds cases at the end of this file. This branch does not touch those lines. - `ImportDefaultNamespaceComboNoDefault1` to `4` stay separate builds. Bun reports only the first entry point's error, so one build with four entry points would check one error instead of four. - No case in the file sets `timeoutScale`. - No cases are grouped into shared builds. Each case has its own module graph with the same file names (`entry.js`, `foo.js`, `bar.js`). Other checks: - `bun bd test ... --todo`: the 4 todo cases still fail, as on main. - `BUN_BUNDLER_TEST_USE_ESBUILD=1`: main fails 32 cases because the API backend passes neither `format` nor `external` to `esbuild.build()`. This branch fails 2: `ImportStarCommonJSCapture` (esbuild adds `default` to the CommonJS namespace, noted in the case) and `ReExportStarEntryPointAndInnerFileExternal` (#37829). - Mutation check of the new output checks with the released bun: capturing `ns` in `ImportStarNoCapture` makes `__export` appear and the case fails. A CommonJS `foo.js` in `ImportStarAndCommonJS` makes `__commonJS` appear and the case fails. - Line 2 (the `itBundled` import) also changes in #36714 and #36134 (they add `expect` to line 1). The merge conflict is those two lines. - Debug ASAN runs after the change: 12.4s, 12.4s, 12.5s, 14.1s, 12.6s, 14.5s (runner's own `Ran 79 tests` line, host load average about 30). Released bun: 0.78s and 0.67s before, 0.25s and 0.24s after. </details> <!-- robobun:evidence:begin --> --- **[stamp-90s]** gate passed · iteration 0 · 1 files touched <details><summary>passes on PR (with fix)</summary> ```console Test-only change. Debug/ASAN (expected pass): $ bun bd test 'test/bundler/esbuild/importstar.test.ts' $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test test/bundler/esbuild/importstar.test.ts bun test v1.4.1 (a6c4cc2) test/bundler/esbuild/importstar.test.ts: (pass) bundler > importstar/ImportStarExportImportStarUnused [860.67ms] (pass) bundler > importstar/ImportStarCapture [933.82ms] (pass) bundler > importstar/ImportStarUnused [1362.73ms] (pass) bundler > importstar/ImportStarExportImportStarNoCapture [981.69ms] (pass) bundler > importstar/ImportStarNoCapture [1252.30ms] (pass) bundler > importstar/ImportStarExportImportStarCapture [737.57ms] (pass) bundler > importstar/ImportStarExportStarAsUnused [729.34ms] (pass) bundler > importstar/ImportStarExportStarAsNoCapture [754.71ms] (pass) bundler > importstar/ImportStarExportStarAsCapture [944.90ms] (pass) bundler > importstar/ImportStarExportStarUnused [767.40ms] (pass) bundler > importstar/ImportStarExportStarCapture [680.23ms] (pass) bundler > importstar/ImportStarCommonJSUnused [664.77ms] (pass) bundler > importstar/ImportStarExportStarNoCapture [791.44ms] (pass) bundler > importstar/ImportStarCommonJSNoCapture [701.31ms] (pass) bundler > importstar/ImportStarCommonJSCapture [782.90ms] (pass) bundler > importstar/ImportStarNoBundleUnused [414.74ms] (pass) bundler > importstar/ImportStarAndCommonJS [682.71ms] (pass) bundler > importstar/ImportStarNoBundleCapture [696.13ms] (pass) bundler > importstar/ImportStarNoBundleNoCapture [689.50ms] (pass) bundler > importstar/ImportStarMangleNoBundleUnused [726.10ms] (pass) bundler > importstar/ImportStarMangleNoBundleCapture [718.23ms] (pass) bundler > importstar/ImportStarMangleNoBundleNoCapture [640.67ms] (pass) bundler > importstar/ImportExportStarAmbiguousError [366.27ms] (pass) bundler > importstar/ImportStarExportStarOmitAmbiguous [723.51ms] (pass) bundler > importstar/ImportExportStarAmbiguous [604.14ms] (pass) bundler > importstar/ReExportStarNameCollisionNotAmbiguousImport [606.40ms] (pass) bundler > importstar/ReExportS ... (truncated) Exit: 0 ``` </details> <details><summary>diff hotspot</summary> ``` test/bundler/esbuild/importstar.test.ts | 44 ++++++++++++++++++++++++++++++--- 1 file changed, 40 insertions(+), 4 deletions(-) ``` </details> **gate history** · 1 passed · 0 rejected · iteration 0 <details><summary>evidence per changed file</summary> ``` file reads edits tests test/bundler/esbuild/importstar.test.ts 2 9 0 ``` </details> <!-- robobun:evidence:end -->
Jarred-Sumner
pushed a commit
that referenced
this pull request
Sep 2, 2026
…ped packages to export * (#41188) ### Problem - `bun build --production` of a React 19 app keeps `react-dom/index.js` and `react-dom/client.js` as `__commonJS` wrappers. Both are `checkDCE(); module.exports = require('./cjs/...production.js')` in production. #41162 handed this shape off. - The parser block for this shape in `src/js_parser/parse/parse_entry.rs` never fired. It matched an `ERequireString`, but `transpose_require` (`src/js_parser/p.rs:1570`) already makes the unwrapped `require()` the identifier of its `import * as ns` namespace. #35797 activated the block and hit the forced wrapper of a default import. ### Fix - The parser matches `module.exports = ns` (also behind the comma that minification makes of the folded `if`) and replaces it with `export * from` on the same import record. The file becomes ESM. A lifted CommonJS file has no export star of its own, so the linker knows the export star of a `COMMONJS_LIFTED_TO_ESM` file is this assignment. - When the re-export target has dynamic exports (CommonJS or external), named imports of a lifted file would bind to `undefined`. The linker then keeps today's `__commonJS` wrapper, and `convert_stmts_for_chunk` prints `module.exports = <value>` inside it instead of dropping the export star: the namespace object, the wrapper call, or the external import. - Verified: `test/bundler/bundler_cjs2esm.test.ts`, 5 of the new cases fail on the released bun. Also the suites in the notes. ### Background - The bundler converts CommonJS files of an allowlist of packages (react, react-dom, scheduler, ...) to ESM: `exports.foo = x` becomes a named export, `require("x")` becomes `import * as ns from "x"`. Such files carry `COMMONJS_LIFTED_TO_ESM` (#41162), and a default import of one binds to its namespace. - `export * from "./b"` resolves at link time. When "./b" has no static exports, the file gets a dynamic fallback object filled by `__reExport`. A lifted file does not use that fallback for unknown named imports (`DynamicFallbackInteropDefault`). - `convert_stmts_for_chunk` already prints lifted statements in CommonJS form when a file is wrapped after all (`var $foo = x` as `exports.foo = x`). The export star follows the same rule. <details><summary>Notes</summary> Numbers, `react@19.0.0` and `react-dom@19.0.0`, entry `import { createRoot } from "react-dom/client"; import { jsx } from "react/jsx-runtime"`, `--target=browser`, measured against a build of main at d6af50f: - `NODE_ENV=production --minify-syntax`: 2 `__commonJS` + 4 `__esm` wrappers and 2 `__toESM` calls, 472,842 bytes. After: none, 457,960 bytes. The entry reads `$createRoot` directly. - `--production`: 177,122 to 172,277 bytes. - With `import ReactDOM from "react-dom"` and `import * as NS from "react-dom"` added: still no wrapper, 463,291 bytes. Cases checked by hand against the released bun and main, all the same output or the lifted output: default, namespace and named imports; `require()` of the lifted file; `import()` of it (`default` is the namespace, as #41162 prints it); code splitting; `--minify`; `--format=cjs` and development mode (no lift, unchanged); a cycle between the lifted file and its target; the target being `module.exports = function` (wrapper kept, `module.exports = __toESM(require_x())`, identical to before); the target external (wrapper kept, `module.exports = ns`); `exports.foo` next to the assignment (wrapper kept); a chained assignment (wrapper kept); a runtime `if` (wrapper kept); the namespace also read as `ns.version` (import kept, export star added). The external target case prints `import * as ns` only because this branch sets `CONTAINS_IMPORT_STAR` on the record in step 6. A plain ESM `export * from "external"` in a non-entry file has the same missing binding today (#15997). #36714 fixes that; this PR does not touch it. Evaluation order is unchanged: the converted `require()` was already hoisted above the side effect as an import. `BabyVec::remove` is new. The parser needs an order-preserving removal of the dropped import part, since `swap_remove` would reorder the remaining imports. #12726 reports the unminified `react/index.js` redirect, which stays wrapped without `--minify-syntax`. #35611 handles that shape. Suites run with the debug build: `bundler_cjs2esm`, `bundler_cjs`, `bundler_edgecase`, `bundler_splitting`, `bundler_regressions`, `bundler_npm`, `bundler_minify`, `bundler_browser`, `bundler_bun`, `bundler_barrel`, `bundler_dynamic_import_dce`, `bundler_defer`, `bundler_loader`, `bundler_promiseall_deadcode`, `esbuild/default`, `esbuild/importstar`, `esbuild/importstar_ts`, `esbuild/dce`, `esbuild/packagejson`, `esbuild/splitting`, `regression/issue/cyclic-imports-async-bundler`. </details> <!-- robobun:evidence:begin --> --- **[human-review]** gate passed · iteration 1 · 5 files touched <details><summary>fails on main (without fix)</summary> ```console ASAN without fix: 5 FAILED $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" "test/bundler/bundler_cjs2esm.test.ts" bun test v1.4.1 (a6c4cc2) test/bundler/bundler_cjs2esm.test.ts: (pass) bundler > cjs2esm/ModuleExportsFunction [783.49ms] (pass) bundler > cjs2esm/ImportNamedFromExportStarCJSModuleRef [372.74ms] (pass) bundler > cjs2esm/ImportNamedFromExportStarCJS [438.17ms] (pass) bundler > cjs2esm/BadNamedImportNamedReExportedFromCommonJS [355.18ms] (pass) bundler > cjs2esm/ExportsFunction [434.25ms] (pass) bundler > cjs2esm/ModuleExportsFunctionTreeShaking [417.76ms] (pass) bundler > cjs2esm/ModuleExportsEqualsRequire [424.24ms] (pass) bundler > cjs2esm/ModuleExportsEqualsRequireEntryPoint [379.40ms] (pass) bundler > cjs2esm/ModuleExportsEqualsRequireEntryPointImportedByEntryPoint [461.02ms] (pass) bundler > cjs2esm/ModuleExportsEqualsRequireEntryPointImportedByEntryPointSplitting [455.98ms] (pass) bundler > cjs2esm/ModuleExportsEqualsRequireTwoEntryPoints [399.05ms] (pass) bundler > cjs2esm/ModuleExportsBasedOnNodeEnvProduction [568.86ms] (pass) bundler > cjs2esm/ModuleExportsBasedOnNodeEnvDevelopment [567 ... (truncated) release without fix: all passed bun test v1.4.1-canary.1 (b5d92ae) test/bundler/bundler_cjs2esm.test.ts: (pass) bundler > cjs2esm/ModuleExportsFunction [18.19ms] (pass) bundler > cjs2esm/ImportNamedFromExportStarCJSModuleRef [8.95ms] (pass) bundler > cjs2esm/ImportNamedFromExportStarCJS [8.53ms] (pass) bundler > cjs2esm/BadNamedImportNamedReExportedFromCommonJS [7.38ms] (pass) bundler > cjs2esm/ExportsFunction [9.69ms] (pass) bundler > cjs2esm/ModuleExportsFunctionTreeShaking [9.32ms] (pass) bundler > cjs2esm/ModuleExportsEqualsRequire [8.80ms] (pass) bundler > cjs2esm/ModuleExportsEqualsRequireEntryPoint [9.27ms] (pass) bundler > cjs2esm/ModuleExportsEqualsRequireEntryPointImportedByEntryPoint [10.60ms] (pass) bundler > cjs2esm/ModuleExportsEqualsRequireEntryPointImportedByEntryPointSplitting [9.94ms] (pass) bundler > cjs2esm/ModuleExportsEqualsRequireTwoEntryPoints [8.90ms] (pass) bundler > cjs2esm/ModuleExportsBasedOnNodeEnvProduction [10.51ms] (pass) bundler > cjs2esm/ModuleExportsBasedOnNodeEnvDevelopment [10.28ms] (pass) bundler > cjs2esm/ModuleExportsEqualsRuntimeCondition [9.21ms] (pass) bundler > cjs2esm/UnwrappedModuleRequireAssigned [9.75ms] (pass) bundler > cjs2esm/UnwrappedModuleRe ... (truncated) ``` </details> <details><summary>passes on PR (with fix)</summary> ```console ASAN with fix: all passed $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" "test/bundler/bundler_cjs2esm.test.ts" bun test v1.4.1 (a6c4cc2) test/bundler/bundler_cjs2esm.test.ts: (pass) bundler > cjs2esm/ModuleExportsFunction [972.77ms] (pass) bundler > cjs2esm/ImportNamedFromExportStarCJSModuleRef [430.35ms] (pass) bundler > cjs2esm/ImportNamedFromExportStarCJS [382.13ms] (pass) bundler > cjs2esm/BadNamedImportNamedReExportedFromCommonJS [434.30ms] (pass) bundler > cjs2esm/ExportsFunction [449.37ms] (pass) bundler > cjs2esm/ModuleExportsFunctionTreeShaking [398.66ms] (pass) bundler > cjs2esm/ModuleExportsEqualsRequire [395.49ms] (pass) bundler > cjs2esm/ModuleExportsEqualsRequireEntryPoint [434.22ms] (pass) bundler > cjs2esm/ModuleExportsEqualsRequireEntryPointImportedByEntryPoint [568.92ms] (pass) bundler > cjs2esm/ModuleExportsEqualsRequireEntryPointImportedByEntryPointSplitting [492.03ms] (pass) bundler > cjs2esm/ModuleExportsEqualsRequireTwoEntryPoints [440.74ms] (pass) bundler > cjs2esm/ModuleExportsBasedOnNodeEnvProduction [737.35ms] (pass) bundler > cjs2esm/ModuleExportsBasedOnNodeEnvDevelopment [664 ... (truncated) release with fix: all passed $ bun scripts/build.ts --profile=release [configured] bun-profile → bun (stripped) in 572ms (unchanged) ninja: Entering directory `/workspace/bun/build/release' [0/5] cargo bun_runtime → libbun_runtime.a �[1m�[92m Compiling�[0m bun_core v0.0.0 (/workspace/bun/src/bun_core) �[1m�[92m Compiling�[0m bun_alloc v0.0.0 (/workspace/bun/src/bun_alloc) �[1m�[92m Compiling�[0m bun_libdeflate_sys v0.0.0 (/workspace/bun/src/libdeflate_sys) �[1m�[92m Compiling�[0m bun_errno v0.0.0 (/workspace/bun/src/errno) �[1m�[92m Compiling�[0m bun_ptr v0.0.0 (/workspace/bun/src/ptr) �[1m�[92m Compiling�[0m bun_boringssl_sys v0.0.0 (/workspace/bun/src/boringssl_sys) �[1m�[92m Compiling�[0m bun_safety v0.0.0 (/workspace/bun/src/safety) �[1m�[92m Compiling�[0m bun_base64 v0.0.0 (/workspace/bun/src/base64) �[1m�[92m Compiling�[0m bun_cares_sys v0.0.0 (/workspace/bun/src/cares_sys) �[1m�[92m Compiling�[0m bun_zlib_sys v0.0.0 (/workspace/bun/src/zlib_sys) �[1m�[92m Compiling�[0m bun_zstd v0.0.0 (/workspace/bun/src/zstd) �[1m�[92m Compiling�[0m bun_picohttp v0.0.0 (/workspace/bun/src/picohttp) �[1m�[92m Compiling�[0m bun_brotli v0.0.0 (/workspace/bun/src/brotli) �[1m�[ ... (truncated) ``` </details> <details><summary>diff hotspot</summary> ``` src/bun_alloc/baby_vec.rs | 15 ++ src/bundler/linker_context/convertStmtsForChunk.rs | 97 +++++++- .../linker_context/scanImportsAndExports.rs | 47 +++- src/js_parser/parse/parse_entry.rs | 248 ++++++++++++--------- test/bundler/bundler_cjs2esm.test.ts | 212 ++++++++++++++++++ 5 files changed, 509 insertions(+), 110 deletions(-) ``` </details> **gate history** · 2 passed · 0 rejected · iteration 1 <details><summary>evidence per changed file</summary> ``` file reads edits tests src/bun_alloc/baby_vec.rs 2 2 15 src/bundler/linker_context/convertStmtsForChunk.rs 7 10 15 src/bundler/linker_context/scanImportsAndExports.rs 14 14 14 src/js_parser/parse/parse_entry.rs 13 15 14 test/bundler/bundler_cjs2esm.test.ts 5 5 14 ``` </details> <!-- robobun:evidence:end -->
Collaborator
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #15997.
Repro
The bundled output contains
__reExport(exports_http, node_crypto)followed by a bareimport "crypto";with nonode_cryptobinding. This is the shape universal-github-app-jwt@2.2.0 (via @octokit/auth-app / @octokit/app) ships.Cause
When a non-entry module has
export * from "<external>"and the output format keeps ES6 imports,convertStmtsForChunkrewrites the export-star intoimport * as ns from "<external>"and emits a__reExport(exports, ns)call that referencesns. The printer, however, emits the* as nsclause only when the import record carriesCONTAINS_IMPORT_STAR, and the parser never sets that flag for a bareexport * fromrecord. So the synthesized import prints as a side-effect-onlyimport "path"and thensreference in__reExportis unbound.The entrypoint case was already fine because
happens_at_runtimeis false there and theexport * fromis printed verbatim.Fix
In
scanImportsAndExports, when marking an external export-star record withCALLS_RUNTIME_RE_EXPORT_FNfor an ESM-output chunk, also setCONTAINS_IMPORT_STARso the printer emits the namespace binding. Gated onrec_source_index.is_invalid() && output_format.keep_es6_import_export_syntax()to match the exact condition under whichconvertStmtsForChunksynthesizes theimport * as nsstatement; the CJS path (which uses__reExport(exports, require(path))) is unaffected.Verification
Added two
itBundledcases intest/bundler/esbuild/importstar.test.ts(non-entryexport * fromexternal, and the issue's exact shape with an additional named import from the same external). Both fail on canary withReferenceError: node_crypto is not defined/ missingimport * as, and pass with this change. Existing importstar / default / edgecase / cjs / splitting bundler suites are green.[review] gate passed · iteration 0 · 2 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 1 passed · 0 rejected · iteration 0
evidence per changed file