From 58223df513c58350c912045129ac8c20f01dd32c Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 4 Sep 2026 11:19:16 +0000 Subject: [PATCH] bundler: keep the resolver's side effects when onResolve returns nothing When every onResolve callback returns nothing, the bundler resolves the import in run_resolver. That path dropped the resolver's side-effects value in two places: - enqueue_parse_task stored loader.side_effects() for the file. So an unused file from a package with "sideEffects": false stayed in the bundle. Also, a data file that an onLoad plugin turned into JavaScript lost its side effects. - The early return for an external import did not set IS_EXTERNAL_WITHOUT_SIDE_EFFECTS. So an unused import of a Node.js builtin stayed in the output. Both places now store the resolver's value, as the path without plugins does. --- src/bundler/bundle_v2.rs | 10 +++- test/bundler/bundler_plugin.test.ts | 89 +++++++++++++++++++++++++++++ 2 files changed, 98 insertions(+), 1 deletion(-) diff --git a/src/bundler/bundle_v2.rs b/src/bundler/bundle_v2.rs index 692acf621f68..32e0af52426e 100644 --- a/src/bundler/bundle_v2.rs +++ b/src/bundler/bundle_v2.rs @@ -2588,6 +2588,14 @@ pub mod bv2_impl { }; if resolve_result.flags.is_external() { + let record: &mut ImportRecord = &mut self.graph.ast.items_import_records_mut() + [import_record.importer_source_index as usize] + .as_mut_slice()[import_record.import_record_index as usize]; + record.flags.set( + bun_ast::ImportRecordFlags::IS_EXTERNAL_WITHOUT_SIDE_EFFECTS, + resolve_result.primary_side_effects_data + != bun_ast::SideEffects::HasSideEffects, + ); return; } @@ -3728,7 +3736,7 @@ pub mod bv2_impl { self.graph.input_files.append(crate::Graph::InputFile { source: core::mem::take(source), loader, - side_effects: loader.side_effects(), + side_effects: resolve_result.primary_side_effects_data, ..Default::default() })?; // `ParseTask::init` takes `bun_ast::Index`; both Index newtypes diff --git a/test/bundler/bundler_plugin.test.ts b/test/bundler/bundler_plugin.test.ts index e9312694e2b2..2d0e83aeac46 100644 --- a/test/bundler/bundler_plugin.test.ts +++ b/test/bundler/bundler_plugin.test.ts @@ -189,6 +189,95 @@ describe("bundler", () => { }); } + // A plugin that returns nothing hands the import back to the resolver. The + // bundle must keep the same side effects as a build without the plugin. + itBundled("plugin/ResolveReturnsUndefinedKeepsSideEffectsFalse", { + files: { + "/entry.js": /* js */ ` + import { used } from "demo-pkg/used.js"; + import { unused } from "demo-pkg/unused.js"; + console.log(used); + `, + "/node_modules/demo-pkg/package.json": /* json */ `{ "sideEffects": false }`, + "/node_modules/demo-pkg/used.js": /* js */ `export const used = "used";`, + "/node_modules/demo-pkg/unused.js": /* js */ ` + console.log("unused.js ran"); + export const unused = 1; + `, + }, + plugins(builder) { + builder.onResolve({ filter: /.*/ }, () => undefined); + }, + run: { + stdout: "used", + }, + }); + + itBundled("plugin/ResolveReturnsUndefinedKeepsSideEffectsArray", { + files: { + "/entry.js": /* js */ ` + import "demo-pkg/register.js"; + import { used } from "demo-pkg"; + console.log(used); + `, + "/node_modules/demo-pkg/package.json": /* json */ `{ "sideEffects": ["./register.js"] }`, + "/node_modules/demo-pkg/index.js": /* js */ ` + export { used } from "./used.js"; + export { unused } from "./unused.js"; + `, + "/node_modules/demo-pkg/register.js": /* js */ `console.log("register.js ran");`, + "/node_modules/demo-pkg/used.js": /* js */ `export const used = "used";`, + "/node_modules/demo-pkg/unused.js": /* js */ ` + console.log("unused.js ran"); + export const unused = 1; + `, + }, + plugins(builder) { + builder.onResolve({ filter: /.*/ }, () => undefined); + }, + run: { + stdout: "register.js ran\nused", + }, + }); + + // The loader that onLoad returns replaces the loader from the file extension. + itBundled("plugin/ResolveReturnsUndefinedOnLoadToJsKeepsSideEffects", { + files: { + "/entry.js": /* js */ ` + import "./setup.txt"; + console.log("entry"); + `, + "/setup.txt": `console.log("setup.txt ran");`, + }, + plugins(builder) { + builder.onResolve({ filter: /.*/ }, () => undefined); + builder.onLoad({ filter: /\.txt$/ }, async args => ({ + contents: await Bun.file(args.path).text(), + loader: "js", + })); + }, + run: { + stdout: "setup.txt ran\nentry", + }, + }); + + // An import of a Node.js builtin has no side effects, so an unused one is dropped. + itBundled("plugin/ResolveReturnsUndefinedDropsUnusedBuiltinImport", { + files: { + "/entry.js": /* js */ ` + import fs from "node:fs"; + console.log("entry"); + `, + }, + target: "node", + plugins(builder) { + builder.onResolve({ filter: /.*/ }, () => undefined); + }, + onAfterBundle(api) { + api.expectFile("/out.js").not.toContain("node:fs"); + }, + }); + // Load Plugin Errors itBundled("plugin/ResolveThrow", { files: resolveFixture,