Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Retain correct dependency order between imports and reexports without scopehoisting #8591

Merged
merged 4 commits into from Nov 4, 2022
Merged

Retain correct dependency order between imports and reexports without scopehoisting #8591

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f42d256
Add test
mischnic Oct 31, 2022
9b87b03
Fix
mischnic Oct 31, 2022
08952f7
Merge branch 'v2' into dev-import-reexport-dep-order
mischnic Oct 31, 2022
1cbf186
Merge branch 'v2' into dev-import-reexport-dep-order
mischnic Nov 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/core/integration-tests/test/integration/js-import-reexport-dep-order/a.js
View file
Open in desktop
@@ -0,0 +1,2 @@
sideEffect("a");
export const a = "a";
2 changes: 2 additions & 0 deletions packages/core/integration-tests/test/integration/js-import-reexport-dep-order/b.js
View file
Open in desktop
@@ -0,0 +1,2 @@
sideEffect("b");
export const b = "b";
2 changes: 2 additions & 0 deletions packages/core/integration-tests/test/integration/js-import-reexport-dep-order/c.js
View file
Open in desktop
@@ -0,0 +1,2 @@
sideEffect("c");
export const c = "c";
5 changes: 5 additions & 0 deletions packages/core/integration-tests/test/integration/js-import-reexport-dep-order/index.js
View file
Open in desktop
@@ -0,0 +1,5 @@
export * from './a'

export {b} from './b'

import './c'
14 changes: 14 additions & 0 deletions packages/core/integration-tests/test/javascript.js
View file
Open in desktop
Expand Up @@ -5302,6 +5302,20 @@ describe('javascript', function () {
assert.deepEqual(res.default, ['a', 'b', 'bar']);
});

it('should retain the correct dependency order between import and reexports', async function () {
let b = await bundle(
path.join(__dirname, 'integration/js-import-reexport-dep-order/index.js'),
);

let calls = [];
await run(b, {
sideEffect(v) {
calls.push(v);
},
});
assert.deepEqual(calls, ['a', 'b', 'c']);
});

it('should not freeze live default imports', async function () {
let b = await bundle(
path.join(__dirname, 'integration/js-import-default-live/index.js'),
Expand Down
182 changes: 99 additions & 83 deletions packages/transformers/js/core/src/modules.rs
View file
Open in desktop
Expand Up @@ -264,43 +264,110 @@ macro_rules! modules_visit_fn {
impl Fold for ESMFold {
fn fold_module(&mut self, node: Module) -> Module {
let mut is_esm = false;
let mut needs_interop_flag = false;

// First pass: collect all imported declarations.
// First pass: collect all imported declarations. On the second pass, exports can be matched to
// imports (to better handle import/export pairs that are really just reexports).
//
// To ensure that all declarations that cause dependencies are kept in the same order, handle
// export declarations with a source in the first pass as well.
for item in &node.body {
if let ModuleItem::ModuleDecl(decl) = &item {
is_esm = true;
if let ModuleDecl::Import(import) = decl {
self.create_require(import.src.value.clone(), import.span);

for specifier in &import.specifiers {
match specifier {
ImportSpecifier::Named(named) => {
let imported = match &named.imported {
Some(imported) => match_export_name(imported).0.clone(),
None => named.local.sym.clone(),
};
self.imports.insert(
id!(named.local),
(import.src.value.clone(), imported.clone()),
);
if imported == js_word!("default") {
match decl {
ModuleDecl::Import(import) => {
self.create_require(import.src.value.clone(), import.span);

for specifier in &import.specifiers {
match specifier {
ImportSpecifier::Named(named) => {
let imported = match &named.imported {
Some(imported) => match_export_name(imported).0.clone(),
None => named.local.sym.clone(),
};
self.imports.insert(
id!(named.local),
(import.src.value.clone(), imported.clone()),
);
if imported == js_word!("default") {
self.create_interop_default(import.src.value.clone());
}
}
ImportSpecifier::Default(default) => {
self.imports.insert(
id!(default.local),
(import.src.value.clone(), "default".into()),
);
self.create_interop_default(import.src.value.clone());
}
ImportSpecifier::Namespace(namespace) => {
self
.imports
.insert(id!(namespace.local), (import.src.value.clone(), "*".into()));
}
}
ImportSpecifier::Default(default) => {
self.imports.insert(
id!(default.local),
(import.src.value.clone(), "default".into()),
);
self.create_interop_default(import.src.value.clone());
}
ImportSpecifier::Namespace(namespace) => {
self
.imports
.insert(id!(namespace.local), (import.src.value.clone(), "*".into()));
}
}
ModuleDecl::ExportNamed(export) => {
needs_interop_flag = true;

if let Some(src) = &export.src {
self.create_require(src.value.clone(), export.span);

for specifier in &export.specifiers {
match specifier {
ExportSpecifier::Named(named) => {
let exported = match &named.exported {
Some(exported) => exported.clone(),
None => named.orig.clone(),
};

if match_export_name(&named.orig).0 == js_word!("default") {
self.create_interop_default(src.value.clone());
}

let specifier = self.create_import_access(
&src.value,
&match_export_name(&named.orig).0,
DUMMY_SP,
);
self.create_export(match_export_name(&exported).0, specifier, export.span);
}
ExportSpecifier::Default(default) => {
self.create_interop_default(src.value.clone());
let specifier =
self.create_import_access(&src.value, &js_word!("default"), DUMMY_SP);
self.create_export(default.exported.sym.clone(), specifier, export.span);
}
ExportSpecifier::Namespace(namespace) => {
let local = self.get_require_name(&src.value, DUMMY_SP);
self.create_export(
match_export_name(&namespace.name).0,
Expr::Ident(local),
export.span,
)
}
}
}
} else {
// Handled below
}
}
ModuleDecl::ExportAll(export) => {
needs_interop_flag = true;
self.create_require(export.src.value.clone(), export.span);
let require_name = self.get_require_name(&export.src.value, export.span);
let export = self.call_helper(
"exportAll".into(),
vec![
Expr::Ident(require_name),
Expr::Ident(Ident::new("exports".into(), DUMMY_SP)),
],
export.span,
);
self.requires.push(export);
}
_ => (),
}
}
}
Expand All @@ -311,58 +378,19 @@ impl Fold for ESMFold {
}

let node = node.fold_children_with(self);
let mut needs_interop_flag = false;
let mut items = vec![];

// Second pass
for item in &node.body {
match &item {
ModuleItem::ModuleDecl(decl) => {
match decl {
ModuleDecl::Import(_import) => {
ModuleDecl::Import(_) | ModuleDecl::ExportAll(_) => {
// Handled above
}
ModuleDecl::ExportNamed(export) => {
needs_interop_flag = true;

if let Some(src) = &export.src {
self.create_require(src.value.clone(), export.span);

for specifier in &export.specifiers {
match specifier {
ExportSpecifier::Named(named) => {
let exported = match &named.exported {
Some(exported) => exported.clone(),
None => named.orig.clone(),
};

if match_export_name(&named.orig).0 == js_word!("default") {
self.create_interop_default(src.value.clone());
}

let specifier = self.create_import_access(
&src.value,
&match_export_name(&named.orig).0,
DUMMY_SP,
);
self.create_export(match_export_name(&exported).0, specifier, export.span);
}
ExportSpecifier::Default(default) => {
self.create_interop_default(src.value.clone());
let specifier =
self.create_import_access(&src.value, &js_word!("default"), DUMMY_SP);
self.create_export(default.exported.sym.clone(), specifier, export.span);
}
ExportSpecifier::Namespace(namespace) => {
let local = self.get_require_name(&src.value, DUMMY_SP);
self.create_export(
match_export_name(&namespace.name).0,
Expr::Ident(local),
export.span,
)
}
}
}
} else {
if export.src.is_none() {
for specifier in &export.specifiers {
if let ExportSpecifier::Named(named) = specifier {
let exported = match &named.exported {
Expand All @@ -386,22 +414,10 @@ impl Fold for ESMFold {
self.create_export(match_export_name(&exported).0, value, export.span);
}
}
} else {
// Handled above
}
}
ModuleDecl::ExportAll(export) => {
needs_interop_flag = true;
self.create_require(export.src.value.clone(), export.span);
let require_name = self.get_require_name(&export.src.value, export.span);
let export = self.call_helper(
"exportAll".into(),
vec![
Expr::Ident(require_name),
Expr::Ident(Ident::new("exports".into(), DUMMY_SP)),
],
export.span,
);
self.requires.push(export);
}
ModuleDecl::ExportDefaultExpr(export) => {
needs_interop_flag = true;
items.push(self.create_exports_assign(
Expand Down