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.

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

fix(es/transforms/module/commonjs): fix named exports overrides #2883

Merged
merged 16 commits into from
Nov 28, 2021
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions crates/swc/tests/fixture/issue-1714/case1/output/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
Object.defineProperty(exports, "__esModule", {
value: true
});
var _exportNames = {
render: true
};
Object.defineProperty(exports, "render", {
enumerable: true,
get: function() {
Expand All @@ -12,6 +15,7 @@ var _customRender = require("./customRender");
var _react = _interopRequireWildcard(require("@testing-library/react"));
Object.keys(_react).forEach(function(key) {
if (key === "default" || key === "__esModule") return;
if (Object.prototype.hasOwnProperty.call(_exportNames, key)) return;
if (key in exports && exports[key] === _react[key]) return;
Object.defineProperty(exports, key, {
enumerable: true,
Expand Down
39 changes: 26 additions & 13 deletions crates/swc_ecma_transforms_module/src/common_js.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,31 @@ where
// Used only if export * exists
let mut exported_names: Option<Ident> = None;

// make a preliminary pass through to collect exported names ahead of time
for item in items.clone() {
Copy link
Member

Choose a reason for hiding this comment

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

I think we don't need clone().

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm fairly new to Rust. Is there a better way to iterate a vector twice? Removing clone would cause the second iteration to fail.

Copy link
Member

Choose a reason for hiding this comment

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

You can use for item in &items instead, as &Vec<T> implements IntoIterator<Item = &T>.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, thank you! One of these days, I will learn Rust properly.

match item {
ModuleItem::ModuleDecl(ModuleDecl::ExportNamed(NamedExport {
ref specifiers,
..
})) => {
for ExportNamedSpecifier { orig, exported, .. } in
specifiers.into_iter().filter_map(|e| match e {
ExportSpecifier::Named(e) => Some(e),
_ => None,
})
{
if let Some(exported) = &exported {
exports.push(exported.sym.clone());
} else {
exports.push(orig.sym.clone());
}
}
}

_ => {}
}
}

for item in items {
self.in_top_level = true;

Expand Down Expand Up @@ -158,21 +183,9 @@ where
ref specifiers,
..
})) => {
// handle: export {sym as alias1, alias2, ...} from "x"
for ExportNamedSpecifier { orig, exported, .. } in
specifiers.into_iter().filter_map(|e| match e {
ExportSpecifier::Named(e) => Some(e),
_ => None,
})
{
if let Some(exported) = &exported {
exports.push(exported.sym.clone());
} else {
exports.push(orig.sym.clone());
}
}
scope.import_to_export(&src, !specifiers.is_empty());
}

_ => {}
}
drop(scope);
Expand Down
10 changes: 7 additions & 3 deletions crates/swc_ecma_transforms_module/tests/common_js.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4161,6 +4161,9 @@ test!(
Object.defineProperty(exports, "__esModule", {
value: true
});
var _exportNames = {
Scope: true
};
Object.defineProperty(exports, "Scope", {
enumerable: true,
get: function() {
Expand All @@ -4170,6 +4173,7 @@ Object.defineProperty(exports, "Scope", {
var _http = require("./http");
Object.keys(_http).forEach(function(key) {
if (key === "default" || key === "__esModule") return;
if (Object.prototype.hasOwnProperty.call(_exportNames, key)) return;
if (key in exports && exports[key] === _http[key]) return;
Object.defineProperty(exports, key, {
enumerable: true,
Expand Down Expand Up @@ -4200,15 +4204,15 @@ export * from './pipes';
Object.defineProperty(exports, "__esModule", {
value: true
});
var _exportNames = {
id: true
};
Object.defineProperty(exports, "id", {
enumerable: true,
get: function() {
return _interfaces.id;
}
});
var _exportNames = {
id: true
};
require("reflect-metadata");
var _http = require("./http");
Object.keys(_http).forEach(function(key) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { customRender } from './customRender';

// re-export everything
export * from '@testing-library/react';

// override render method
export { customRender as render };
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
"use strict";
Object.defineProperty(exports, "__esModule", {
value: true
});
var _exportNames = {
render: true
};
Object.defineProperty(exports, "render", {
enumerable: true,
get: function() {
return _customRender.customRender;
}
});
var _customRender = require("./customRender");
var _react = require("@testing-library/react");
Object.keys(_react).forEach(function(key) {
if (key === "default" || key === "__esModule") return;
if (Object.prototype.hasOwnProperty.call(_exportNames, key)) return;
if (key in exports && exports[key] === _react[key]) return;
Object.defineProperty(exports, key, {
enumerable: true,
get: function() {
return _react[key];
}
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { customRender } from './customRender';

// override render method
export { customRender as render };

// re-ordering the export wildcard from case 1 should make no difference to output

// re-export everything
export * from '@testing-library/react';
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
"use strict";
Object.defineProperty(exports, "__esModule", {
value: true
});
Object.defineProperty(exports, "render", {
enumerable: true,
get: function() {
return _customRender.customRender;
}
});
var _exportNames = {
render: true
};
var _customRender = require("./customRender");
var _react = require("@testing-library/react");
Object.keys(_react).forEach(function(key) {
if (key === "default" || key === "__esModule") return;
if (Object.prototype.hasOwnProperty.call(_exportNames, key)) return;
if (key in exports && exports[key] === _react[key]) return;
Object.defineProperty(exports, key, {
enumerable: true,
get: function() {
return _react[key];
}
});
});