Skip to content

fix(bundler): propagate star import through barrel namespace re-exports#28173

Merged
Jarred-Sumner merged 4 commits intomainfrom
farm/226567da/fix-barrel-namespace-propagation
Mar 17, 2026
Merged

fix(bundler): propagate star import through barrel namespace re-exports#28173
Jarred-Sumner merged 4 commits intomainfrom
farm/226567da/fix-barrel-namespace-propagation

Conversation

@robobun
Copy link
Copy Markdown
Collaborator

@robobun robobun commented Mar 17, 2026

Summary

  • Fixes barrel optimization dropping exports when a barrel re-exports namespace imports (import * as X from './mod'; export { X }) with sideEffects: false
  • The BFS propagation now correctly threads alias_is_star through BarrelExportResolution, treating namespace re-exports as star imports in the target module

Root Cause

When the barrel optimization BFS encountered a re-exported namespace import like:

// utils/index.ts (sideEffects: false)
import * as typed from './arrays/typed/index.js';
export { typed };

It propagated to ./arrays/typed/index.js with is_star = false and an empty alias. The target barrel's own re-exports (e.g., export { toDataView } from './misc.js') were then incorrectly deferred because the BFS tried to find an export named "" instead of loading all exports.

Fix

Added alias_is_star to BarrelExportResolution and used it during BFS propagation. When the underlying import is import * as ns, the propagation now uses is_star = true, ensuring the target module's exports are all loaded.

Verification

USE_SYSTEM_BUN=1 bun test test/regression/issue/28170.test.ts � FAIL (toDataView is not defined)
bun bd test test/regression/issue/28170.test.ts                � PASS

Closes #28170
Closes #28137

claude and others added 3 commits March 17, 2026 01:06
When a barrel file re-exports a namespace import (`import * as X from
'./mod'; export { X }`), the barrel optimization BFS was propagating
to the target module with `is_star = false`. This caused the target's
own re-exports to be incorrectly deferred, dropping function definitions
from the bundled output.

The fix threads `alias_is_star` through `BarrelExportResolution` so the
BFS correctly treats namespace re-exports as star imports, ensuring all
exports from the target module are included.

Closes #28170
Closes #28137

https://claude.ai/code/session_0188mQLPgSvNV2rVAqDh42mt
@robobun
Copy link
Copy Markdown
Collaborator Author

robobun commented Mar 17, 2026

Updated 8:42 PM PT - Mar 16th, 2026

@robobun, your commit 5208312 has 1 failures in Build #39772 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 28173

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

bun-28173 --bun

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 17, 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: a3408d25-fef6-4a49-955e-038eafd264b3

📥 Commits

Reviewing files that changed from the base of the PR and between 165bc69 and 5208312.

📒 Files selected for processing (1)
  • test/regression/issue/28170.test.ts

Walkthrough

Added alias_is_star field to BarrelExportResolution to track namespace (star) imports and propagate them correctly during barrel export resolution. Added a regression test that verifies namespace re-export propagation across a multi-package monorepo-style workspace.

Changes

Cohort / File(s) Summary
Barrel imports core implementation
src/bundler/barrel_imports.zig
Added alias_is_star: bool to BarrelExportResolution. resolveBarrelExport now sets this field from import entries. scheduleBarrelDeferredImports uses the flag to enqueue propagated exports with correct is_star behavior for namespace re-exports.
Regression test
test/regression/issue/28170.test.ts
New test that builds a three-package workspace with barrel and namespace re-exports, bundles the app, runs the output, and asserts console output and exit code to validate star-import propagation across packages.
🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely summarizes the main change: fixing barrel optimization to propagate star imports through namespace re-exports, which is the core issue addressed by the PR.
Description check ✅ Passed The PR description provides comprehensive context including summary, root cause analysis, the fix explanation, and verification steps, though it doesn't follow the template's required section headings exactly.

✏️ 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/regression/issue/28170.test.ts`:
- Around line 91-93: The test currently asserts installExit === 0 but doesn't
surface captured logs on failure; update the failure path around the
installProc/installExit check (and the similar block at lines 112-119) to log
the captured process output before asserting: await the stored streams
(installProc.stderr.text() and installProc.stdout.text()) and print them (e.g.,
console.error) if installExit is non-zero so CI failures show the stderr/stdout;
do the same for the corresponding process variables in the later block (the
build/install process variables used in lines 112-119) before calling
expect(...).toBe(0).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: d767914e-a1e9-47cd-8e3c-207e8220c5cc

📥 Commits

Reviewing files that changed from the base of the PR and between 31e0c9c and 165bc69.

📒 Files selected for processing (2)
  • src/bundler/barrel_imports.zig
  • test/regression/issue/28170.test.ts

Comment thread test/regression/issue/28170.test.ts Outdated
@github-actions
Copy link
Copy Markdown
Contributor

Found 3 issues this PR may fix:

  1. Barrel exports can be undefined in dev & prod builds #28166 - Barrel exports can be undefined in dev & prod builds (directly involves barrel exports becoming undefined with namespace imports, matching the core issue this PR fixes)
  2. Bun build does not import values re-exported from another package when sideEffects is false #18008 - Bun build does not import values re-exported from another package when sideEffects is false (specifically involves re-exported values being dropped when sideEffects: false, which is the exact scenario this PR addresses)
  3. Re-export before any import causes referencing an undefined variable #15997 - Re-export before any import causes referencing an undefined variable (involves re-export ordering issues that may be related to barrel optimization behavior)

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

🤖 Generated with Claude Code

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 — small, well-targeted fix threading alias_is_star through BarrelExportResolution with a good regression test.

Extended reasoning...

Overview

This PR fixes a bug where the barrel optimization BFS incorrectly hardcoded is_star = false when propagating through re-exported namespace imports (import * as X from "./mod"; export { X }). The fix adds alias_is_star: bool to the BarrelExportResolution struct, populates it from the existing import_entry.alias_is_star field in resolveBarrelExport, and uses it in the BFS propagation step instead of the hardcoded false. This is a 3-line production code change. A comprehensive regression test is included that sets up a workspace with barrel files using namespace re-exports and verifies the bundled output.

Security risks

None. This is a bundler optimization fix that corrects which exports are loaded during barrel optimization BFS. No auth, crypto, permissions, or user-facing input handling is involved.

Level of scrutiny

Low scrutiny is appropriate. The change is mechanical — it threads an existing boolean field through an intermediate struct so the BFS correctly identifies star imports. The field alias_is_star already exists on NamedImport (defined in src/ast.zig:589) and is already used elsewhere in the codebase (e.g., ImportScanner.zig, LinkerContext.zig). No CODEOWNERS are affected.

Other factors

  • The regression test comprehensively covers the fix scenario with a multi-package workspace setup that exercises the exact barrel re-export pattern that was broken.
  • The only bug found by automated review is a minor nit about asserting stderr before exit code in the test, which is a diagnostic improvement, not a correctness issue.
  • The PR closes two reported issues (#28170 and #28137).

Comment thread test/regression/issue/28170.test.ts Outdated
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 — small, well-scoped fix threading an existing boolean through barrel BFS propagation, with a solid regression test.

Extended reasoning...

Overview

This PR touches two files: src/bundler/barrel_imports.zig (the core fix) and test/regression/issue/28170.test.ts (a new regression test). The Zig change adds an alias_is_star: bool field to the internal BarrelExportResolution struct, populates it from the already-existing import_entry.alias_is_star field in resolveBarrelExport, and uses it at the single BFS propagation point where the value was previously hardcoded to false. This is a 3-line struct addition and a 1-line behavioral fix.

Security risks

None. This is an internal bundler optimization path — no user input handling, no auth, no network, no file system access beyond what the bundler already does.

Level of scrutiny

The change is small and mechanical: it threads an existing boolean through one intermediate struct. The alias_is_star field is already well-established in the codebase (src/ast.zig:589, ImportScanner.zig, LinkerContext.zig, and already used in barrel_imports.zig lines 283 and 351 for initial BFS seeding). The fix is a natural extension of the existing pattern. The regression test creates a realistic monorepo workspace setup and verifies the bundled output executes correctly.

Other factors

  • Previous review nits (from coderabbitai and my prior run) about surfacing install/build errors in test assertions have been addressed (commit 5208312).
  • All inline comments are resolved.
  • The PR fixes real user-reported issues (#28170, #28137) with clear root cause analysis.
  • No CODEOWNERS concerns for these paths.
  • The test follows established patterns in the test suite.

@Jarred-Sumner Jarred-Sumner merged commit 47826b3 into main Mar 17, 2026
63 of 65 checks passed
@Jarred-Sumner Jarred-Sumner deleted the farm/226567da/fix-barrel-namespace-propagation branch March 17, 2026 06:27
structwafel pushed a commit to structwafel/bun that referenced this pull request Apr 25, 2026
…ts (oven-sh#28173)

## Summary

- Fixes barrel optimization dropping exports when a barrel re-exports
namespace imports (`import * as X from './mod'; export { X }`) with
`sideEffects: false`
- The BFS propagation now correctly threads `alias_is_star` through
`BarrelExportResolution`, treating namespace re-exports as star imports
in the target module

## Root Cause

When the barrel optimization BFS encountered a re-exported namespace
import like:

```ts
// utils/index.ts (sideEffects: false)
import * as typed from './arrays/typed/index.js';
export { typed };
```

It propagated to `./arrays/typed/index.js` with `is_star = false` and an
empty alias. The target barrel's own re-exports (e.g., `export {
toDataView } from './misc.js'`) were then incorrectly deferred because
the BFS tried to find an export named `""` instead of loading all
exports.

## Fix

Added `alias_is_star` to `BarrelExportResolution` and used it during BFS
propagation. When the underlying import is `import * as ns`, the
propagation now uses `is_star = true`, ensuring the target module's
exports are all loaded.

## Verification

```
USE_SYSTEM_BUN=1 bun test test/regression/issue/28170.test.ts � FAIL (toDataView is not defined)
bun bd test test/regression/issue/28170.test.ts                � PASS
```

Closes oven-sh#28170
Closes oven-sh#28137

---------

Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

4 participants