Carry import attributes through the parser, printer, bundler and runtime; add the bytes loader - #40836
Open
robobun wants to merge 9 commits into
Open
Carry import attributes through the parser, printer, bundler and runtime; add the bytes loader#40836robobun wants to merge 9 commits into
robobun wants to merge 9 commits into
Conversation
The parser keeps the whole `with { ... }` clause on the import record
instead of reducing it to a loader. The printer prints it as written on
every import, `export * from` and `export {} from` that stays in the
output, for every target. The bundler keys the module graph on
(path, attributes), so one file imported with two `type` values is two
modules, and reports a `type` that names no loader. The runtime reports
the same case with ERR_IMPORT_ATTRIBUTE_UNSUPPORTED.
`with` is a keyword, so a newline before it is allowed; `assert` keeps
the no-newline rule. A duplicate attribute key is a syntax error.
Adds the `bytes` loader: `with { type: "bytes" }` gives the file's bytes
as a Uint8Array. The bundler inlines them as base64 decoded by the new
`__toBytes` runtime helper; a standalone executable embeds the raw bytes.
Contributor
|
Warning Review limit reached
On-demand reviews are free for the next 23 days. After that, they cost $0.25 per reviewed file. Or wait 13 seconds for your next included review. View limit detailsLimit details: You’ve used all 5 included reviews currently available. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (43)
Comment |
Collaborator
Author
require.extensions wrappers call Bun__transpileFile with an empty string for the type attribute, which the new unknown-type check rejected.
![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.
Additional findings (outside the current diff — GitHub can't attach inline comments there):
-
🟣
src/bundler/bundle_v2.rs— PluginonResolvesuccess path still keyspath_to_source_index_maponpath.textalone, ignoring import attributes (pre-existing — partial fix)Extended reasoning...
The PR's stated fix is that the bundler now keys the module graph on
(path, attributes)viaImportRecord::module_graph_key, soimport a from './x.json' with { type: 'text' }andimport b from './x.json' with { type: 'json' }become distinct modules. Every direct-resolve/file-map path inresolve_import_recordsandenqueue_on_resolve_pluginswas updated to computemodule_keywithkey_attributes. However, when a JS plugin'sonResolvehook returns aSuccessresult, the handler at ~line 4929 still doespath_to_source_index_map(...).get_or_put(path.text)with the raw resolved path and no attributes —MiniImportRecord(line 1062) does not carryattributesat all, so the callback has nothing to key on. Two imports of the same file with differentwith { type: ... }attributes that both flow through a matchingonResolveplugin therefore collapse to a single source index and the first import's loader wins — the exact bug the PR fixes on the non-plugin path. Relative to base this path behaves the same as before, so it is a leftover site of the class the PR is…Verification: pre-existing — partial fix. At src/bundler/bundle_v2.rs:4926-4930 the plugin
onResolveSuccess handler still keys the module map on the raw path:let existing = this .path_to_source_index_map(resolve.import_record.original_target) .get_or_put(path.text) .expect("oom");Every sibling insert/lookup site in this file was migrated to `ImportRecord::module_graph_key(path.text,… -
🟣
src/bundler/bundle_v2.rs— Plugin fallback path (run_resolver) skips the new unknown-typecheck, sowith { type: "nope" }silently falls back to the extension loader when any onResolve filter matched (pre-existing — partial fix)Extended reasoning...
The PR adds
has_unsupported_type_attributeand calls it fromresolve_import_recordsat lines 6434 and 6785 so that a bundled import with an unrecognisedtypeattribute fails the build. But an import whose specifier matches a registeredonResolvefilter takes the plugin dispatch at line 6363 (enqueue_on_resolve_plugin_if_neededreturns true →continue), never reaching either check. When the plugin's callback returnsundefined, theNoMatchbranch (line 4850) callsrun_resolver, which resolves the file and picks a loader at lines 2639-2651:record.loaderisNonefor an unknowntype(parse_path only sets it whenLoader::from_stringmatched), so it falls through topath.loader(&transpiler.options.loaders).unwrap_or(Loader::File). Concretely, with a build that registersonResolve({ filter: /\.json$/ }, () => undefined)and a source containingimport x from "./data.json" with { type: "nope" }, the bundle succeeds and loadsdata.jsonwith the JSON loader — whereas the identical build without the plugin now errors with `Importing with a type attribute of…Verification: pre-existing — partial fix. The new
has_unsupported_type_attributeguard (bundle_v2.rs:6171-6196) is invoked only fromresolve_import_recordsat :6434 and :6785. Both call sites are downstream of the plugin dispatch at :6363-6371: whenenqueue_on_resolve_plugin_if_neededfindsplugins.has_any_matches(...)(:5814) it dispatches and returnstrue, and the loopcontinues before either…
…ead decoder globals off globalThis The onResolve success path keyed the module map on the path alone and took the loader from the extension, and the no-match fallback skipped the unknown-type check. The runtime now exempts type: "macro" like the bundler. __toBytes reads Buffer and Uint8Array off globalThis so they do not become reserved names that rename user code shadowing them.
…) and require() with an unknown type, and the dev server re-export
Collaborator
Author
|
Dedupe pass against the four older import attribute PRs, run on a debug build of this branch (head Coverage of the older PRs' test additions:
Commit |
This was referenced Aug 28, 2026
…he external check on the plugin path
Collaborator
Author
|
CI on cb28fb7: every lane passes except darwin x64, where |
This was referenced Sep 1, 2026
Jarred-Sumner
pushed a commit
that referenced
this pull request
Sep 14, 2026
… under --isolate (#42679) Fixes #33904. Re-lands #33905. Same printer change as @alii's 0de84ef on #35605, carried alone to land first. ### Problem - A runtime plugin resolves `./data.bar?custom` into namespace `custom`. `bun test --isolate` and `--parallel` never run the namespaced `onLoad`: `Expected: "FROM_PLUGIN" Received: "<abs>/data.bar"`. Debug builds stop with `error: Imports different between parseFromSourceCode and fallbackParse`. - In `src/js_printer/lib.rs`, `print_import_record_path` prints that record as `"custom:/abs/data.bar"`, but the three `ModuleInfo` recording sites (`import`, `export {} from`, `export * from`) interned only `path.text`. The module record asked for `/abs/data.bar`. ### Fix - `printed_namespace()` decides whether a record prints as `namespace:path`. `print_import_record_path` and the new `ModuleInfo::str_for_import_record` both call it. The three recording sites use the latter. - The transpiler cache version goes from 32 to 33. The on-disk cache stores the same record, so an older entry brings the bug back. - Verified: three new cases in `test/cli/test/isolation.test.ts` (`--isolate`, `--parallel`, on-disk cache) fail on 1.4.3-canary. Also ran `test/js/bun/plugin/`. - Self-reviewed: 4 concerns raised, 3 addressed. Not done: keep plugin-rewritten files out of the on-disk cache, an older separate bug (Notes). ### Background - `--isolate` gives each test file a fresh global. Bun builds the JSC module record from `ModuleInfo`, a table the printer fills, with no second parse. - The runtime linker (`src/bundler/linker.rs:462`) runs plugin `onResolve` before the print, stores the answer as `path.text` plus `path.namespace`, and sets `PRINT_NAMESPACE_IN_PATH`. - A source specifier `virt:thing` breaks the same way when namespace `virt` has an `onResolve`. With only an `onLoad` it works. <details><summary>Notes</summary> **Related PRs** - #33905 was the first version of this fix. A stale-PR cleanup closed it on 2026-09-13 with no review verdict. - #35605 (open, conflicts with main) attaches `ModuleInfo` to every ESM transpile, not only under `--isolate`. With it, this bug reaches plain `bun run`. Its commit 0de84ef makes the same change to the printer (one predicate, the same three sites). This PR does not depend on #35605 and does not change when `ModuleInfo` is built. Whichever lands second keeps one helper. - #40836 (open) rewrites the same three recording sites for import attributes and still interns `path.text` there. It has to call `str_for_import_record` after a rebase. **Which plugins were affected** - The trigger is the linker rewrite, not the `?query`. The record breaks when the namespaced module cannot be found again from the bare `path.text`. - `./data.bar?custom` with an `onResolve` that strips the query and sets a namespace: broken. This is the issue's case. - `virt:thing` in source with an `onResolve` for namespace `virt`: broken (`Cannot find package 'resolved-other'` in the new test). This is the esbuild-style virtual module pattern. - `./data.bar` with `onResolve({ filter: /\.bar$/ })` into a namespace: worked by accident. The runtime resolve of the bare path matched the same `onResolve` again. - A namespaced specifier with only an `onLoad`: worked. The linker does not rewrite the record, so `path.text` keeps the prefix. - `onResolve` with no namespace (a plain redirect): worked, and still takes the `None` arm of `printed_namespace`. The new test keeps one such import. **Scope of the change** - `request_module` and `request_module_with_phase` have three callers in the printer. All three are changed. The `require()` and `import()` printers also call `print_import_record_path`, but they add nothing to the module record. - Only `src/bundler/linker.rs:472` sets `PRINT_NAMESPACE_IN_PATH`, and only the runtime transpile path (`src/runtime/jsc_hooks.rs`) reaches it with a `ModuleInfo`. The bundler and `bun build --compile` never set the flag. **On-disk transpiler cache** - Stale entry check: ran the issue's repro with a test file above the 4 KiB cache floor and `BUN_RUNTIME_TRANSPILER_CACHE_PATH` set. The `.pile` entry from 1.4.3-canary holds `custom:/abs/data.bar` in the output and `/abs/data.bar` in the ESM record, and the second run fails from the cache. The cache file name is the input hash only, so a fixed build reads that entry unless the version changes. Version 22 was bumped for the same reason. - The third new test runs the fixture cold and warm against a private cache directory, with one module padded past 4 KiB. It covers the record round trip through the cache entry. - Not changed here: the on-disk cache stores output that holds a plugin's link-time `onResolve` answer, under a key made from the importing file's content. If the plugin later answers differently, the old answer is served until the importing file changes. This is independent of `--isolate` and older than this bug. One line after the rewrite in `src/bundler/linker.rs` (`result.runtime_transpiler_cache = None`) would stop it. `parse_entry.rs` already drops the cache in a like case, when it rewrites `@jest/globals` to `bun:test`. The cost is a transpile on every run for each file that a resolve plugin touches, for example every importer of a path alias. That trade needs a maintainer's call, so it is left for a separate PR. **Self-review** 1. The PR body did not name #35605 and #40836, and said a source-written `custom:foo` was never affected. Fixed above. 2. No test for a namespace that is already in the source. Added the `virt:` imports. 3. The cache version bump was checked by hand only. Added the cold and warm cache test. 4. Keep plugin-rewritten output out of the on-disk cache. Not done, see above. **Tests** - The new cases fail on the unfixed release build with `Cannot find package 'resolved-other'` (first error) and `SyntaxError: Export named 'named' not found in module '<dir>/reexport-clause.ts'`. The same fixture passes there without `--isolate`. - Suites run with the debug build: `test/cli/test/isolation.test.ts` (39 pass), `test/js/bun/plugin/` (48 pass), `test/cli/run/transpiler-cache.test.ts`, `test/regression/issue/30887.test.ts`, `test/js/bun/typescript/type-export.test.ts`, `test/cli/inspect/debugger-buntranspiledmodule.test.ts`. Two cases in the last four (`--drop invalidates cache`, one `--compile` case) hit the 5 s default timeout on the local debug build and pass with a longer timeout. </details> <!-- robobun:evidence:begin --> --- **[human-review]** gate passed · iteration 0 · 3 files touched <details><summary>fails on main (without fix)</summary> ```console ASAN without fix: 3 FAILED $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/cli/test/isolation.test.ts bun test v1.4.3 (b993710) test/cli/test/isolation.test.ts: (pass) bun test --isolate > without --isolate, leaked global is visible to next file [430.63ms] (pass) bun test --isolate > with --isolate, each file gets a fresh global [483.42ms] (pass) bun test --isolate > without --isolate, --preload still runs once (regression) [282.91ms] (pass) bun test --isolate > with --isolate, --preload re-runs in each file's fresh global [448.63ms] (pass) bun test --isolate > with --isolate, module state is not shared between files [379.00ms] (pass) bun test --isolate > with --isolate, a file's process.chdir() is undone before the next file [1257.31ms] (pass) bun test --isolate > with --isolate, a file's process.env writes with native side effects are undone before the next file [1568.05ms] 439 | using dir = tempDir("isolate-plugin-namespace", pluginNamespaceFixture); 440 | const { stderr, exitCode } = await runTests(String(dir), args, ["./a.test.ts", "./b.test.ts"], { 441 | ...bunEnv, 442 | ...en ... (truncated) release without fix: 3 FAILED bun test v1.4.3-canary.1 (b993710) test/cli/test/isolation.test.ts: 439 | using dir = tempDir("isolate-plugin-namespace", pluginNamespaceFixture); 440 | const { stderr, exitCode } = await runTests(String(dir), args, ["./a.test.ts", "./b.test.ts"], { 441 | ...bunEnv, 442 | ...env, 443 | }); 444 | expect(normalizeBunSnapshot(stderr, dir)).toContain("2 pass"); ^ error: expect(received).toContain(expected) Expected to contain: "2 pass" Received: "a.test.ts:\n\n# Unhandled error between tests\n-------------------------------\nerror: Cannot find package 'resolved-other' from '<dir>/a.test.ts'\n-------------------------------\n\n\nb.test.ts:\n\n# Unhandled error between tests\n-------------------------------\nerror: Cannot find package 'resolved-other' from '<dir>/b.test.ts'\n-------------------------------\n\n\n 0 pass\n 2 fail\n 2 errors\nRan 2 tests across 2 files." at <anonymous> (/workspace/bun/test/cli/test/isolation.test.ts:444:47) (pass) bun test --isolate > without --isolate, leaked global is visible to next file [26.77ms] (pass) bun test --isolate > with --isolate, --preloa ... (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/cli/test/isolation.test.ts bun test v1.4.3 (b993710) test/cli/test/isolation.test.ts: (pass) bun test --isolate > with --isolate, each file gets a fresh global [352.05ms] (pass) bun test --isolate > without --isolate, leaked global is visible to next file [439.29ms] (pass) bun test --isolate > without --isolate, --preload still runs once (regression) [297.02ms] (pass) bun test --isolate > with --isolate, --preload re-runs in each file's fresh global [336.20ms] (pass) bun test --isolate > with --isolate, module state is not shared between files [371.07ms] (pass) bun test --isolate > with --isolate, a file's process.env writes with native side effects are undone before the next file [1392.12ms] (pass) bun test --isolate > with --isolate, a file's process.chdir() is undone before the next file [1461.43ms] (pass) bun test --isolate > cached module records keep the namespace a plugin onResolve gives an import (--isolate) [542.79ms] (pass) bun test --isolate > with --isolate, cached module records keep short, Latin-1 and UTF-16 names ... (truncated) release with fix: all passed $ bun scripts/build.ts --profile=release [configured] bun-profile → bun (stripped) target linux-x64-gnu build type Release build dir ./build/release revision 68e0e0b features baseline 23 deps, 131 codegen, 1176 objects in 689ms ninja: Entering directory `/workspace/bun/build/release' [1/1248] install /workspace/bun bun install v1.4.3-canary.1 (b993710) Checked 22 installs across 61 packages (no changes) [7.00ms] [2/1248] gen ErrorCode+*.h [3/1248] install /workspace/bun/packages/bun-error bun install v1.4.3-canary.1 (b993710) Checked 1 install across 2 packages (no changes) [1.00ms] [4/1248] gen bindgenv2 [5/1248] install /workspace/bun/src/node-fallbacks bun install v1.4.3-canary.1 (b993710) Checked 111 installs across 104 packages (no changes) [4.00ms] [6/1248] gen node-fallbacks/react-refresh.js Bundled 1 module in 4ms react-refresh.js 4.81 KB (entry point) [7/1248] fetch zlib [zlib] up to date [8/1248] fetch tinycc [tinycc] up to date [9/1247] fetch libjpeg-turbo [libjpeg-turbo] up to date [10/1247] gen bake.{client,server,error}.js -> bake.client.js, bake.server.js, bake.error.js [11/1247] gen .bind.ts → Gene ... (truncated) ``` </details> <details><summary>diff hotspot</summary> ``` src/js_printer/lib.rs | 51 +++++++++-------- src/jsc/RuntimeTranspilerCache.rs | 5 +- test/cli/test/isolation.test.ts | 116 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 149 insertions(+), 23 deletions(-) ``` </details> **gate history** · 1 passed · 0 rejected · iteration 0 <details><summary>evidence per changed file</summary> ``` file reads edits tests src/js_printer/lib.rs 6 5 23 src/jsc/RuntimeTranspilerCache.rs 2 1 23 test/cli/test/isolation.test.ts 4 3 23 ``` </details> <!-- robobun:evidence:end --> Co-authored-by: Alistair Smith <hi@alistair.sh>
|
Production use case for the bytes loader. We run a nitro server bundle on Vercel's Bun runtime and bun test on the same modules. The parsers load web-tree-sitter and two So we generate a 435 KB module of base64 strings and commit it, with a regen hook and a drift test. This PR removes all of that. #41834 now carries the module-map part alone. Could the bytes loader and the unknown-type error land the same way, on top of |
usrbinkat
pushed a commit
to usrbinkat/bun
that referenced
this pull request
Sep 15, 2026
… under --isolate (oven-sh#42679) Fixes oven-sh#33904. Re-lands oven-sh#33905. Same printer change as @alii's 0de84ef on oven-sh#35605, carried alone to land first. ### Problem - A runtime plugin resolves `./data.bar?custom` into namespace `custom`. `bun test --isolate` and `--parallel` never run the namespaced `onLoad`: `Expected: "FROM_PLUGIN" Received: "<abs>/data.bar"`. Debug builds stop with `error: Imports different between parseFromSourceCode and fallbackParse`. - In `src/js_printer/lib.rs`, `print_import_record_path` prints that record as `"custom:/abs/data.bar"`, but the three `ModuleInfo` recording sites (`import`, `export {} from`, `export * from`) interned only `path.text`. The module record asked for `/abs/data.bar`. ### Fix - `printed_namespace()` decides whether a record prints as `namespace:path`. `print_import_record_path` and the new `ModuleInfo::str_for_import_record` both call it. The three recording sites use the latter. - The transpiler cache version goes from 32 to 33. The on-disk cache stores the same record, so an older entry brings the bug back. - Verified: three new cases in `test/cli/test/isolation.test.ts` (`--isolate`, `--parallel`, on-disk cache) fail on 1.4.3-canary. Also ran `test/js/bun/plugin/`. - Self-reviewed: 4 concerns raised, 3 addressed. Not done: keep plugin-rewritten files out of the on-disk cache, an older separate bug (Notes). ### Background - `--isolate` gives each test file a fresh global. Bun builds the JSC module record from `ModuleInfo`, a table the printer fills, with no second parse. - The runtime linker (`src/bundler/linker.rs:462`) runs plugin `onResolve` before the print, stores the answer as `path.text` plus `path.namespace`, and sets `PRINT_NAMESPACE_IN_PATH`. - A source specifier `virt:thing` breaks the same way when namespace `virt` has an `onResolve`. With only an `onLoad` it works. <details><summary>Notes</summary> **Related PRs** - oven-sh#33905 was the first version of this fix. A stale-PR cleanup closed it on 2026-09-13 with no review verdict. - oven-sh#35605 (open, conflicts with main) attaches `ModuleInfo` to every ESM transpile, not only under `--isolate`. With it, this bug reaches plain `bun run`. Its commit 0de84ef makes the same change to the printer (one predicate, the same three sites). This PR does not depend on oven-sh#35605 and does not change when `ModuleInfo` is built. Whichever lands second keeps one helper. - oven-sh#40836 (open) rewrites the same three recording sites for import attributes and still interns `path.text` there. It has to call `str_for_import_record` after a rebase. **Which plugins were affected** - The trigger is the linker rewrite, not the `?query`. The record breaks when the namespaced module cannot be found again from the bare `path.text`. - `./data.bar?custom` with an `onResolve` that strips the query and sets a namespace: broken. This is the issue's case. - `virt:thing` in source with an `onResolve` for namespace `virt`: broken (`Cannot find package 'resolved-other'` in the new test). This is the esbuild-style virtual module pattern. - `./data.bar` with `onResolve({ filter: /\.bar$/ })` into a namespace: worked by accident. The runtime resolve of the bare path matched the same `onResolve` again. - A namespaced specifier with only an `onLoad`: worked. The linker does not rewrite the record, so `path.text` keeps the prefix. - `onResolve` with no namespace (a plain redirect): worked, and still takes the `None` arm of `printed_namespace`. The new test keeps one such import. **Scope of the change** - `request_module` and `request_module_with_phase` have three callers in the printer. All three are changed. The `require()` and `import()` printers also call `print_import_record_path`, but they add nothing to the module record. - Only `src/bundler/linker.rs:472` sets `PRINT_NAMESPACE_IN_PATH`, and only the runtime transpile path (`src/runtime/jsc_hooks.rs`) reaches it with a `ModuleInfo`. The bundler and `bun build --compile` never set the flag. **On-disk transpiler cache** - Stale entry check: ran the issue's repro with a test file above the 4 KiB cache floor and `BUN_RUNTIME_TRANSPILER_CACHE_PATH` set. The `.pile` entry from 1.4.3-canary holds `custom:/abs/data.bar` in the output and `/abs/data.bar` in the ESM record, and the second run fails from the cache. The cache file name is the input hash only, so a fixed build reads that entry unless the version changes. Version 22 was bumped for the same reason. - The third new test runs the fixture cold and warm against a private cache directory, with one module padded past 4 KiB. It covers the record round trip through the cache entry. - Not changed here: the on-disk cache stores output that holds a plugin's link-time `onResolve` answer, under a key made from the importing file's content. If the plugin later answers differently, the old answer is served until the importing file changes. This is independent of `--isolate` and older than this bug. One line after the rewrite in `src/bundler/linker.rs` (`result.runtime_transpiler_cache = None`) would stop it. `parse_entry.rs` already drops the cache in a like case, when it rewrites `@jest/globals` to `bun:test`. The cost is a transpile on every run for each file that a resolve plugin touches, for example every importer of a path alias. That trade needs a maintainer's call, so it is left for a separate PR. **Self-review** 1. The PR body did not name oven-sh#35605 and oven-sh#40836, and said a source-written `custom:foo` was never affected. Fixed above. 2. No test for a namespace that is already in the source. Added the `virt:` imports. 3. The cache version bump was checked by hand only. Added the cold and warm cache test. 4. Keep plugin-rewritten output out of the on-disk cache. Not done, see above. **Tests** - The new cases fail on the unfixed release build with `Cannot find package 'resolved-other'` (first error) and `SyntaxError: Export named 'named' not found in module '<dir>/reexport-clause.ts'`. The same fixture passes there without `--isolate`. - Suites run with the debug build: `test/cli/test/isolation.test.ts` (39 pass), `test/js/bun/plugin/` (48 pass), `test/cli/run/transpiler-cache.test.ts`, `test/regression/issue/30887.test.ts`, `test/js/bun/typescript/type-export.test.ts`, `test/cli/inspect/debugger-buntranspiledmodule.test.ts`. Two cases in the last four (`--drop invalidates cache`, one `--compile` case) hit the 5 s default timeout on the local debug build and pass with a longer timeout. </details> <!-- robobun:evidence:begin --> --- **[human-review]** gate passed · iteration 0 · 3 files touched <details><summary>fails on main (without fix)</summary> ```console ASAN without fix: 3 FAILED $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/cli/test/isolation.test.ts bun test v1.4.3 (b993710) test/cli/test/isolation.test.ts: (pass) bun test --isolate > without --isolate, leaked global is visible to next file [430.63ms] (pass) bun test --isolate > with --isolate, each file gets a fresh global [483.42ms] (pass) bun test --isolate > without --isolate, --preload still runs once (regression) [282.91ms] (pass) bun test --isolate > with --isolate, --preload re-runs in each file's fresh global [448.63ms] (pass) bun test --isolate > with --isolate, module state is not shared between files [379.00ms] (pass) bun test --isolate > with --isolate, a file's process.chdir() is undone before the next file [1257.31ms] (pass) bun test --isolate > with --isolate, a file's process.env writes with native side effects are undone before the next file [1568.05ms] 439 | using dir = tempDir("isolate-plugin-namespace", pluginNamespaceFixture); 440 | const { stderr, exitCode } = await runTests(String(dir), args, ["./a.test.ts", "./b.test.ts"], { 441 | ...bunEnv, 442 | ...en ... (truncated) release without fix: 3 FAILED bun test v1.4.3-canary.1 (b993710) test/cli/test/isolation.test.ts: 439 | using dir = tempDir("isolate-plugin-namespace", pluginNamespaceFixture); 440 | const { stderr, exitCode } = await runTests(String(dir), args, ["./a.test.ts", "./b.test.ts"], { 441 | ...bunEnv, 442 | ...env, 443 | }); 444 | expect(normalizeBunSnapshot(stderr, dir)).toContain("2 pass"); ^ error: expect(received).toContain(expected) Expected to contain: "2 pass" Received: "a.test.ts:\n\n# Unhandled error between tests\n-------------------------------\nerror: Cannot find package 'resolved-other' from '<dir>/a.test.ts'\n-------------------------------\n\n\nb.test.ts:\n\n# Unhandled error between tests\n-------------------------------\nerror: Cannot find package 'resolved-other' from '<dir>/b.test.ts'\n-------------------------------\n\n\n 0 pass\n 2 fail\n 2 errors\nRan 2 tests across 2 files." at <anonymous> (/workspace/bun/test/cli/test/isolation.test.ts:444:47) (pass) bun test --isolate > without --isolate, leaked global is visible to next file [26.77ms] (pass) bun test --isolate > with --isolate, --preloa ... (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/cli/test/isolation.test.ts bun test v1.4.3 (b993710) test/cli/test/isolation.test.ts: (pass) bun test --isolate > with --isolate, each file gets a fresh global [352.05ms] (pass) bun test --isolate > without --isolate, leaked global is visible to next file [439.29ms] (pass) bun test --isolate > without --isolate, --preload still runs once (regression) [297.02ms] (pass) bun test --isolate > with --isolate, --preload re-runs in each file's fresh global [336.20ms] (pass) bun test --isolate > with --isolate, module state is not shared between files [371.07ms] (pass) bun test --isolate > with --isolate, a file's process.env writes with native side effects are undone before the next file [1392.12ms] (pass) bun test --isolate > with --isolate, a file's process.chdir() is undone before the next file [1461.43ms] (pass) bun test --isolate > cached module records keep the namespace a plugin onResolve gives an import (--isolate) [542.79ms] (pass) bun test --isolate > with --isolate, cached module records keep short, Latin-1 and UTF-16 names ... (truncated) release with fix: all passed $ bun scripts/build.ts --profile=release [configured] bun-profile → bun (stripped) target linux-x64-gnu build type Release build dir ./build/release revision 68e0e0b features baseline 23 deps, 131 codegen, 1176 objects in 689ms ninja: Entering directory `/workspace/bun/build/release' [1/1248] install /workspace/bun bun install v1.4.3-canary.1 (b993710) Checked 22 installs across 61 packages (no changes) [7.00ms] [2/1248] gen ErrorCode+*.h [3/1248] install /workspace/bun/packages/bun-error bun install v1.4.3-canary.1 (b993710) Checked 1 install across 2 packages (no changes) [1.00ms] [4/1248] gen bindgenv2 [5/1248] install /workspace/bun/src/node-fallbacks bun install v1.4.3-canary.1 (b993710) Checked 111 installs across 104 packages (no changes) [4.00ms] [6/1248] gen node-fallbacks/react-refresh.js Bundled 1 module in 4ms react-refresh.js 4.81 KB (entry point) [7/1248] fetch zlib [zlib] up to date [8/1248] fetch tinycc [tinycc] up to date [9/1247] fetch libjpeg-turbo [libjpeg-turbo] up to date [10/1247] gen bake.{client,server,error}.js -> bake.client.js, bake.server.js, bake.error.js [11/1247] gen .bind.ts → Gene ... (truncated) ``` </details> <details><summary>diff hotspot</summary> ``` src/js_printer/lib.rs | 51 +++++++++-------- src/jsc/RuntimeTranspilerCache.rs | 5 +- test/cli/test/isolation.test.ts | 116 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 149 insertions(+), 23 deletions(-) ``` </details> **gate history** · 1 passed · 0 rejected · iteration 0 <details><summary>evidence per changed file</summary> ``` file reads edits tests src/js_printer/lib.rs 6 5 23 src/jsc/RuntimeTranspilerCache.rs 2 1 23 test/cli/test/isolation.test.ts 4 3 23 ``` </details> <!-- robobun:evidence:end --> Co-authored-by: Alistair Smith <hi@alistair.sh>
This branch has not been deployed
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
3 participants
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.
Problem
parse_path(src/js_parser/parse/mod.rs) reducedwith { ... }to a loader and dropped the clause; the printer re-synthesizedwith { type }only forimportunder--target=bun. Other targets,--no-bundleand re-exports lost it (Node:ERR_IMPORT_ATTRIBUTE_MISSING), and non-typekeys were dropped everywhere.typevalues collapsed into one module with the first import's loader. Unknowntypevalues (andbytes) were ignored, a newline beforewithwas a syntax error, duplicate keys were accepted.Fix
ImportRecord.attributeskeeps the clause as written. The printer prints it (aswith) on every import and re-export that stays in the output, for every target, and derives ModuleInfoFetchParametersfrom thetypeattribute as JSC's parser does.(path, attributes)(ImportRecord::module_graph_key), like esbuild and JSC's module map, on the plugin paths too. An unknowntypeis an error when the file is bundled; the runtime throwsERR_IMPORT_ATTRIBUTE_UNSUPPORTED. Duplicate keys are a parse error. Onlyassertkeeps the no-newline rule.bytesloader: aUint8Arrayof the file. Bundles inline base64 decoded by the__toBytesruntime helper;--compileembeds the raw bytes.test/bundler/bundler_import_attributes.test.ts(32 cases, 29 fail on 1.4.1) andtest/js/bun/import-attributes/import-attributes.test.ts. Also the transpiler, loader, plugin, compile, bundler, bake and esbuild suites.Background
(specifier, ScriptFetchParameters::Type):jsonis built in, any othertypeis host-defined and Bun maps it to a loader.export default <expr>.__toBytesis the first runtime helper called from one, sogenerate_code_for_lazy_exportbinds the call to the runtime's export.Notes
Overlapping open PRs this supersedes in part: #37476 (bundler identity), #38407 (re-export loader), #33255 (unknown type). #28512 rejects unknown keys; this PR keeps them, as JSC does under
BUN_JSC_ADDITIONS, sowith { type: "json", custom: "x" }still loads. #38461 (splitting rewrites a dynamic import to a chunk but keeps its attributes) is a separate bug.Decisions:
assertprints aswith. No current engine acceptsassertand rejectswith.typekeys are kept verbatim and never error, in the bundler or the runtime.type: "webassembly"maps to thewasmloader, so the attribute JSC reserves keeps loading.wasmfiles as before.bunBakeGraphis left out of the key: it routes the import to the SSR graph, which has its own map, soreact-dom/serverstays one module there.onResolvefallback path the unknown-type check runs after the external check, like the direct path, so an external import keeps anytype.with.typefrom the loader (an existing test asserts it).readFileSync(__filename)), like the sqlite shim, sorequire("<bunfs path>")gets the array itself.__toBytesreadsBufferandUint8ArrayoffglobalThis. A free reference inruntime.jsbecomes a reserved name in every bundle, which renames user code that shadows it (lodash'svar BufferbecameBuffer2);test/bundler/bundler_bytecode_portable.test.tscaught that.type: "macro"is exempt from the unknown-type error in the runtime as well as the bundler; a dynamicimport(x, { with: { type: "macro" } })loads the module as before.require.extensionswrappers pass an empty type attribute; it counts as none.RuntimeTranspilerCacheversion 27 -> 28.Found while testing, handed off separately:
export * from "./data.json"does not forward the JSON keys inbun build(no attributes involved).Suites run with the debug build: transpiler.test.js, import-attributes.test.ts, bundler_loader, esbuild/loader, esbuild/default, esbuild/metafile, bundler_edgecase, bundler_bun, bundler_cjs, bundler_splitting, bundler_html, html-import-manifest, compile-asset-bunfs, bundler_compile (one pre-existing debug-only failure:
HelloWorldWithProcessVersionsBuncomparesprocess.versions.bunto the release version), bundler_plugin, native-plugin, bake/dev/bundle, webkit-upgrade-3722912f, import-defer, macro-test, the resolve loader suites, the regression tests that mention import attributes, source-lints, bun-types.[review] gate passed · iteration 1 · 43 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 4 passed · 0 rejected · iteration 1
evidence per changed file