From 7dee5190d6ff8e7ceb7d0ecff23d79a5bd608c27 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 22 Jul 2026 03:35:22 +0000 Subject: [PATCH 01/12] bundler: use onResolve-returned path for external imports When an onResolve plugin returns { path, external: true }, the returned path was discarded and the emitted import kept the original specifier, so plugins could not rewrite externalized imports (bare specifier to a CDN URL, vendored path, etc.). BundleV2::on_resolve dropped result.path and result.namespace in the external branch without writing them back to the import record. The external branch now mirrors the native resolver's external handling and rewrites the import record's path when it differs from the original. Also fixes the adjacent case where { external: true } with no path produced a 'Could not resolve' error; runOnResolvePlugins now falls back to the input specifier, matching esbuild. Fixes #2805 --- src/bundler/bundle_v2.rs | 63 ++++++++++++++++++++--------- src/js/builtins/BundlerPlugin.ts | 10 +++-- test/bundler/bundler_plugin.test.ts | 55 +++++++++++++++++++++++++ 3 files changed, 104 insertions(+), 24 deletions(-) diff --git a/src/bundler/bundle_v2.rs b/src/bundler/bundle_v2.rs index 77b4f33d014a..dba131381cda 100644 --- a/src/bundler/bundle_v2.rs +++ b/src/bundler/bundle_v2.rs @@ -4706,27 +4706,26 @@ pub mod bv2_impl { } jsc_api::JSBundler::ResolveValue::Success(result) => { let mut out_source_index: Option = None; + // SAFETY: `result.{path,namespace}` are `Box<[u8]>` whose heap + // allocations are moved into `this.free_list` below (in the + // `!found_existing` and `external` branches) and thus outlive `BundleV2`. + // Erase to `'static` so `Fs::Path<'static>` can borrow them across + // `path_with_pretty_initialized` / `ParseTask`. In the `found_existing` + // branch `path` is dead before the boxes drop, so the dangling + // `'static` is never observed. + let (result_path_static, result_ns_static): (&'static [u8], &'static [u8]) = unsafe { + ( + &*std::ptr::from_ref::<[u8]>(result.path.as_ref()), + &*std::ptr::from_ref::<[u8]>(result.namespace.as_ref()), + ) + }; + let mut path = Fs::Path::init(result_path_static); + if result.namespace.is_empty() || result.namespace.as_ref() == b"file" { + path.namespace = b"file"; + } else { + path.namespace = result_ns_static; + } if !result.external { - // SAFETY: `result.{path,namespace}` are `Box<[u8]>` whose heap - // allocations are moved into `this.free_list` below (in the - // `!found_existing` branch) and thus outlive `BundleV2`. Erase - // to `'static` so `Fs::Path<'static>` can borrow them across - // `path_with_pretty_initialized` / `ParseTask`. In the `found_existing`/`external` - // branches `path` is dead before the boxes drop, so the dangling - // `'static` is never observed. - let (result_path_static, result_ns_static): (&'static [u8], &'static [u8]) = unsafe { - ( - &*std::ptr::from_ref::<[u8]>(result.path.as_ref()), - &*std::ptr::from_ref::<[u8]>(result.namespace.as_ref()), - ) - }; - let mut path = Fs::Path::init(result_path_static); - if result.namespace.is_empty() || result.namespace.as_ref() == b"file" { - path.namespace = b"file"; - } else { - path.namespace = result_ns_static; - } - // SAFETY: `GetOrPutResult` borrows `&mut this` for its whole // lifetime, blocking the `free_list`/`graph` accesses below. // Capture `value_ptr` as a raw ptr + `found_existing` and drop @@ -4846,6 +4845,30 @@ pub mod bv2_impl { drop(result.namespace); drop(result.path); } + } else if resolve.import_record.kind != ImportKind::EntryPointBuild { + // `{ path, external: true }` from an onResolve plugin: rewrite the + // import record's path so the emitted external import uses the + // plugin-returned specifier (esbuild parity). + let source_import_records = &mut this.graph.ast.items_import_records_mut() + [resolve.import_record.importer_source_index as usize]; + if (source_import_records.len() as u32) + > resolve.import_record.import_record_index + { + let import_record: &mut ImportRecord = &mut source_import_records + .as_mut_slice() + [resolve.import_record.import_record_index as usize]; + if !strings::eql_long(import_record.path.text, path.text, true) { + import_record.path = path_as_static(&path); + this.free_list.push(result.namespace); + this.free_list.push(result.path); + } else { + drop(result.namespace); + drop(result.path); + } + } else { + drop(result.namespace); + drop(result.path); + } } else { if resolve.import_record.kind == ImportKind::EntryPointBuild { let log = this.log_for_resolution_failures( diff --git a/src/js/builtins/BundlerPlugin.ts b/src/js/builtins/BundlerPlugin.ts index 9f05e17b5988..28243b1634f1 100644 --- a/src/js/builtins/BundlerPlugin.ts +++ b/src/js/builtins/BundlerPlugin.ts @@ -441,16 +441,18 @@ export function runOnResolvePlugins(this: BundlerPlugin, specifier, inputNamespa throw new TypeError("onResolve plugins 'namespace' field must be a string if provided"); } + if (typeof external !== "boolean" && !$isUndefinedOrNull(external)) { + throw new TypeError('onResolve plugins "external" field must be boolean or unspecified'); + } + if (!path) { - continue; + if (external) path = inputPath; + else continue; } if (!userNamespace) { userNamespace = inputNamespace; } - if (typeof external !== "boolean" && !$isUndefinedOrNull(external)) { - throw new TypeError('onResolve plugins "external" field must be boolean or unspecified'); - } if (!external) { if (userNamespace === "file") { diff --git a/test/bundler/bundler_plugin.test.ts b/test/bundler/bundler_plugin.test.ts index 3e357e85c298..eb317ae491b5 100644 --- a/test/bundler/bundler_plugin.test.ts +++ b/test/bundler/bundler_plugin.test.ts @@ -441,6 +441,61 @@ describe("bundler", () => { }, }; }); + // https://github.com/oven-sh/bun/issues/2805 + itBundled("plugin/ResolveExternalRewritesPath", { + files: { + "index.ts": /* ts */ ` + import React from "react"; + console.log(React); + `, + }, + plugins(builder) { + builder.onResolve({ filter: /^react$/ }, () => { + return { path: "https://esm.sh/react@19", external: true }; + }); + }, + onAfterBundle(api) { + const contents = api.readFile("/out.js"); + expect(contents).toContain(`from "https://esm.sh/react@19"`); + expect(contents).not.toContain(`from "react"`); + }, + }); + itBundled("plugin/ResolveExternalRewritesPathRequire", { + files: { + "index.ts": /* ts */ ` + const React = require("react"); + console.log(React); + `, + }, + format: "cjs", + plugins(builder) { + builder.onResolve({ filter: /^react$/ }, () => { + return { path: "./vendor/react.cjs", external: true }; + }); + }, + onAfterBundle(api) { + const contents = api.readFile("/out.js"); + expect(contents).toContain(`require("./vendor/react.cjs")`); + expect(contents).not.toContain(`require("react")`); + }, + }); + itBundled("plugin/ResolveExternalWithoutPath", { + files: { + "index.ts": /* ts */ ` + import lodash from "lodash"; + console.log(lodash); + `, + }, + plugins(builder) { + builder.onResolve({ filter: /^lodash$/ }, () => { + return { external: true }; + }); + }, + onAfterBundle(api) { + const contents = api.readFile("/out.js"); + expect(contents).toContain(`from "lodash"`); + }, + }); itBundled("plugin/ResolveOverrideFile", ({ root }) => { return { files: { From dc18fd95b9b365b0edc462c0e58010a44bba0e14 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 22 Jul 2026 03:54:46 +0000 Subject: [PATCH 02/12] bundler: fold the entry point external error into the rewrite branch Main (#39799) already logs "The entry point ... cannot be marked as external" from on_resolve. Restructure so that arm comes first and the import path rewrite is the plain else, instead of a nested check inside the fallthrough. Adapt the 29264 regression test: now that { external: true } without a path no longer falls through to NoMatch, its catch-all filter must skip the entry point or the build fails on the new entry point error. --- src/bundler/bundle_v2.rs | 34 ++++++++++++++--------------- test/regression/issue/29264.test.ts | 5 ++++- 2 files changed, 20 insertions(+), 19 deletions(-) diff --git a/src/bundler/bundle_v2.rs b/src/bundler/bundle_v2.rs index dba131381cda..1092c632f560 100644 --- a/src/bundler/bundle_v2.rs +++ b/src/bundler/bundle_v2.rs @@ -4845,7 +4845,22 @@ pub mod bv2_impl { drop(result.namespace); drop(result.path); } - } else if resolve.import_record.kind != ImportKind::EntryPointBuild { + } else if resolve.import_record.kind == ImportKind::EntryPointBuild { + let log = this.log_for_resolution_failures( + &resolve.import_record.source_file, + resolve.import_record.original_target.bake_graph(), + ); + log.add_error_fmt( + None, + bun_ast::Loc::EMPTY, + format_args!( + "The entry point {} cannot be marked as external", + bun_core::fmt::quote(&resolve.import_record.specifier), + ), + ); + drop(result.namespace); + drop(result.path); + } else { // `{ path, external: true }` from an onResolve plugin: rewrite the // import record's path so the emitted external import uses the // plugin-returned specifier (esbuild parity). @@ -4869,23 +4884,6 @@ pub mod bv2_impl { drop(result.namespace); drop(result.path); } - } else { - if resolve.import_record.kind == ImportKind::EntryPointBuild { - let log = this.log_for_resolution_failures( - &resolve.import_record.source_file, - resolve.import_record.original_target.bake_graph(), - ); - log.add_error_fmt( - None, - bun_ast::Loc::EMPTY, - format_args!( - "The entry point {} cannot be marked as external", - bun_core::fmt::quote(&resolve.import_record.specifier), - ), - ); - } - drop(result.namespace); - drop(result.path); } if let Some(source_index) = out_source_index { diff --git a/test/regression/issue/29264.test.ts b/test/regression/issue/29264.test.ts index a2f1112758c4..02b998a94cfe 100644 --- a/test/regression/issue/29264.test.ts +++ b/test/regression/issue/29264.test.ts @@ -12,7 +12,10 @@ test("#29264 bundler survives external + missing imports in same file", { timeou { name: "mark-bare-external", setup(build) { - build.onResolve({ filter: /^[^.]/ }, () => ({ external: true })); + build.onResolve({ filter: /^[^.]/ }, args => { + if (args.kind === "entry-point-build") return; + return { external: true }; + }); }, }, ], From d989e00a97103396b2d7a0f7a5e8406e7a369c5a Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 22 Jul 2026 04:23:36 +0000 Subject: [PATCH 03/12] test: keep 29264 exercising NoMatch->run_resolver on the error path Add a third bare import whose onResolve returns undefined so the deferred NoMatch -> run_resolver path (and its unchecked import_records index) is still reached while ./src supplies the last_error trigger. Without this the adapted test would pass with the error-path import_records store reverted. --- test/regression/issue/29264.test.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/test/regression/issue/29264.test.ts b/test/regression/issue/29264.test.ts index 02b998a94cfe..8796398cc4e8 100644 --- a/test/regression/issue/29264.test.ts +++ b/test/regression/issue/29264.test.ts @@ -14,7 +14,10 @@ test("#29264 bundler survives external + missing imports in same file", { timeou setup(build) { build.onResolve({ filter: /^[^.]/ }, args => { if (args.kind === "entry-point-build") return; - return { external: true }; + if (args.path === "src") return { external: true }; + // "other": fall through to NoMatch -> run_resolver so the + // unchecked import_records[..] access there is still + // exercised against the error-path store (#29264). }); }, }, @@ -30,6 +33,7 @@ test("#29264 bundler survives external + missing imports in same file", { timeou `, "index.js": /* js */ ` import "src"; + import "other"; import "./src"; `, }); @@ -46,11 +50,9 @@ test("#29264 bundler survives external + missing imports in same file", { timeou // Before the fix, the child crashed in Bun.build — segfault (release) or // index-out-of-bounds panic (debug/ASAN) — so "DONE:caught" never printed. - // We deliberately don't assert on the bare "src" import; whether the - // plugin's `{ external: true }` (with no `path`) falls through to a - // resolver error is plugin semantics, not what this test guards against. const combined = stdout + stderr; expect(combined).toContain("DONE:caught"); expect(combined).toContain('Could not resolve: "./src"'); + expect(combined).toContain('Could not resolve: "other"'); expect(exitCode).toBe(0); }); From 3093c3d01f6c687d752592caa66b2b18bd87d65f Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 21 Aug 2026 01:47:30 +0000 Subject: [PATCH 04/12] bundler: shorten the external rewrite comment --- src/bundler/bundle_v2.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/bundler/bundle_v2.rs b/src/bundler/bundle_v2.rs index 1092c632f560..e0c4803c2c8a 100644 --- a/src/bundler/bundle_v2.rs +++ b/src/bundler/bundle_v2.rs @@ -4861,9 +4861,7 @@ pub mod bv2_impl { drop(result.namespace); drop(result.path); } else { - // `{ path, external: true }` from an onResolve plugin: rewrite the - // import record's path so the emitted external import uses the - // plugin-returned specifier (esbuild parity). + // Like esbuild, the external import is printed with the plugin's path. let source_import_records = &mut this.graph.ast.items_import_records_mut() [resolve.import_record.importer_source_index as usize]; if (source_import_records.len() as u32) From 424e2b247c27951c84827a4b32f7f4755cf90d79 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 21 Aug 2026 02:26:40 +0000 Subject: [PATCH 05/12] bundler: route the external path rewrite through the pending import queue on_resolve wrote the plugin path straight into the importer's record and dropped it when the records were not on the graph yet, while the sibling source index arm queues for patch_import_record_source_indices in that case. PendingImport is now an enum with both kinds of answer, and both arms go through apply_or_defer_pending_import. Queued external paths are applied after the path map pass so they are never looked up as modules. Compare the plugin path against the dispatched specifier instead of the record, which is the same bytes and does not need the record present. Tests: the ESM case also covers export-from, export-star and import(), and a same-path answer leaves the import unchanged. --- src/bundler/bundle_v2.rs | 178 ++++++++++++++++++---------- test/bundler/bundler_plugin.test.ts | 35 +++++- 2 files changed, 145 insertions(+), 68 deletions(-) diff --git a/src/bundler/bundle_v2.rs b/src/bundler/bundle_v2.rs index e0c4803c2c8a..8045c34ac11e 100644 --- a/src/bundler/bundle_v2.rs +++ b/src/bundler/bundle_v2.rs @@ -51,10 +51,45 @@ pub use bv2_impl::JSBundleCompletionTask; /// `jsc::api::JSBundler::FileMap` — re-exported from the canonical def below. pub use api::JSBundler::FileMap; +/// An onResolve plugin answer for one import record of an importer whose +/// records may not be on the graph yet (see +/// `resolve_tasks_waiting_for_import_source_index`). #[derive(Clone, Copy)] -pub struct PendingImport { - pub(crate) to_source_index: Index, - pub(crate) import_record_index: u32, +pub enum PendingImport { + SourceIndex { + import_record_index: u32, + to_source_index: Index, + }, + /// `{ path, external: true }`: the record is printed with `path`, whose + /// bytes are parked on `BundleV2::free_list`. + ExternalPath { + import_record_index: u32, + path: bun_paths::fs::Path<'static>, + }, +} + +impl PendingImport { + pub(crate) fn import_record_index(self) -> u32 { + match self { + Self::SourceIndex { + import_record_index, + .. + } + | Self::ExternalPath { + import_record_index, + .. + } => import_record_index, + } + } + + pub(crate) fn apply(self, import_record: &mut bun_ast::ImportRecord) { + match self { + Self::SourceIndex { + to_source_index, .. + } => import_record.source_index = to_source_index, + Self::ExternalPath { path, .. } => import_record.path = path, + } + } } pub struct BundleV2<'a> { @@ -4706,13 +4741,12 @@ pub mod bv2_impl { } jsc_api::JSBundler::ResolveValue::Success(result) => { let mut out_source_index: Option = None; - // SAFETY: `result.{path,namespace}` are `Box<[u8]>` whose heap - // allocations are moved into `this.free_list` below (in the - // `!found_existing` and `external` branches) and thus outlive `BundleV2`. - // Erase to `'static` so `Fs::Path<'static>` can borrow them across - // `path_with_pretty_initialized` / `ParseTask`. In the `found_existing` - // branch `path` is dead before the boxes drop, so the dangling - // `'static` is never observed. + // SAFETY: `result.{path,namespace}` are `Box<[u8]>`. Every arm below + // either moves both boxes into `this.free_list`, which outlives the + // graph, before it stores `path` (`!found_existing`, and the external + // rewrite), or drops them after its last read of `path` without + // storing it (`found_existing`, entry point, unchanged specifier). So + // the erased `'static` borrow is never read after the bytes are freed. let (result_path_static, result_ns_static): (&'static [u8], &'static [u8]) = unsafe { ( &*std::ptr::from_ref::<[u8]>(result.path.as_ref()), @@ -4860,28 +4894,20 @@ pub mod bv2_impl { ); drop(result.namespace); drop(result.path); + } else if strings::eql_long(&resolve.import_record.specifier, path.text, true) { + drop(result.namespace); + drop(result.path); } else { // Like esbuild, the external import is printed with the plugin's path. - let source_import_records = &mut this.graph.ast.items_import_records_mut() - [resolve.import_record.importer_source_index as usize]; - if (source_import_records.len() as u32) - > resolve.import_record.import_record_index - { - let import_record: &mut ImportRecord = &mut source_import_records - .as_mut_slice() - [resolve.import_record.import_record_index as usize]; - if !strings::eql_long(import_record.path.text, path.text, true) { - import_record.path = path_as_static(&path); - this.free_list.push(result.namespace); - this.free_list.push(result.path); - } else { - drop(result.namespace); - drop(result.path); - } - } else { - drop(result.namespace); - drop(result.path); - } + this.free_list.push(result.namespace); + this.free_list.push(result.path); + this.apply_or_defer_pending_import( + resolve.import_record.importer_source_index, + PendingImport::ExternalPath { + import_record_index: resolve.import_record.import_record_index, + path: path_as_static(&path), + }, + ); } if let Some(source_index) = out_source_index { @@ -4897,29 +4923,13 @@ pub mod bv2_impl { .entry_point_original_names .put(source_index.get(), &resolve.import_record.specifier); } else { - let source_import_records = - &mut this.graph.ast.items_import_records_mut() - [resolve.import_record.importer_source_index as usize]; - if source_import_records.len() as u32 - <= resolve.import_record.import_record_index - { - let entry = this - .resolve_tasks_waiting_for_import_source_index - .get_or_put(resolve.import_record.importer_source_index) - .expect("oom"); - if !entry.found_existing { - *entry.value_ptr = Vec::new(); - } - let _ = entry.value_ptr.push(PendingImport { - to_source_index: source_index, + this.apply_or_defer_pending_import( + resolve.import_record.importer_source_index, + PendingImport::SourceIndex { import_record_index: resolve.import_record.import_record_index, - }); - } else { - let import_record: &mut ImportRecord = &mut source_import_records - .as_mut_slice() - [resolve.import_record.import_record_index as usize]; - import_record.source_index = source_index; - } + to_source_index: source_index, + }, + ); } } } @@ -6816,6 +6826,28 @@ pub mod bv2_impl { } impl<'a> BundleV2<'a> { + /// Writes a plugin answer to the importer's record, or queues it for + /// `patch_import_record_source_indices` while the importer's records are + /// not on the graph yet. + fn apply_or_defer_pending_import(&mut self, importer: IndexInt, pending: PendingImport) { + let import_records = &mut self.graph.ast.items_import_records_mut()[importer as usize]; + if let Some(import_record) = import_records + .as_mut_slice() + .get_mut(pending.import_record_index() as usize) + { + pending.apply(import_record); + return; + } + let entry = self + .resolve_tasks_waiting_for_import_source_index + .get_or_put(importer) + .expect("oom"); + if !entry.found_existing { + *entry.value_ptr = Vec::new(); + } + let _ = entry.value_ptr.push(pending); + } + /// Patch source_index on import records from pathToSourceIndexMap and /// resolve_tasks_waiting_for_import_source_index. Called after /// processResolveQueue has registered new modules. @@ -6834,22 +6866,30 @@ pub mod bv2_impl { || ctx.loader == Loader::Html || ctx.loader.is_css(); - if let Some(idx) = self + let pending = self .resolve_tasks_waiting_for_import_source_index .get_index(&ctx.source_index.get()) - { - let (_, value) = self - .resolve_tasks_waiting_for_import_source_index - .swap_remove_at(idx); - for to_assign in value.slice() { - if save_import_record_source_index - || input_file_loaders[to_assign.to_source_index.get() as usize].is_css() + .map(|idx| { + self.resolve_tasks_waiting_for_import_source_index + .swap_remove_at(idx) + .1 + }); + if let Some(pending) = &pending { + for to_assign in pending.slice() { + if let PendingImport::SourceIndex { + import_record_index, + to_source_index, + } = *to_assign { - import_records.as_mut_slice()[to_assign.import_record_index as usize] - .source_index = to_assign.to_source_index; + if save_import_record_source_index + || input_file_loaders[to_source_index.get() as usize].is_css() + { + to_assign.apply( + &mut import_records.as_mut_slice()[import_record_index as usize], + ); + } } } - drop(value); } // Inlined `self.path_to_source_index_map(ctx.target)` (== `&mut self.graph.build_graphs[target]`) @@ -6873,6 +6913,18 @@ pub mod bv2_impl { } } } + + // After the map pass: an external path must not be looked up as a module. + if let Some(pending) = pending { + for to_assign in pending.slice() { + if matches!(to_assign, PendingImport::ExternalPath { .. }) { + to_assign.apply( + &mut import_records.as_mut_slice() + [to_assign.import_record_index() as usize], + ); + } + } + } } fn generate_server_html_module( diff --git a/test/bundler/bundler_plugin.test.ts b/test/bundler/bundler_plugin.test.ts index eb317ae491b5..06df3ae69365 100644 --- a/test/bundler/bundler_plugin.test.ts +++ b/test/bundler/bundler_plugin.test.ts @@ -446,18 +446,43 @@ describe("bundler", () => { files: { "index.ts": /* ts */ ` import React from "react"; - console.log(React); + import { createRoot } from "react-dom/client"; + export { h } from "preact"; + export * from "mobx"; + console.log(React, createRoot, await import("lodash")); `, }, plugins(builder) { - builder.onResolve({ filter: /^react$/ }, () => { - return { path: "https://esm.sh/react@19", external: true }; + builder.onResolve({ filter: /^(react|react-dom\/client|preact|mobx|lodash)$/ }, args => { + return { path: "https://esm.sh/" + args.path, external: true }; }); }, onAfterBundle(api) { const contents = api.readFile("/out.js"); - expect(contents).toContain(`from "https://esm.sh/react@19"`); - expect(contents).not.toContain(`from "react"`); + expect(contents).toContain(`from "https://esm.sh/react"`); + expect(contents).toContain(`from "https://esm.sh/react-dom/client"`); + expect(contents).toContain(`from "https://esm.sh/preact"`); + expect(contents).toContain(`from "https://esm.sh/mobx"`); + expect(contents).toContain(`import("https://esm.sh/lodash")`); + for (const original of ["react", "react-dom/client", "preact", "mobx", "lodash"]) { + expect(contents).not.toContain(`"${original}"`); + } + }, + }); + itBundled("plugin/ResolveExternalSamePath", { + files: { + "index.ts": /* ts */ ` + import React from "react"; + console.log(React); + `, + }, + plugins(builder) { + builder.onResolve({ filter: /^react$/ }, args => { + return { path: args.path, external: true }; + }); + }, + onAfterBundle(api) { + expect(api.readFile("/out.js")).toContain(`from "react"`); }, }); itBundled("plugin/ResolveExternalRewritesPathRequire", { From 5637b6a9f9686edefcc8b80cbff1348fbd59602d Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 21 Aug 2026 02:32:41 +0000 Subject: [PATCH 06/12] bundler: shorten PendingImport doc comments --- src/bundler/bundle_v2.rs | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/bundler/bundle_v2.rs b/src/bundler/bundle_v2.rs index 8045c34ac11e..35740e6a9b4f 100644 --- a/src/bundler/bundle_v2.rs +++ b/src/bundler/bundle_v2.rs @@ -51,17 +51,14 @@ pub use bv2_impl::JSBundleCompletionTask; /// `jsc::api::JSBundler::FileMap` — re-exported from the canonical def below. pub use api::JSBundler::FileMap; -/// An onResolve plugin answer for one import record of an importer whose -/// records may not be on the graph yet (see -/// `resolve_tasks_waiting_for_import_source_index`). +/// An onResolve answer, queued in `resolve_tasks_waiting_for_import_source_index`. #[derive(Clone, Copy)] pub enum PendingImport { SourceIndex { import_record_index: u32, to_source_index: Index, }, - /// `{ path, external: true }`: the record is printed with `path`, whose - /// bytes are parked on `BundleV2::free_list`. + /// `path` borrows bytes parked on `BundleV2::free_list`. ExternalPath { import_record_index: u32, path: bun_paths::fs::Path<'static>, @@ -6826,9 +6823,7 @@ pub mod bv2_impl { } impl<'a> BundleV2<'a> { - /// Writes a plugin answer to the importer's record, or queues it for - /// `patch_import_record_source_indices` while the importer's records are - /// not on the graph yet. + /// Defers to `patch_import_record_source_indices` until the importer's records are on the graph. fn apply_or_defer_pending_import(&mut self, importer: IndexInt, pending: PendingImport) { let import_records = &mut self.graph.ast.items_import_records_mut()[importer as usize]; if let Some(import_record) = import_records From d49cc5937d5f3bdf2b2034ee2d2d2e5fe03f97c7 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 21 Aug 2026 03:51:06 +0000 Subject: [PATCH 07/12] bundler: mark resolved externals so barrel passes do not resolve them again resolve_import_records runs over a barrel's whole record list each time a consumer un-defers one of its records. It skipped records with a module (source_index) but had no way to recognise a record already left external, so every external was resolved again. That was harmless while the path was unchanged, but after an onResolve plugin rewrote the path the second pass matched no plugin filter and failed on the rewritten specifier: Could not resolve: "react-vendored" Add ImportRecordFlags::IS_EXTERNAL, set it at each external arm of resolve_import_records and in PendingImport::ExternalPath, and skip records that carry it. Every plugin external answer now goes through ExternalPath, so the same path case is flagged too and no longer re-dispatches. The test pins the rewrite across a barrel revisit. It fails on released Bun (no rewrite) and on the previous commit (the resolve error above). --- src/ast/import_record.rs | 6 +++ src/bundler/bundle_v2.rs | 57 ++++++++++++++++------------- test/bundler/bundler_plugin.test.ts | 44 ++++++++++++++++++++++ 3 files changed, 82 insertions(+), 25 deletions(-) diff --git a/src/ast/import_record.rs b/src/ast/import_record.rs index 290da2cf7f71..60f6b1525ada 100644 --- a/src/ast/import_record.rs +++ b/src/ast/import_record.rs @@ -72,6 +72,12 @@ bitflags::bitflags! { const WAS_ORIGINALLY_REQUIRE = 1 << 9; + /// Resolution finished and left the import external. `path` may no + /// longer be the source specifier, so a later pass over the same + /// records (a barrel whose deferred records are resolved on demand) + /// must not resolve it again. + const IS_EXTERNAL = 1 << 10; + /// If true, this import can be removed if it's unused const IS_EXTERNAL_WITHOUT_SIDE_EFFECTS = 1 << 11; diff --git a/src/bundler/bundle_v2.rs b/src/bundler/bundle_v2.rs index 35740e6a9b4f..cf8ce057ee55 100644 --- a/src/bundler/bundle_v2.rs +++ b/src/bundler/bundle_v2.rs @@ -84,7 +84,12 @@ impl PendingImport { Self::SourceIndex { to_source_index, .. } => import_record.source_index = to_source_index, - Self::ExternalPath { path, .. } => import_record.path = path, + Self::ExternalPath { path, .. } => { + import_record.path = path; + import_record + .flags + .insert(bun_ast::ImportRecordFlags::IS_EXTERNAL); + } } } } @@ -4740,10 +4745,9 @@ pub mod bv2_impl { let mut out_source_index: Option = None; // SAFETY: `result.{path,namespace}` are `Box<[u8]>`. Every arm below // either moves both boxes into `this.free_list`, which outlives the - // graph, before it stores `path` (`!found_existing`, and the external - // rewrite), or drops them after its last read of `path` without - // storing it (`found_existing`, entry point, unchanged specifier). So - // the erased `'static` borrow is never read after the bytes are freed. + // graph, before it stores `path` (`!found_existing`, external), or + // drops them without storing `path` (`found_existing`, entry point). + // So the erased `'static` borrow is never read after the bytes are freed. let (result_path_static, result_ns_static): (&'static [u8], &'static [u8]) = unsafe { ( &*std::ptr::from_ref::<[u8]>(result.path.as_ref()), @@ -4891,9 +4895,6 @@ pub mod bv2_impl { ); drop(result.namespace); drop(result.path); - } else if strings::eql_long(&resolve.import_record.specifier, path.text, true) { - drop(result.namespace); - drop(result.path); } else { // Like esbuild, the external import is printed with the plugin's path. this.free_list.push(result.namespace); @@ -6018,13 +6019,11 @@ pub mod bv2_impl { import_record.source_index = Index::INVALID; } - estimated_resolve_queue_count += (!(import_record - .flags - .contains(bun_ast::ImportRecordFlags::IS_INTERNAL) - || import_record - .flags - .contains(bun_ast::ImportRecordFlags::IS_UNUSED) - || import_record.source_index.is_valid())) + estimated_resolve_queue_count += (!(import_record.flags.intersects( + bun_ast::ImportRecordFlags::IS_INTERNAL + | bun_ast::ImportRecordFlags::IS_UNUSED + | bun_ast::ImportRecordFlags::IS_EXTERNAL, + ) || import_record.source_index.is_valid())) as usize; } if let Some(only) = only_records { @@ -6052,6 +6051,7 @@ pub mod bv2_impl { || import_record.flags.contains(bun_ast::ImportRecordFlags::IS_INTERNAL) // Don't resolve pre-resolved imports || import_record.source_index.is_valid() + || import_record.flags.contains(bun_ast::ImportRecordFlags::IS_EXTERNAL) { continue; } @@ -6067,7 +6067,8 @@ pub mod bv2_impl { if import_record.path.text == src.path.pretty { if self.dev_server.is_some() { import_record.flags.insert( - bun_ast::ImportRecordFlags::IS_EXTERNAL_WITHOUT_SIDE_EFFECTS, + bun_ast::ImportRecordFlags::IS_EXTERNAL + | bun_ast::ImportRecordFlags::IS_EXTERNAL_WITHOUT_SIDE_EFFECTS, ); import_record.source_index = Index::INVALID; } else { @@ -6112,9 +6113,10 @@ pub mod bv2_impl { }; import_record.tag = replacement.tag; import_record.source_index = Index::INVALID; - import_record - .flags - .insert(bun_ast::ImportRecordFlags::IS_EXTERNAL_WITHOUT_SIDE_EFFECTS); + import_record.flags.insert( + bun_ast::ImportRecordFlags::IS_EXTERNAL + | bun_ast::ImportRecordFlags::IS_EXTERNAL_WITHOUT_SIDE_EFFECTS, + ); continue; } @@ -6123,9 +6125,10 @@ pub mod bv2_impl { import_record.path = bun_paths::fs::Path::init(new_text); import_record.path.namespace = b"bun"; import_record.source_index = Index::INVALID; - import_record - .flags - .insert(bun_ast::ImportRecordFlags::IS_EXTERNAL_WITHOUT_SIDE_EFFECTS); + import_record.flags.insert( + bun_ast::ImportRecordFlags::IS_EXTERNAL + | bun_ast::ImportRecordFlags::IS_EXTERNAL_WITHOUT_SIDE_EFFECTS, + ); // don't link bun continue; @@ -6134,9 +6137,10 @@ pub mod bv2_impl { // By default, we treat .sqlite files as external. if import_record.loader == Some(Loader::Sqlite) { - import_record - .flags - .insert(bun_ast::ImportRecordFlags::IS_EXTERNAL_WITHOUT_SIDE_EFFECTS); + import_record.flags.insert( + bun_ast::ImportRecordFlags::IS_EXTERNAL + | bun_ast::ImportRecordFlags::IS_EXTERNAL_WITHOUT_SIDE_EFFECTS, + ); continue; } @@ -6479,6 +6483,9 @@ pub mod bv2_impl { { import_record.path = path_as_static(&resolve_result.path_pair.primary); } + import_record + .flags + .insert(bun_ast::ImportRecordFlags::IS_EXTERNAL); import_record.flags.set( bun_ast::ImportRecordFlags::IS_EXTERNAL_WITHOUT_SIDE_EFFECTS, resolve_result.primary_side_effects_data diff --git a/test/bundler/bundler_plugin.test.ts b/test/bundler/bundler_plugin.test.ts index 06df3ae69365..e4372b00aa43 100644 --- a/test/bundler/bundler_plugin.test.ts +++ b/test/bundler/bundler_plugin.test.ts @@ -485,6 +485,50 @@ describe("bundler", () => { expect(api.readFile("/out.js")).toContain(`from "react"`); }, }); + // A barrel's records are resolved again each time a consumer un-defers one of + // them. The rewritten external must be skipped by those passes, not resolved + // as the vendored specifier. late.js is loaded only after the react answer is + // queued, so its un-defer of `a` runs after the rewrite landed. + itBundled("plugin/ResolveExternalRewriteSurvivesBarrelRevisit", () => { + const reactAnswered = Promise.withResolvers(); + return { + files: { + "/entry.js": /* js */ ` + import { React } from "barrel"; + import "./late.js"; + console.log(React); + `, + "/late.js": ``, + "/node_modules/barrel/package.json": JSON.stringify({ + name: "barrel", + main: "./index.js", + sideEffects: false, + }), + "/node_modules/barrel/index.js": /* js */ ` + export { default as React } from "react"; + export { a } from "./a.js"; + export { b } from "./b.js"; + `, + "/node_modules/barrel/a.js": `export const a = "a";`, + "/node_modules/barrel/b.js": `export const b = "b";`, + }, + plugins(builder) { + builder.onResolve({ filter: /^react$/ }, () => { + reactAnswered.resolve(); + return { path: "react-vendored", external: true }; + }); + builder.onLoad({ filter: /late\.js$/ }, async () => { + await reactAnswered.promise; + return { contents: `import { a } from "barrel"; console.log(a);`, loader: "js" }; + }); + }, + onAfterBundle(api) { + const contents = api.readFile("/out.js"); + expect(contents).toContain(`from "react-vendored"`); + expect(contents).not.toContain(`"react"`); + }, + }; + }); itBundled("plugin/ResolveExternalRewritesPathRequire", { files: { "index.ts": /* ts */ ` From e4b176dda6e5c04a96baf38b237103ac792913ea Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 21 Aug 2026 03:52:42 +0000 Subject: [PATCH 08/12] ast: shorten the IS_EXTERNAL doc comment --- src/ast/import_record.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/ast/import_record.rs b/src/ast/import_record.rs index 60f6b1525ada..4f87b2c7ae7e 100644 --- a/src/ast/import_record.rs +++ b/src/ast/import_record.rs @@ -72,10 +72,8 @@ bitflags::bitflags! { const WAS_ORIGINALLY_REQUIRE = 1 << 9; - /// Resolution finished and left the import external. `path` may no - /// longer be the source specifier, so a later pass over the same - /// records (a barrel whose deferred records are resolved on demand) - /// must not resolve it again. + /// Resolved as external. `path` may have been rewritten, so a second + /// resolution pass over the same records skips it. const IS_EXTERNAL = 1 << 10; /// If true, this import can be removed if it's unused From 7d978676f06386e5ea12d6483787f3402140f684 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 21 Aug 2026 03:54:58 +0000 Subject: [PATCH 09/12] ast: one line doc for IS_EXTERNAL --- src/ast/import_record.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/ast/import_record.rs b/src/ast/import_record.rs index 4f87b2c7ae7e..34a6093c2759 100644 --- a/src/ast/import_record.rs +++ b/src/ast/import_record.rs @@ -72,8 +72,7 @@ bitflags::bitflags! { const WAS_ORIGINALLY_REQUIRE = 1 << 9; - /// Resolved as external. `path` may have been rewritten, so a second - /// resolution pass over the same records skips it. + /// Resolution is done and `path` is the specifier to print. const IS_EXTERNAL = 1 << 10; /// If true, this import can be removed if it's unused From 80fb2469dca0fc051c5da976e0e7ed153dcccdf9 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 21 Aug 2026 04:50:45 +0000 Subject: [PATCH 10/12] bundler: skip external records in the path map pass too patch_import_record_source_indices runs again on a barrel revisit, and its path map pass matched a rewritten external whose path text equals a bundled module's key, linking the import to that module: No matching export in "virt:react-vendored" for import "default" Skip IS_EXTERNAL records there as resolve_import_records does. Queued external paths no longer need a separate pass after the map, so both kinds of pending answer are applied in one loop. The barrel test now also re-exports a bundled virtual module that shares the external's path text. --- src/bundler/bundle_v2.rs | 39 ++++++++++------------------- test/bundler/bundler_plugin.test.ts | 26 +++++++++++++------ 2 files changed, 32 insertions(+), 33 deletions(-) diff --git a/src/bundler/bundle_v2.rs b/src/bundler/bundle_v2.rs index cf8ce057ee55..4c644b812045 100644 --- a/src/bundler/bundle_v2.rs +++ b/src/bundler/bundle_v2.rs @@ -6868,29 +6868,28 @@ pub mod bv2_impl { || ctx.loader == Loader::Html || ctx.loader.is_css(); - let pending = self + if let Some(idx) = self .resolve_tasks_waiting_for_import_source_index .get_index(&ctx.source_index.get()) - .map(|idx| { - self.resolve_tasks_waiting_for_import_source_index - .swap_remove_at(idx) - .1 - }); - if let Some(pending) = &pending { + { + let (_, pending) = self + .resolve_tasks_waiting_for_import_source_index + .swap_remove_at(idx); for to_assign in pending.slice() { if let PendingImport::SourceIndex { - import_record_index, - to_source_index, + to_source_index, .. } = *to_assign { - if save_import_record_source_index - || input_file_loaders[to_source_index.get() as usize].is_css() + if !save_import_record_source_index + && !input_file_loaders[to_source_index.get() as usize].is_css() { - to_assign.apply( - &mut import_records.as_mut_slice()[import_record_index as usize], - ); + continue; } } + to_assign.apply( + &mut import_records.as_mut_slice() + [to_assign.import_record_index() as usize], + ); } } @@ -6915,18 +6914,6 @@ pub mod bv2_impl { } } } - - // After the map pass: an external path must not be looked up as a module. - if let Some(pending) = pending { - for to_assign in pending.slice() { - if matches!(to_assign, PendingImport::ExternalPath { .. }) { - to_assign.apply( - &mut import_records.as_mut_slice() - [to_assign.import_record_index() as usize], - ); - } - } - } } fn generate_server_html_module( diff --git a/test/bundler/bundler_plugin.test.ts b/test/bundler/bundler_plugin.test.ts index e4372b00aa43..7fefc65ec388 100644 --- a/test/bundler/bundler_plugin.test.ts +++ b/test/bundler/bundler_plugin.test.ts @@ -485,18 +485,21 @@ describe("bundler", () => { expect(api.readFile("/out.js")).toContain(`from "react"`); }, }); - // A barrel's records are resolved again each time a consumer un-defers one of - // them. The rewritten external must be skipped by those passes, not resolved - // as the vendored specifier. late.js is loaded only after the react answer is - // queued, so its un-defer of `a` runs after the rewrite landed. + // A barrel's records are resolved and patched again each time a consumer + // un-defers one of them. The rewritten external must survive both: it must + // not be resolved as the vendored specifier, and its path must not be matched + // against the bundled "virt" module that shares the same path text. late.js is + // loaded only after both answers are queued, so its un-defer of `a` runs after + // the rewrite landed. itBundled("plugin/ResolveExternalRewriteSurvivesBarrelRevisit", () => { const reactAnswered = Promise.withResolvers(); + const virtAnswered = Promise.withResolvers(); return { files: { "/entry.js": /* js */ ` - import { React } from "barrel"; + import { React, x } from "barrel"; import "./late.js"; - console.log(React); + console.log(React, x); `, "/late.js": ``, "/node_modules/barrel/package.json": JSON.stringify({ @@ -506,6 +509,7 @@ describe("bundler", () => { }), "/node_modules/barrel/index.js": /* js */ ` export { default as React } from "react"; + export { x } from "virt"; export { a } from "./a.js"; export { b } from "./b.js"; `, @@ -517,14 +521,22 @@ describe("bundler", () => { reactAnswered.resolve(); return { path: "react-vendored", external: true }; }); + builder.onResolve({ filter: /^virt$/ }, () => { + virtAnswered.resolve(); + return { path: "react-vendored", namespace: "virt" }; + }); + builder.onLoad({ filter: /.*/, namespace: "virt" }, () => { + return { contents: `export const x = "x";`, loader: "js" }; + }); builder.onLoad({ filter: /late\.js$/ }, async () => { - await reactAnswered.promise; + await Promise.all([reactAnswered.promise, virtAnswered.promise]); return { contents: `import { a } from "barrel"; console.log(a);`, loader: "js" }; }); }, onAfterBundle(api) { const contents = api.readFile("/out.js"); expect(contents).toContain(`from "react-vendored"`); + expect(contents).toContain(`"x"`); expect(contents).not.toContain(`"react"`); }, }; From af0a3e471ff0a6a3d88d7f6ad392e185c87de814 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 22 Aug 2026 00:29:53 +0000 Subject: [PATCH 11/12] bundler: drop IS_EXTERNAL, superseded by #39874 A barrel un-defer now resolves and patches only the un-deferred records (only_records), so neither pass reaches a record an earlier answer left external. The flag no longer guards anything, and bit 10 is the last free ImportRecordFlags bit, which #39874 chose not to take. This restores the external arm to its previous shape: an unchanged specifier is left alone, and a queued ExternalPath is applied after the path map pass so that a plugin path is never looked up as a module. The barrel test stays. It now pins that #39874's index list covers the rewritten record and the bundled module that shares its path text. --- src/ast/import_record.rs | 3 -- src/bundler/bundle_v2.rs | 96 +++++++++++++++++++++------------------- 2 files changed, 51 insertions(+), 48 deletions(-) diff --git a/src/ast/import_record.rs b/src/ast/import_record.rs index 34a6093c2759..290da2cf7f71 100644 --- a/src/ast/import_record.rs +++ b/src/ast/import_record.rs @@ -72,9 +72,6 @@ bitflags::bitflags! { const WAS_ORIGINALLY_REQUIRE = 1 << 9; - /// Resolution is done and `path` is the specifier to print. - const IS_EXTERNAL = 1 << 10; - /// If true, this import can be removed if it's unused const IS_EXTERNAL_WITHOUT_SIDE_EFFECTS = 1 << 11; diff --git a/src/bundler/bundle_v2.rs b/src/bundler/bundle_v2.rs index 4c644b812045..35740e6a9b4f 100644 --- a/src/bundler/bundle_v2.rs +++ b/src/bundler/bundle_v2.rs @@ -84,12 +84,7 @@ impl PendingImport { Self::SourceIndex { to_source_index, .. } => import_record.source_index = to_source_index, - Self::ExternalPath { path, .. } => { - import_record.path = path; - import_record - .flags - .insert(bun_ast::ImportRecordFlags::IS_EXTERNAL); - } + Self::ExternalPath { path, .. } => import_record.path = path, } } } @@ -4745,9 +4740,10 @@ pub mod bv2_impl { let mut out_source_index: Option = None; // SAFETY: `result.{path,namespace}` are `Box<[u8]>`. Every arm below // either moves both boxes into `this.free_list`, which outlives the - // graph, before it stores `path` (`!found_existing`, external), or - // drops them without storing `path` (`found_existing`, entry point). - // So the erased `'static` borrow is never read after the bytes are freed. + // graph, before it stores `path` (`!found_existing`, and the external + // rewrite), or drops them after its last read of `path` without + // storing it (`found_existing`, entry point, unchanged specifier). So + // the erased `'static` borrow is never read after the bytes are freed. let (result_path_static, result_ns_static): (&'static [u8], &'static [u8]) = unsafe { ( &*std::ptr::from_ref::<[u8]>(result.path.as_ref()), @@ -4895,6 +4891,9 @@ pub mod bv2_impl { ); drop(result.namespace); drop(result.path); + } else if strings::eql_long(&resolve.import_record.specifier, path.text, true) { + drop(result.namespace); + drop(result.path); } else { // Like esbuild, the external import is printed with the plugin's path. this.free_list.push(result.namespace); @@ -6019,11 +6018,13 @@ pub mod bv2_impl { import_record.source_index = Index::INVALID; } - estimated_resolve_queue_count += (!(import_record.flags.intersects( - bun_ast::ImportRecordFlags::IS_INTERNAL - | bun_ast::ImportRecordFlags::IS_UNUSED - | bun_ast::ImportRecordFlags::IS_EXTERNAL, - ) || import_record.source_index.is_valid())) + estimated_resolve_queue_count += (!(import_record + .flags + .contains(bun_ast::ImportRecordFlags::IS_INTERNAL) + || import_record + .flags + .contains(bun_ast::ImportRecordFlags::IS_UNUSED) + || import_record.source_index.is_valid())) as usize; } if let Some(only) = only_records { @@ -6051,7 +6052,6 @@ pub mod bv2_impl { || import_record.flags.contains(bun_ast::ImportRecordFlags::IS_INTERNAL) // Don't resolve pre-resolved imports || import_record.source_index.is_valid() - || import_record.flags.contains(bun_ast::ImportRecordFlags::IS_EXTERNAL) { continue; } @@ -6067,8 +6067,7 @@ pub mod bv2_impl { if import_record.path.text == src.path.pretty { if self.dev_server.is_some() { import_record.flags.insert( - bun_ast::ImportRecordFlags::IS_EXTERNAL - | bun_ast::ImportRecordFlags::IS_EXTERNAL_WITHOUT_SIDE_EFFECTS, + bun_ast::ImportRecordFlags::IS_EXTERNAL_WITHOUT_SIDE_EFFECTS, ); import_record.source_index = Index::INVALID; } else { @@ -6113,10 +6112,9 @@ pub mod bv2_impl { }; import_record.tag = replacement.tag; import_record.source_index = Index::INVALID; - import_record.flags.insert( - bun_ast::ImportRecordFlags::IS_EXTERNAL - | bun_ast::ImportRecordFlags::IS_EXTERNAL_WITHOUT_SIDE_EFFECTS, - ); + import_record + .flags + .insert(bun_ast::ImportRecordFlags::IS_EXTERNAL_WITHOUT_SIDE_EFFECTS); continue; } @@ -6125,10 +6123,9 @@ pub mod bv2_impl { import_record.path = bun_paths::fs::Path::init(new_text); import_record.path.namespace = b"bun"; import_record.source_index = Index::INVALID; - import_record.flags.insert( - bun_ast::ImportRecordFlags::IS_EXTERNAL - | bun_ast::ImportRecordFlags::IS_EXTERNAL_WITHOUT_SIDE_EFFECTS, - ); + import_record + .flags + .insert(bun_ast::ImportRecordFlags::IS_EXTERNAL_WITHOUT_SIDE_EFFECTS); // don't link bun continue; @@ -6137,10 +6134,9 @@ pub mod bv2_impl { // By default, we treat .sqlite files as external. if import_record.loader == Some(Loader::Sqlite) { - import_record.flags.insert( - bun_ast::ImportRecordFlags::IS_EXTERNAL - | bun_ast::ImportRecordFlags::IS_EXTERNAL_WITHOUT_SIDE_EFFECTS, - ); + import_record + .flags + .insert(bun_ast::ImportRecordFlags::IS_EXTERNAL_WITHOUT_SIDE_EFFECTS); continue; } @@ -6483,9 +6479,6 @@ pub mod bv2_impl { { import_record.path = path_as_static(&resolve_result.path_pair.primary); } - import_record - .flags - .insert(bun_ast::ImportRecordFlags::IS_EXTERNAL); import_record.flags.set( bun_ast::ImportRecordFlags::IS_EXTERNAL_WITHOUT_SIDE_EFFECTS, resolve_result.primary_side_effects_data @@ -6868,28 +6861,29 @@ pub mod bv2_impl { || ctx.loader == Loader::Html || ctx.loader.is_css(); - if let Some(idx) = self + let pending = self .resolve_tasks_waiting_for_import_source_index .get_index(&ctx.source_index.get()) - { - let (_, pending) = self - .resolve_tasks_waiting_for_import_source_index - .swap_remove_at(idx); + .map(|idx| { + self.resolve_tasks_waiting_for_import_source_index + .swap_remove_at(idx) + .1 + }); + if let Some(pending) = &pending { for to_assign in pending.slice() { if let PendingImport::SourceIndex { - to_source_index, .. + import_record_index, + to_source_index, } = *to_assign { - if !save_import_record_source_index - && !input_file_loaders[to_source_index.get() as usize].is_css() + if save_import_record_source_index + || input_file_loaders[to_source_index.get() as usize].is_css() { - continue; + to_assign.apply( + &mut import_records.as_mut_slice()[import_record_index as usize], + ); } } - to_assign.apply( - &mut import_records.as_mut_slice() - [to_assign.import_record_index() as usize], - ); } } @@ -6914,6 +6908,18 @@ pub mod bv2_impl { } } } + + // After the map pass: an external path must not be looked up as a module. + if let Some(pending) = pending { + for to_assign in pending.slice() { + if matches!(to_assign, PendingImport::ExternalPath { .. }) { + to_assign.apply( + &mut import_records.as_mut_slice() + [to_assign.import_record_index() as usize], + ); + } + } + } } fn generate_server_html_module( From 1b41dce337dc6b46c5d0bc9932e52db43d907a6e Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 22 Aug 2026 01:37:40 +0000 Subject: [PATCH 12/12] bundler: an external plugin answer clears a source_index set from the specifier The path map is keyed by path text alone, and the map pass runs over records whose plugin answer is still pending. When an earlier plugin answer bundled a module at the same text as a later import's specifier, that import got the module's source_index, and the external answer left it there. Released Bun prints var __INVALID__REF__ = __commonJS(function(exports) {}); var import_react = __toESM(, 1); for that input. The debug build asserts in print_code_for_file_in_chunk_js. PendingImport::ExternalPath::apply now resets source_index, and every external answer goes through it, including one that returns the specifier unchanged. The applied-after-the-map-pass order makes the queued case come out the same. Tests cover both answer shapes. Both fail on released Bun. --- src/bundler/bundle_v2.rs | 16 +++++------ test/bundler/bundler_plugin.test.ts | 44 +++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 8 deletions(-) diff --git a/src/bundler/bundle_v2.rs b/src/bundler/bundle_v2.rs index 35740e6a9b4f..335ad86feb51 100644 --- a/src/bundler/bundle_v2.rs +++ b/src/bundler/bundle_v2.rs @@ -84,7 +84,11 @@ impl PendingImport { Self::SourceIndex { to_source_index, .. } => import_record.source_index = to_source_index, - Self::ExternalPath { path, .. } => import_record.path = path, + Self::ExternalPath { path, .. } => { + import_record.path = path; + // The path map pass may have matched the source specifier to a module. + import_record.source_index = Index::INVALID; + } } } } @@ -4740,10 +4744,9 @@ pub mod bv2_impl { let mut out_source_index: Option = None; // SAFETY: `result.{path,namespace}` are `Box<[u8]>`. Every arm below // either moves both boxes into `this.free_list`, which outlives the - // graph, before it stores `path` (`!found_existing`, and the external - // rewrite), or drops them after its last read of `path` without - // storing it (`found_existing`, entry point, unchanged specifier). So - // the erased `'static` borrow is never read after the bytes are freed. + // graph, before it stores `path` (`!found_existing`, external), or + // drops them without storing `path` (`found_existing`, entry point). + // So the erased `'static` borrow is never read after the bytes are freed. let (result_path_static, result_ns_static): (&'static [u8], &'static [u8]) = unsafe { ( &*std::ptr::from_ref::<[u8]>(result.path.as_ref()), @@ -4891,9 +4894,6 @@ pub mod bv2_impl { ); drop(result.namespace); drop(result.path); - } else if strings::eql_long(&resolve.import_record.specifier, path.text, true) { - drop(result.namespace); - drop(result.path); } else { // Like esbuild, the external import is printed with the plugin's path. this.free_list.push(result.namespace); diff --git a/test/bundler/bundler_plugin.test.ts b/test/bundler/bundler_plugin.test.ts index 7fefc65ec388..dfe98b2bc89a 100644 --- a/test/bundler/bundler_plugin.test.ts +++ b/test/bundler/bundler_plugin.test.ts @@ -485,6 +485,50 @@ describe("bundler", () => { expect(api.readFile("/out.js")).toContain(`from "react"`); }, }); + // The path map is keyed by path text alone. Once the "virt" answer has put + // "react" in it, the map pass over late.js matches its import of "react" to the + // virt module before the plugin answers external for it. late.js is loaded + // only after the virt answer is queued, so that order is fixed. Released Bun + // keeps that match and prints `__INVALID__REF__` and `__toESM(, 1)`. + for (const [name, answer] of [ + ["WithPath", (args: { path: string }) => ({ path: args.path, external: true })], + ["WithoutPath", () => ({ external: true })], + ] as const) { + itBundled(`plugin/ResolveExternalClearsModuleMatchedBySpecifier${name}`, () => { + const virtAnswered = Promise.withResolvers(); + return { + files: { + "/entry.js": /* js */ ` + import { x } from "virt"; + import "./late.js"; + console.log(x); + `, + "/late.js": ``, + }, + plugins(builder) { + builder.onResolve({ filter: /^virt$/ }, () => { + virtAnswered.resolve(); + return { path: "react", namespace: "virt" }; + }); + builder.onLoad({ filter: /.*/, namespace: "virt" }, () => { + return { contents: `export const x = "x";`, loader: "js" }; + }); + builder.onResolve({ filter: /^react$/ }, answer); + builder.onLoad({ filter: /late\.js$/ }, async () => { + await virtAnswered.promise; + return { contents: `import React from "react"; console.log(React);`, loader: "js" }; + }); + }, + onAfterBundle(api) { + const contents = api.readFile("/out.js"); + expect(contents).toContain(`from "react"`); + expect(contents).toContain(`"x"`); + expect(contents).not.toContain(`__toESM(,`); + expect(contents).not.toContain(`__INVALID__REF__`); + }, + }; + }); + } // A barrel's records are resolved and patched again each time a consumer // un-defers one of them. The rewritten external must survive both: it must // not be resolved as the vendored specifier, and its path must not be matched