Skip to content

fix(bundler): barrel optimization defers export * as Name targets#28167

Open
robobun wants to merge 5 commits intomainfrom
claude/fix-barrel-export-star-as-28166
Open

fix(bundler): barrel optimization defers export * as Name targets#28167
robobun wants to merge 5 commits intomainfrom
claude/fix-barrel-export-star-as-28166

Conversation

@robobun
Copy link
Copy Markdown
Collaborator

@robobun robobun commented Mar 16, 2026

Summary

  • Fix barrel optimization incorrectly deferring exports from export * as Name from '...' targets in the dev server
  • When the BFS propagation encountered a namespace re-export (export * as Toast from './namespace'), it propagated the alias ("Toast") as a named export request to the target module, but the target exports individual members (Root, Title), not "Toast" — so all exports were deferred as unused
  • Track alias_is_star in BarrelExportResolution and propagate as a star import when the resolution is a namespace re-export

Test plan

  • Added dev server regression test barrel optimization: export * as Name through nested barrels (#28166) mirroring the Chakra UI / Ark UI package structure
  • Added production bundler regression test barrel/ExportStarAsNameNestedBarrels with identical fixture
  • All 6 barrel optimization dev server tests pass
  • All 18 bundle.test.ts tests pass
  • All production bundler barrel tests pass

Closes #28166

🤖 Generated with Claude Code

When the barrel optimizer's BFS encountered `export * as Name from './file'`,
it propagated the alias (e.g. "Toast") as a named export request to the target
module. But the target exports individual members (Root, Title), not "Toast",
so all its exports were incorrectly deferred as unused.

Track `alias_is_star` in `BarrelExportResolution` and propagate as a star
import when the export resolves to a namespace re-export, ensuring the target
module's exports are all preserved.

Closes #28166

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@robobun
Copy link
Copy Markdown
Collaborator Author

robobun commented Mar 16, 2026

Updated 12:17 AM PT - Mar 17th, 2026

@Jarred-Sumner, your commit 39a6035 has 2 failures in Build #39780 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 28167

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

bun-28167 --bun

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 16, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 2d8dc887-f54e-4a89-9b1d-efaa7ac6d926

📥 Commits

Reviewing files that changed from the base of the PR and between ce5e8d0 and 0f930e2.

📒 Files selected for processing (2)
  • test/bake/dev/bundle.test.ts
  • test/bundler/bundler_barrel.test.ts

Walkthrough

Adds tracking for export * as Name through barrel resolution by introducing an alias_is_star flag, updating resolution/optimization logic to avoid deferring star-aliased exports, and adding tests that validate cross-package and barrel-chain namespace re-exports.

Changes

Cohort / File(s) Summary
Barrel export logic
src/bundler/barrel_imports.zig
Added alias_is_star: bool to BarrelExportResolution; resolveBarrelExport now returns alias_is_star; applyBarrelOptimizationImpl marks star-aliased resolutions needed and bails out to avoid deferral; scheduleBarrelDeferredImports propagates star semantics and queues deferred imports with is_star.
Dev bundle test
test/bake/dev/bundle.test.ts
Added dev test "barrel optimization: export * as Name cross-package" that constructs cross-package barrels re-exporting via export * as and asserts runtime output for namespace-imported values.
Bundler unit test
test/bundler/bundler_barrel.test.ts
Added unit test barrel/ExportStarAsNameCrossPackage to validate propagation of export * as Name through nested barrels across packages and verify correct resolution at bundle/runtime.

Possibly related PRs

  • oven-sh/bun PR 27524: changes to export-* propagation in barrel optimization relevant to star-export deferral fixes.
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main fix: barrel optimization incorrectly deferring export * as Name targets, which is the core issue addressed in the PR.
Description check ✅ Passed The PR description covers the required sections: what the PR does (barrel optimization fix for export * as Name), verification methods (dev and production tests), and all test results mentioned.
Linked Issues check ✅ Passed The PR fully addresses issue #28166 by fixing the barrel optimization logic to correctly handle export * as Name targets through the alias_is_star tracking and propagation mechanism, with comprehensive regression tests.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing the barrel optimization issue: core logic changes in barrel_imports.zig and corresponding test coverage for the specific fix.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

📝 Coding Plan
  • Generate coding plan for human review comments

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@test/bake/dev/bundle.test.ts`:
- Around line 616-668: The test case named in devTest("barrel optimization:
export * as Name through nested barrels (`#28166`)") only covers the dev server;
add a production/bundler twin that uses the production test helper (e.g.
prodTest or bundleTest matching your test harness) with the identical files
fixture (same nested outer-lib/inner-lib structure and module files) and the
same assertion (expecting "result: ROOT TITLE") to verify the production build
path also preserves the namespace re-exports; place it next to the existing
devTest and use the same test body (build/serve the bundle and verify output) so
the regression is guarded in both dev and prod.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 4b3ebaf8-bc8b-4a38-aa75-0206ef2ea571

📥 Commits

Reviewing files that changed from the base of the PR and between d50ab98 and 186ee2c.

📒 Files selected for processing (2)
  • src/bundler/barrel_imports.zig
  • test/bake/dev/bundle.test.ts

Comment thread test/bake/dev/bundle.test.ts Outdated
@github-actions
Copy link
Copy Markdown
Contributor

Found 5 issues this PR may fix:

  1. bun build --target bun and bun build --compile can emit a re-exported namespace binding without the underlying definition #28137 - Re-exported namespace binding without underlying definition, directly matches the barrel optimization fix
  2. Bun build does not import values re-exported from another package when sideEffects is false #18008 - Re-exported values with sideEffects false not imported, related to barrel deferring logic
  3. Re-export before any import causes referencing an undefined variable #15997 - Re-export before import causes undefined variable, matches star export handling issues
  4. sideEffects field breaks bundler tree-shaking in Bun 1.3.10 #27709 - sideEffects breaks tree-shaking, potentially related to barrel optimization interactions
  5. bun build generates undefined variable when bundling @swc/wasm-typescript #14939 - Undefined variable in bundling output, could be related to export resolution problems

If this is helpful, consider adding 'Fixes #' to the PR description to auto-close the issue on merge.

🤖 Generated with Claude Code

@Jarred-Sumner
Copy link
Copy Markdown
Collaborator

Can you come up with a test that fails in the regular itBundled tests? If not and this bug is devServer-only, then can we limit the de-optimization here to only the devServer?

Copy link
Copy Markdown
Contributor

@claude claude Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — focused fix correctly propagating namespace re-exports as star imports through barrel optimization.

Extended reasoning...

Overview

This PR fixes a bug in the barrel optimization where export * as Name from "..." targets were incorrectly deferred. Two files are changed: src/bundler/barrel_imports.zig (the fix) and test/bake/dev/bundle.test.ts (regression test). The fix adds an alias_is_star boolean to BarrelExportResolution, then uses it in two places: (1) early-return in applyBarrelOptimizationImpl to skip barrel deferral entirely when a namespace re-export is requested, and (2) propagate as a star import in scheduleBarrelDeferredImports BFS so the target module keeps all its exports.

Security risks

No security implications — this is bundler optimization logic that affects which submodules are lazily loaded during dev server builds. No auth, crypto, or permission-related code is touched.

Level of scrutiny

Moderate scrutiny is appropriate. The barrel optimization is non-trivial logic, but this change is small (adding one boolean field and two conditional checks), follows existing patterns (alias_is_star is already used elsewhere in the codebase at LinkerContext.zig and ImportScanner.zig), and the logic is straightforward: namespace re-exports need all exports from the target, not just the alias name.

Other factors

The regression test is well-structured, mirroring a real-world Chakra UI / Ark UI package structure with nested barrels and namespace re-exports. The PR description notes all existing barrel optimization tests continue to pass. No outstanding reviewer comments exist.

@bensaufley
Copy link
Copy Markdown

FYI @Jarred-Sumner it is not dev-only; more info in the related issue #28166

Add a production bundler twin of the dev server test to guard against
regressions in both code paths.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@test/bundler/bundler_barrel.test.ts`:
- Around line 1597-1648: Add a runtime execution check to the
"barrel/ExportStarAsNameNestedBarrels" test: enable the test runner's run option
(or set run: true) so the generated bundle executes, then assert the process
stdout contains the expected runtime output "ROOT TITLE" produced by /entry.js's
console.log (in addition to the existing onAfterBundle static checks for
ToastRoot/ToastTitle).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 2d419d05-afc5-4d32-af11-5d1f25664dd0

📥 Commits

Reviewing files that changed from the base of the PR and between 186ee2c and d75b24c.

📒 Files selected for processing (1)
  • test/bundler/bundler_barrel.test.ts

Comment thread test/bundler/bundler_barrel.test.ts Outdated
Copy link
Copy Markdown
Collaborator

@Jarred-Sumner Jarred-Sumner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neither of the tests fail on main. No proof this PR fixes the issue.

@alii
Copy link
Copy Markdown
Member

alii commented Mar 16, 2026

@robobun Read the comments and adopt this PR

@robobun
Copy link
Copy Markdown
Collaborator Author

robobun commented Mar 16, 2026

Verification — 0f930e2 (push 3)

Gate 1 (CI): ⏳ Buildkite #39767 running on 0f930e2.

Gate 2 (Classification): Bug fix → test proof required.

Gate 3 (Test proof): ⏳ In progress.

  • Main binary FAILS (good — proves the bug):
    USE_SYSTEM_BUN=1 /tmp/bun-debug-main test test/bundler/bundler_barrel.test.ts -t "ExportStarAsNameCrossPackage"
    → ReferenceError: toDataView is not defined (barrel optimization defers typed/index.js)
    
  • ⏳ Building PR binary to verify fix passes...

Gate 4 (Diff): Pending re-check after test proof.

Gate 5 (Bot convergence): All prior threads resolved. Waiting for bots on new head.

Status: 🔨 Building PR binary for test proof step 3.

Comment thread test/bundler/bundler_barrel.test.ts Outdated
Comment thread src/bundler/barrel_imports.zig
Copy link
Copy Markdown
Collaborator Author

@robobun robobun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gate 3 (Test proof): FAIL

Both new tests pass on the baked main binary (d50ab984, v1.3.11-debug), which means the tests don't reproduce the bug.

Root cause: The test fixture uses export * from './components/index.js' in outer-lib/index.js. This marks components/index.js as is_export_star_target = true (barrel_imports.zig:68), causing the barrel optimization to be skipped entirely on that module. The buggy code path — where alias_is_star propagation is incorrect — is never exercised.

Required changes:

  1. Test fixture: Change outer-lib/index.js from export * from './components/index.js' to named re-exports like export { Toast } from './components/index.js' (or add more barrel layers), so components/index.js is NOT an export star target and the barrel optimization actually runs on it.
  2. Production test: Add run: { stdout: "ROOT TITLE" } to the itBundled test for runtime verification.
  3. Comment fix: Reword barrel_imports.zig:104 comment — says "Mark all non-internal records as needed" but the mechanism is an early return that skips the unused-marking loop.

Verify after fixing: the new test must FAIL on the baked binary (USE_SYSTEM_BUN=1 build/debug/bun-debug test ...) and PASS on the rebuilt PR binary.

claude and others added 3 commits March 16, 2026 22:19
Change outer-lib/index.js from `export * from` to `export { Toast } from`
so that components/index.js is not marked as is_export_star_target, allowing
barrel optimization to actually run on it. Add runtime verification to the
production bundler test. Reword comment in barrel_imports.zig for accuracy.

https://claude.ai/code/session_01EnB4DmogoHrPTjPA617dJd
…l bug

The previous test fixture imported the namespace directly from the
barrel entry, which allowed Phase 1 seeding in scheduleBarrelDeferredImports
to correctly mark the target as needing all exports. The bug only manifests
when a DIFFERENT package (codec) imports the namespace from the barrel
(utils), causing the barrel optimization to incorrectly defer the namespace
target's sub-modules.

New fixture: entry → codec (uses typed.toDataView) → utils barrel
(export * as typed). On the buggy code path, barrel optimization for
the utils barrel defers typed/index.js, so toDataView is never defined
in the bundle output (ReferenceError at runtime).

Test proof:
  USE_SYSTEM_BUN=1 bun test → FAILS (ReferenceError: toDataView is not defined)
  bun bd test              → PASSES (2 8)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Barrel exports can be undefined in dev & prod builds

5 participants