Skip to content

Commit eec7d73

Browse files
shulaodashulaoda
authored
fix(lazy-barrel): load import-then-export specifiers when barrel has local exports (#8895)
 ## Summary Fixes #8799 When a barrel module uses the `import { a } from './a'; export { a }` pattern (import-then-export) alongside a local export like `export default b`, importing the local export requires the barrel to execute. However, the lazy barrel optimization incorrectly treated the shared import record for `./a` the same as a dedicated re-export record (`export { a } from './a'`), giving it an empty specifier set. This caused `a` to not be properly loaded, breaking the barrel's execution. The fix introduces an `IsReExportOnly` flag on `ImportRecordMeta` to distinguish dedicated re-export records (`export { .. } from '..'`, `export * as ns from '..'`) from shared import records (`import { a } from '..'; export { a }`). When a barrel has local exports and needs to execute, only dedicated re-export records use empty specifiers — shared import records are loaded with their full specifiers since we cannot statically determine whether their bindings are used by the barrel's own code.
1 parent 1226947 commit eec7d73

File tree

10 files changed

+84
-7
lines changed

10 files changed

+84
-7
lines changed

‎crates/rolldown/src/ast_scanner/mod.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -665,6 +665,7 @@ impl<'me, 'ast: 'me> AstScanner<'me, 'ast> {
665665
if let Some(exported) = &decl.exported {
666666
// export * as ns from '...'
667667
self.add_star_re_export(exported.name().as_str(), id, decl.span);
668+
self.result.import_records[id].meta.insert(ImportRecordMeta::IsReExportOnly);
668669
} else {
669670
// export * from '...'
670671
self.result.import_records[id].meta.insert(ImportRecordMeta::IsExportStar);
@@ -717,6 +718,7 @@ impl<'me, 'ast: 'me> AstScanner<'me, 'ast> {
717718
.insert(record_idx, ImportAttribute::from_with_clause(with_clause));
718719
}
719720
self.result.imports.insert(decl.span, record_idx);
721+
self.result.import_records[record_idx].meta.insert(ImportRecordMeta::IsReExportOnly);
720722
} else {
721723
decl.specifiers.iter().for_each(|spec| {
722724
if let Some(local_symbol_id) = self.get_root_binding(spec.local.name().as_str()) {

‎crates/rolldown_common/src/types/import_record.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,9 @@ bitflags::bitflags! {
7272
/// If a record is a re-export-all from an external module, and that re-export-all chain continues uninterrupted to the entry point,
7373
/// we can reuse the original re-export-all declaration instead of generating complex interoperability code.
7474
const EntryLevelExternal = 1 << 9;
75+
/// The import record is solely for re-export purposes, created by
76+
/// `export { .. } from '..'` or `export * as ns from '..'`
77+
const IsReExportOnly = 1 << 10;
7578

7679
const TopLevelPureDynamicImport = Self::IsTopLevel.bits() | Self::PureDynamicImport.bits();
7780
}

‎crates/rolldown_common/src/types/lazy_barrel.rs‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,9 @@ impl ImportedExports {
125125
pub struct ExportSource {
126126
imported: Specifier,
127127
record_idx: ImportRecordIdx,
128+
/// `true` for `export { a } from './x'` (dedicated re-export record).
129+
/// `false` for `import { a } from './x'; export { a }` (shared import record).
130+
is_direct_reexport: bool,
128131
}
129132

130133
#[derive(Debug, Default)]
@@ -226,7 +229,8 @@ impl BarrelInfo {
226229
}
227230
if has_local_export {
228231
let mut reexports = FxHashSet::with_capacity(self.named.len() + self.star.len());
229-
reexports.extend(self.named.values().map(|v| v.record_idx));
232+
reexports
233+
.extend(self.named.values().filter(|v| v.is_direct_reexport).map(|v| v.record_idx));
230234
reexports.extend(self.star.iter().copied());
231235
imported_exports_per_record.retain(|rec_idx, rec| match needs_records.entry(*rec_idx) {
232236
Entry::Occupied(_) => true,
@@ -294,11 +298,14 @@ pub fn try_extract_lazy_barrel_info(
294298
for (export_name, local_export) in &ecma_view.named_exports {
295299
if let Some(named_import) = ecma_view.named_imports.get(&local_export.referenced) {
296300
// Re-export: the export references an import
301+
let is_direct_reexport =
302+
raw_import_records[named_import.record_idx].meta.contains(ImportRecordMeta::IsReExportOnly);
297303
barrel_info.named.insert(
298304
export_name.clone(),
299305
ExportSource {
300306
record_idx: named_import.record_idx,
301307
imported: named_import.imported.clone(),
308+
is_direct_reexport,
302309
},
303310
);
304311
} else {

‎packages/rolldown/tests/fixtures/lazy-barrel/import-export-with-default/_config.ts‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
import path from 'node:path';
2+
import { expect } from 'vitest';
3+
import { defineTest } from 'rolldown-tests';
4+
5+
const transformedIds: string[] = [];
6+
7+
export default defineTest({
8+
config: {
9+
experimental: {
10+
lazyBarrel: true,
11+
},
12+
treeshake: {
13+
moduleSideEffects(id) {
14+
if (/barrel[\\/](index|a)\.js$/.test(id)) {
15+
return false;
16+
}
17+
return true;
18+
},
19+
},
20+
plugins: [
21+
{
22+
name: 'track-transforms',
23+
transform(_, id) {
24+
if (id.startsWith('\0')) {
25+
return;
26+
}
27+
transformedIds.push(id);
28+
},
29+
},
30+
],
31+
},
32+
afterTest: () => {
33+
const relativeIds = transformedIds.map((id) =>
34+
path.relative(import.meta.dirname, id).replace(/\\/g, '/'),
35+
);
36+
// barrel/index.js uses `import { a } from './a.js'; export { a }` (import-then-export)
37+
// and `const b = a(); export default b` (local export using the imported binding).
38+
// When `default` is imported, barrel must execute, which requires `a` from a.js.
39+
// Since the import record for a.js is NOT a direct re-export record,
40+
// it must be loaded with its specifiers so the barrel can access `a`.
41+
// a.js is itself a barrel (`export { a } from './aa.js'`), so if the `a` specifier
42+
// was correctly requested, aa.js must also be loaded.
43+
expect(relativeIds).toContain('main.js');
44+
expect(relativeIds).toContain('barrel/index.js');
45+
expect(relativeIds).toContain('barrel/a.js');
46+
expect(relativeIds).toContain('barrel/aa.js');
47+
expect(transformedIds.length).toBe(4);
48+
},
49+
});

‎packages/rolldown/tests/fixtures/lazy-barrel/import-export-with-default/barrel/a.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
export { a } from './aa.js';

‎packages/rolldown/tests/fixtures/lazy-barrel/import-export-with-default/barrel/aa.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
export function a() {
2+
return 'a';
3+
}

‎packages/rolldown/tests/fixtures/lazy-barrel/import-export-with-default/barrel/index.js‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
import { a } from './a.js';
2+
export { a };
3+
const b = a();
4+
export default b;

‎packages/rolldown/tests/fixtures/lazy-barrel/import-export-with-default/main.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
import b from './barrel';
2+
console.log(b);

‎packages/rolldown/tests/fixtures/lazy-barrel/treeshake-behavior/case-own-export-default/_config.ts‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,10 @@ export default defineTest({
4646
// When barrel executes, ALL its import records must be loaded because
4747
// sideEffects can only be determined after transform hook.
4848
// This includes both imports and re-exports.
49-
// However, d.js's re-export `export { dd } from './dd.js'` is not loaded
50-
// because `dd` is not requested by anyone.
49+
// Barrel has `import { d, dd } from './d.js'; export { d, dd }` (import-then-export).
50+
// Since the import record for d.js is shared (not a direct `export { } from`),
51+
// d.js is loaded with its specifiers `{d, dd}`. d.js in turn re-exports `dd`
52+
// from dd.js, so dd.js is also loaded.
5153
// g.js is loaded because barrel imports `gg` from it.
5254
// g.js is a pure re-export barrel, so gg.js is loaded to resolve `gg`.
5355
expect(relativeIds).toContain('main.js');
@@ -56,10 +58,11 @@ export default defineTest({
5658
expect(relativeIds).toContain('../barrel/b.js');
5759
expect(relativeIds).toContain('../barrel/c.js');
5860
expect(relativeIds).toContain('../barrel/d.js');
61+
expect(relativeIds).toContain('../barrel/dd.js');
5962
expect(relativeIds).toContain('../barrel/e.js');
6063
expect(relativeIds).toContain('../barrel/f.js');
6164
expect(relativeIds).toContain('../barrel/g.js');
6265
expect(relativeIds).toContain('../barrel/gg.js');
63-
expect(transformedIds.length).toBe(10);
66+
expect(transformedIds.length).toBe(11);
6467
},
6568
});

‎packages/rolldown/tests/fixtures/lazy-barrel/treeshake-behavior/case-own-export/_config.ts‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,10 @@ export default defineTest({
4545
// When barrel executes, ALL its import records must be loaded because
4646
// sideEffects can only be determined after transform hook.
4747
// This includes both imports and re-exports.
48-
// However, d.js's re-export `export { dd } from './dd.js'` is not loaded
49-
// because `dd` is not requested by anyone.
48+
// Barrel has `import { d, dd } from './d.js'; export { d, dd }` (import-then-export).
49+
// Since the import record for d.js is shared (not a direct `export { } from`),
50+
// d.js is loaded with its specifiers `{d, dd}`. d.js in turn re-exports `dd`
51+
// from dd.js, so dd.js is also loaded.
5052
// g.js is loaded because barrel imports `gg` from it.
5153
// g.js is a pure re-export barrel, so gg.js is loaded to resolve `gg`.
5254
expect(relativeIds).toContain('main.js');
@@ -55,10 +57,11 @@ export default defineTest({
5557
expect(relativeIds).toContain('../barrel/b.js');
5658
expect(relativeIds).toContain('../barrel/c.js');
5759
expect(relativeIds).toContain('../barrel/d.js');
60+
expect(relativeIds).toContain('../barrel/dd.js');
5861
expect(relativeIds).toContain('../barrel/e.js');
5962
expect(relativeIds).toContain('../barrel/f.js');
6063
expect(relativeIds).toContain('../barrel/g.js');
6164
expect(relativeIds).toContain('../barrel/gg.js');
62-
expect(transformedIds.length).toBe(10);
65+
expect(transformedIds.length).toBe(11);
6366
},
6467
});

0 commit comments

Comments
 (0)