[robobun]

Problem

• With onResolve({ filter: /.*/ }, () => undefined), Bun.build tree-shakes differently than without the plugin. An unused file from a package with "sideEffects": false stays in the bundle and runs.
• The cause is run_resolver in src/bundler/bundle_v2.rs. It drops the resolver's primary_side_effects_data in two places. enqueue_parse_task stores loader.side_effects() (line 3731 on main). The early return for an external import (line 2590) does not set IS_EXTERNAL_WITHOUT_SIDE_EFFECTS.

Fix

• Both places now store the resolver's value, as the path without plugins does (bundle_v2.rs:2754, bundle_v2.rs:6652 and ParseTask::init).
• This also fixes two cases. A data file that onLoad turns into JavaScript keeps its side effects. An unused import of a Node.js builtin is dropped.
• Verified: four new cases in test/bundler/bundler_plugin.test.ts fail on bun 1.4.1 and pass with this change. Other suites: see Notes.
• Self-reviewed. I fixed the external-import gap, and I removed the data-loader special case because it drops code. The jsx overwrite in enqueue_parse_task is a different field (see Notes).

Background

• The resolver reads the "sideEffects" field of the nearest package.json and stores the result in primary_side_effects_data. It also marks a Node.js builtin as free of side effects.
• The linker reads one side-effects value for each file (LinkerContext::file_has_no_side_effects). If a file has no side effects, an import with no used bindings does not keep the file.
• If every onResolve callback returns nothing for a file import, the bundler resolves it in run_resolver (ResolveValue::NoMatch). That function is a separate copy of resolve_import_records.

Notes

Data files (JSON, text, TOML):

• Before this change, this path marked a data file as pure data. It took that value from the loader for the file extension, before onLoad ran. So when onLoad returned JavaScript, the bundle dropped that code. The third test covers this. The path without plugins does not have this special case.
• Now a data file gets the same value as in a build without plugins. So a build with a pass-through plugin can get larger output. Example: entry a.js uses data.json, and entry b.js imports it without using it. Then b.js also gets the JSON. A build without plugins does this today. esbuild marks data files as pure data after onLoad. A separate change can do that for both paths.
• I compared the output with and without a pass-through plugin for JSON, text and TOML imports (with and without splitting), the original repro, builtins with target node and bun, and the "sideEffects" array form. The output is the same in each case.

Not in this PR (other values on the same path):

• enqueue_parse_task replaces the tsconfig jsx value from the resolver with the transpiler's value (line 3745 on main). A fix must keep the transpiler's value for files: entries, as resolve_import_records does.
• The early return for an external import also skips the ExternalRewritePath path rewrite.
• bundler: resolve files imports through the shared steps (HTML import manifest, url assets) #38605 and bundler: copy HTML url assets resolved through files: or an onResolve plugin #38635 cover HTML imports on this path.
• A larger change can send this path through resolve_import_records and remove run_resolver.

Tests:

• The first test imports the two files directly. With "sideEffects": false, a re-export barrel does not show the bug: the barrel pass reads the resolver's value from the ParseTask, so it never loads the unused file. The array form does not use the barrel pass, so the second test uses a barrel.
• Suites run with the debug build: bundler_plugin, esbuild/dce, bundler_barrel, bundler_plugin_chain, bundler_files, bundler_loader, bundler_splitting, bun-build-api, bake/dev/plugins, bake/dev/css, bun-serve-html, and the regression tests for Tree-shaking drops a still-referenced declaration when onResolve returns undefined #40606, segmentation fault when build.onResolve returns undefined in a Bun.plugin  #22199 and Bun segfaults when there are both external and unresolved/missing non-external imports #29264.

Related:

• Bun.build plugin onResolve treeshaking issue #2952 reported this bug with async () => null. It still reproduces on 1.4.1.
• bundler: make optimizeImports tree-shake mixed barrels #31147 has the enqueue_parse_task hunk, together with an optimizeImports change. A review of that PR asked for this fix as a separate PR.
• The repro of sideEffects: false + catch-all onResolve plugin causes live bindings to be tree-shaken from namespace exports #29445 passes on 1.4.1 and on this branch.

---

no test proof · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/bundler/bundler_plugin.test.ts

[robobun]

Status

Reproduced on bun 1.4.1 (canary e0a2b82fd):

• node_modules/demo-pkg/package.json is { "sideEffects": false }. unused.js logs a line and exports a value.
• entry.js imports used and unused from the package, and uses only used.
• Bun.build without plugins: the bundle does not contain unused.js.
• Bun.build with onResolve({ filter: /.*/ }, () => undefined): the bundle contains unused.js, and its log line runs.

The four new cases in test/bundler/bundler_plugin.test.ts fail on 1.4.1 and pass on this branch with a debug build.

[robobun]

^{Updated 10:12 AM PT - Sep 4th, 2026}

✅ @robobun, your commit 58223df513c58350c912045129ac8c20f01dd32c passed in Build #110188! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 41380

That installs a local version of the PR into your bun-41380 executable, so you can run:

bun-41380 --bun

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Essentials

Run ID: 69e176da-5678-4784-ba55-dfde0a849913

📥 Commits

Reviewing files that changed from the base of the PR and between d296efb and 58223df.

📒 Files selected for processing (2)

• src/bundler/bundle_v2.rs
• test/bundler/bundler_plugin.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 6 remain after this review.

---

Walkthrough

The bundler now uses resolver-provided side-effects metadata for external imports and parsed input files. New plugin tests cover undefined onResolve results, custom loaders, package sideEffects data, and unused Node.js builtin imports.

Changes

Resolver side-effects handling

Layer / File(s)	Summary
Propagate resolver side-effects metadata src/bundler/bundle_v2.rs	External imports and parsed input files now use resolver-provided side-effects metadata.
Validate delegated plugin resolution test/bundler/bundler_plugin.test.ts	Tests cover undefined onResolve results with package side-effects data, custom onLoad loaders, and unused Node.js builtin imports.

Suggested reviewers: jarred-sumner, dylan-conway, alii

Merge Risk: ⚪ Minimal · up to 58223

This change restores correct tree-shaking behavior while retaining side effects when loaders transform files into JavaScript. The affected resolver paths are covered by targeted bundler regression cases, with no remaining merge-blocking risk identified.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly and concisely describes the main change: preserving resolver side-effects metadata when onResolve returns nothing.
Description check	✅ Passed	The description fully explains the problem, fix, impact, and verification. It does not use the template headings exactly, but it provides the required information in equivalent sections.

---

_{Comment @coderabbitai help to get the list of available commands.}

[claude]

LGTM — brings the plugin fallback path in line with the non-plugin resolve path.

What was reviewed:

• Both hunks mirror the sibling non-plugin path exactly: the external-import flag set at line 2594-2598 matches resolve_import_records at 6659-6663, and side_effects at 3739 matches the entry-point append at 2762.
• Checked that enqueue_parse_task's copy-for-bundling branch (3768) still overwrites to NoSideEffectsPureData after this change, so non-onLoad data files keep their prior behavior.
• Tests cover both changed sites (false / array / onLoad-loader-swap / builtin-external) and are placed in the existing bundler_plugin.test.ts per convention; no CODEOWNERS cover these paths.

Extended reasoning...

Overview

This PR fixes side-effects tracking in the bundler when a plugin's onResolve returns undefined and the import falls through to run_resolver. Two spots in src/bundler/bundle_v2.rs are changed: the external-import early return now sets IS_EXTERNAL_WITHOUT_SIDE_EFFECTS from resolve_result.primary_side_effects_data, and enqueue_parse_task now stores resolve_result.primary_side_effects_data on the input file instead of loader.side_effects(). Four itBundled tests are added to test/bundler/bundler_plugin.test.ts.

Security risks

None. This is tree-shaking metadata propagation inside the bundler; no parsing of untrusted input, no auth/crypto/permissions surface, no new external I/O.

Level of scrutiny

Low-to-moderate. The change is two lines of logic, and each is a byte-for-byte copy of the corresponding line in the non-plugin sibling path (resolve_import_records at 6659-6663 and the entry-point input_files.append at 2754-2762). REVIEW.md's "fix the whole class" guidance is satisfied — the PR author explicitly enumerated the remaining divergences on this path (jsx overwrite, ExternalRewritePath, HTML imports) and scoped them out with reasons, which is the required treatment. The loader.side_effects() → primary_side_effects_data swap could have regressed pure-data files, but the copy-for-bundling branch immediately below (line 3768) still overwrites to NoSideEffectsPureData when no onLoad plugin fires, and the third test proves the onLoad case now correctly retains side effects.

Other factors

Tests are in the right file, use itBundled, and cover a solid variant matrix: sideEffects: false, sideEffects: [array] via a barrel (deliberately chosen because the barrel pass would mask the bug for false), an onLoad loader swap, and an external Node builtin with target: "node". The PR description states all four fail on 1.4.1 and pass on the branch. No CODEOWNERS entries cover src/bundler/ or test/bundler/. Exit reason was dry_streak with no candidate findings. No prior reviewer activity on the timeline.

[robobun]

#45408 covers this. A declined import now goes through resolve_import_records, which keeps the side effects that the resolver reports. The cases of this PR are in the tests of #45408. This PR can close when #45408 merges.

[robobun]

Closing in favor of #45408. It sends a declined import back through resolve_import_records, which keeps the side effects that the resolver reports, and it contains the cases of this PR.