Repository navigation
Conversation
`export * from "fs"` in a browser build names a disabled import record: no module is behind it. The linker handled it as a real external module. - In a module that is not an entry point, ESM output had `__reExport(exports_re, fs)` and never declared `fs`. - cjs and iife output re-exported the stub function itself, so the module got its `length` and `name` as exports. - A wrapped entry point kept `export * from "fs"` inside its wrapper, which does not parse. convert_stmts_for_chunk now removes the statement in these cases. Two cases stay as they are: a lifted `module.exports = require()`, and an unwrapped entry point of ESM output, where the statement prints as written.
…each condition
The condition now reads `CALLS_RUNTIME_RE_EXPORT_FN`, which
`scan_imports_and_exports` sets, and the wrap kind. It no longer
repeats the entry point and format test of that function.
New cases: a file whose names are only imported by name, a plugin module
with the path "fs", and an unwrapped entry point of ESM output. The
`module.exports = require("fs")` case now checks the printed
assignment.
|
Status: ready for review. CI build 135694 on b658c55: 183 jobs passed. The 5 failed jobs have one failing test, How I reproduced it echo 'export * from "fs"; export const b = 1;' > re.js
echo 'import * as r from "./re.js"; console.log(Object.keys(r));' > entry.js
bun build entry.js --target=browser --outfile=out.mjs
bun out.mjs
PR: #47449 |
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Filter export-star records and verify core cases src/bundler/linker_context/convertStmtsForChunk.rs, test/bundler/bundler_browser.test.ts |
The linker skips qualifying export-star records. Tests cover stubbed builtins and browser-disabled modules across output formats. |
Verify plugin and entry-point cases test/bundler/bundler_browser.test.ts |
Tests cover plugin resolution to fs, entry points with and without wrappers, and a dependency that assigns require("fs") to module.exports. |
Suggested reviewers: jarred-sumner, alii
Priority:
Merge Risk: 
b658c
-
The change removes qualifying disabled re-exports while preserving existing entry-point behavior. No concrete merge-blocking issue remains established; merge after normal checks pass.
Pre-merge checks: 
-
Check name Status Explanation Title check The title clearly and concisely describes the main linker change: removing export-star output for disabled imports. Description check The description explains the problem, fix, scope, trade-offs, and verification results. It does not use the template headings exactly, but it provides the required change summary and verification deta… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Description check
-
Explanation
The description explains the problem, fix, scope, trade-offs, and verification results. It does not use the template headings exactly, but it provides the required change summary and verification details.
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 @coderabbitai help to get the list of available commands.
Finishing Touches: 💡 1
🛠️ Fix failing CI checks: 💡
-
- Commit to this branch
- Create a new PR
|
Updated 4:07 AM PT - Oct 30th, 2026
❌ @robobun, your commit b658c55 has 1 failures in
🧪 To try this PR locally: bunx bun-pr 47449That installs a local version of the PR into your bun-47449 --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 findings, I also checked two things that turned out fine: every other place the bundler sets is_disabled on an import record outside the browser target comes with a resolve error (or HANDLES_IMPORT_ERRORS, which an export * record never carries), so the new guard in convert_stmts_for_chunk cannot drop an export * from a successful node/bun build; and a lifted-CommonJS module.exports = require("fs") is forced to a __commonJS wrap by has_dynamic_exports_due_to_export_star whenever it is not an unwrapped ESM entry, so the is_module_exports_of_wrapped_file exclusion covers that shape.
Extended reasoning...
The change is a 14-line early-continue in the bundler linker's export-star handling plus browser-target bundler tests; it touches no security-sensitive surface. Two inline findings (a pre-existing --splitting gap and a test nit) are already posted, and the note above records the sibling producers of is_disabled and the lifted-CommonJS wrap path that were traced and ruled out.
| if record.path.is_disabled | ||
| && !record.source_index.is_valid() | ||
| && !is_module_exports_of_wrapped_file | ||
| && (wrap != WrapKind::None | ||
| || record | ||
| .flags | ||
| .contains(ImportRecordFlags::CALLS_RUNTIME_RE_EXPORT_FN)) | ||
| { |
There was a problem hiding this comment.
🟣 pre-existing, not blocking: pre-existing: browser builds with --splitting still emit export * from "fs" verbatim in a lazily loaded chunk, so the import() of that chunk fails in a browser. The new condition at src/bundler/linker_context/convertStmtsForChunk.rs:177-180 needs wrap != None or CALLS_RUNTIME_RE_EXPORT_FN, and neither holds for an unwrapped ESM entry point, which a dynamic-import target becomes under splitting. Fix: drop a disabled, source-less export * in every ESM entry point too (user-specified and DynamicImport kinds), since no module is behind the record in any of them; the pin in browser/NodeBuiltinStubExportStarEntryPointESM should then assert no fs import remains. [also at: src/bundler/linker_context/convertStmtsForChunk.rs:180 - Pre-existing, left by this partial fix: a browser ESM bundle whose entry point has export * from "fs" still ships that statement verbatim, so the browser fails to resolve "fs" at load.]
A small fix can ride a push you are already making; otherwise a short reply is enough.
Why this was flagged
Trigger: bun build --target=browser --format=esm --splitting where a module reached only through import() contains export * from "fs" (or export * from "./x" with a browser map false entry). Under splitting that module gets EntryPoint::Kind::DynamicImport (src/bundler/LinkerGraph.rs:49), so scan_imports_and_exports leaves happens_at_runtime false (src/bundler/linker_context/scanImportsAndExports.rs:1351-1352) and never sets CALLS_RUNTIME_RE_EXPORT_FN; wrap is None. The new guard at convertStmtsForChunk.rs:174-181 therefore does not fire, the statement falls through to stmts.append at line 665, and the printer writes export * from "fs" as written (src/js_printer/lib.rs:5608-5618). In a browser the chunk fails to resolve the bare specifier and the import() rejects. The base branch prints the same verbatim statement here, so this is pre-existing. The new test browser/NodeBuiltinStubExportStarEntryPointESM (test/bundler/bundler_browser.test.ts) pins this output as expected.
Verification: pre-existing; acknowledged in diff. Under --splitting an import() target is an unwrapped ESM entry point, so CALLS_RUNTIME_RE_EXPORT_FN is never inserted and the guard at src/bundler/linker_context/convertStmtsForChunk.rs:174-183 does not fire; export * from "fs" is printed as written. The base branch takes the identical fall-through, so merging makes nothing worse.
There was a problem hiding this comment.
Confirmed. This PR leaves that output as it is on main, on purpose.
The mechanism is as you describe. scan_imports_and_exports plans no run-time re-export for an entry point of ESM output (src/bundler/linker_context/scanImportsAndExports.rs:1351), and with --splitting a module that import() loads is an entry point. So the flag is not set, the file is not wrapped, and the statement prints as written.
Why the change is not here: for a builtin, that output loads in a Node.js host today. An entry point with export * from "node:fs" re-exports the 104 names of the host module under Node.js 26.3.0, and none after the change. That changes a bundle that works, so a maintainer must decide it first. The description has the question and the facts under "Question for a maintainer": the AWS package under --splitting (ERR_MODULE_NOT_FOUND), what esbuild 0.21.5 prints, and the --external spellings.
If the answer is yes, the change is small: delete the last clause of the condition, and make browser/NodeBuiltinStubExportStarEntryPointESM expect no import. I then add --splitting cases for import() of a builtin and of a "browser" map false file. A release build with that clause deleted fails only that one case today.
The plugin case depends on the path map, which is keyed by path text alone. The entry point case pins output that the change leaves as it was. Each comment now says so.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Problem
__reExport(exports_re, fs)forexport * from "fs"in a non-entry module:ReferenceError: fs is not defined.@aws-sdk/credential-provider-web-identity3.664.0 throwsReferenceError: fromTokenFile is not defined.lengthandname. A wrapped entry point keeps the statement in its wrapper:SyntaxError.convert_stmts_for_chunk(src/bundler/linker_context/convertStmtsForChunk.rs:184) handles a disabled import as an external module.Fix
convert_stmts_for_chunkremoves the statement of a disabled record with no source index, where it would become__reExportor sit in a wrapper.undefined.test/bundler/bundler_browser.test.ts, 13 new cases. 8 fail on main'ssrc/. The other 5 pin the conditions.;between the twovars of a disabledimport d, * as ns#47435, 8 in Downsides.Background
"browser"mapfalseentry. The printer prints the stub(() => ({})).__reExport(target, mod)copies the property names ofmodtotargetat run time, for anexport *target outside the bundle.ReferenceErrors. See Notes.Downsides
export * from "fs"no longer exportslengthandname.__reExporthelper stays in the bundle (775 B in a one-module probe)..text: +112 B.Notes
No linked issue.
What each change does. Release builds (
--lto=off) of main 9fe1cd6, of main with the source change of #41220, of this PR, and of both. Browser target, output run with Node.js 26.3.0.export * from "fs"in a module that is imported staticallyReferenceError: fs is not defined["b"]["b"]["b"]require()orimport()["c","length","name"]["c"]["c"]["b","length","name"]["b"]["b"]export * from "node:fs"; export * from "pkg", andpkgexportsname: "pkg"nameis"""pkg""pkg"SyntaxError--external '@aws-sdk/client-sts'ReferenceError: fromTokenFile is not definedvar fs = (() => ({}))has its value. In a wrapped module thevarruns first, and the stub's own names become exports.git merge-treegives no conflict. With both, this file has 36 pass and the two new cases of bundler: name the namespace of an external export * in a non-entry module #41220 pass.Question for a maintainer. Should this statement also go away where main prints it as written? This PR does not change that output. It is an unwrapped entry point of ESM output. With
--splitting, a module thatimport()loads is one too.export * from "node:fs"there re-exports the 104 names of the host module under Node.js. A browser cannot load the file.export * from "./from-file"of a"browser"mapfalsefile is printed with that path. The AWS package, loaded withimport()under--splitting, fails withERR_MODULE_NOT_FOUND.export *for the"browser"map case. For the builtin it stops withCould not resolve "fs".--external fsdoes not keep the import (Flagging Node Dependencies with--externaldoes not work #2701, resolver: check user externals before browser node builtin polyfills #35212).--external 'fs*'does.SyntaxErroron main. This PR removes the statement there.Not changed here. Each row gives the same output on main and on this PR.
export *in a"sideEffects": falsepackageReferenceError: exports_barrel is not definedexport *of a real external in a non-entry module, ESM outputReferenceErrorexport *of a real external in a wrapped entry pointSyntaxError: Unexpected token 'export'"browser"map entry set tofalseconvert_stmts_for_chunk_for_dev_server.--external fsimport("fs")of a stubbed builtinPromise.resolve(<stub>)and brokeawait import("node:fs")in a default-target bundle under Node.js.module.exportsis a function, throughexport *length,nameandprototypeare exports.;ofimport d, * as nsOther readers of a disabled record are not changed: an unused
import * as fs from "fs"still printsvar fs = (() => ({}));, and the metafile lists the import with"external": true. Related: #46548 builds the page of a server build with the options of the browser target.The condition.
record.path.is_disabled && !record.source_index.is_valid(): a plugin module with the pathfsgets linked to a later import of"fs". That record is disabled and has a module behind it, so itsexport *stays. 12 of 12 bundles of that state are byte-identical to main.!is_module_exports_of_wrapped_file:module.exports = require("fs")in a file of an unwrapped package (react-dom18) is anexport *inside the bundler. The wrapper still assigns the stub.wrap != WrapKind::None || CALLS_RUNTIME_RE_EXPORT_FN:scan_imports_and_exportssets the flag where it plans the__reExportcall. The condition reads that plan and does not repeat the entry point test.Five release builds, each with one clause deleted, and the case that then fails:
source_indextest: the 3PluginModuleWithBuiltinPathExportStarcasesis_module_exports_of_wrapped_filetest:NodeBuiltinStubModuleExportsRequireNodeBuiltinStubExportStarEntryPointESMwraptest:NodeBuiltinStubExportStarWrappedEntryPointesbuild/importstar.test.tshas 99 pass on each of the five.scan_imports_and_exportsis not changed. A guard there keeps the helper out of the bundle, but it rewrites the lines that bundler: name the namespace of an external export * in a non-entry module #41220 changes.Measurements. Release builds with
--lto=offof main 9fe1cd6 and of main with this change, in one build directory. ThinLTO was not measured..text: 103,985,936 to 103,986,048 B (+112 B,size -A).convert_stmts_for_chunk: 6,350 to 6,455 B (nm -S).scan_imports_and_exports: 56,344 B on both. The size of thebunfile does not change.export * fromstatement that the linker converts: 2 hits for a 9-module bundle with two such statements (gdb breakpoint count, debug build). A record that is not disabled leaves after one byte test. No allocation.--minify, with and without--splitting, and--no-bundle. Thelibraries.jscorpus ofbundler_bytecode_portable.test.tshas the same sha256 on both builds.export *output. The outputs ofimport(), ofimport d, * as nsand of an ESM entry point withexport *are byte-identical.scan_imports_and_exportsit is 507 B. Minified: 595, 588 and 235 B.Tests.
bundler_browser.test.ts36 pass, 0 fail. Also 0 fail and 838 pass inesbuild/importstar,esbuild/importstar_ts,esbuild/default,esbuild/dce,esbuild/splitting,bundler_barrel,bundler_cjs2esm,bundler_edgecaseandbundler_plugin.test/bundler(161 files, 10,171 tests). Main: 10 failures. This PR: the 8 new cases pass, and 2 failures stay. Both fail on main too when each file runs alone:--cpu-prof attributes native time to the calling function, andencoder output is identical on every platform, whose snapshot does not match in my container (both builds print the same hashes). In the full run of this PR, a third test of that file failed after a 5 s time limit killed its shared child.test/bundlerdid not run on a debug build.src/with this test file: the 8 cases fail (release build of 9fe1cd6).[human-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