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

Fix hoisting for optional member expressions #8121

Merged
merged 3 commits into from May 22, 2022
Merged

Fix hoisting for optional member expressions #8121

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cb2d87b
Add test
mischnic May 21, 2022
6138ad9
Key exports_locals by Id instead of JsWord
mischnic May 21, 2022
d2cbe48
Handle optional chaining wrapper in hoist
mischnic May 21, 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
5 changes: 5 additions & 0 deletions packages/core/integration-tests/test/integration/scope-hoisting/es6/rename-member-prop/a.js
View file
Open in desktop
@@ -0,0 +1,5 @@
export function foo(x) {
return [x.foo, x?.foo];
}

output = foo;
12 changes: 12 additions & 0 deletions packages/core/integration-tests/test/scope-hoisting.js
View file
Open in desktop
Expand Up @@ -186,6 +186,18 @@ describe('scope hoisting', function () {
assert.deepEqual(output, ['1', '2']);
});

it("doesn't rename member expression properties", async function () {
let b = await bundle(
path.join(
__dirname,
'/integration/scope-hoisting/es6/rename-member-prop/a.js',
),
);

let output = await run(b);
assert.deepEqual(output({foo: 12}), [12, 12]);
});

it('supports renaming imports', async function () {
let b = await bundle(
path.join(
Expand Down
42 changes: 27 additions & 15 deletions packages/transformers/js/core/src/hoist.rs
View file
Open in desktop
Expand Up @@ -254,10 +254,7 @@ impl<'a> Fold for Hoist<'a> {
id.0
} else {
self
.get_export_ident(
DUMMY_SP,
self.collect.exports_locals.get(&id.0).unwrap(),
)
.get_export_ident(DUMMY_SP, self.collect.exports_locals.get(&id).unwrap())
.sym
};
self.exported_symbols.push(ExportedSymbol {
Expand Down Expand Up @@ -514,6 +511,21 @@ impl<'a> Fold for Hoist<'a> {

fn fold_expr(&mut self, node: Expr) -> Expr {
match node {
Expr::OptChain(opt) => {
return Expr::OptChain(OptChainExpr {
span: opt.span,
question_dot_token: opt.question_dot_token,
base: match opt.base {
OptChainBase::Call(call) => OptChainBase::Call(call.fold_with(self)),
OptChainBase::Member(member) => OptChainBase::Member(MemberExpr {
span: member.span,
obj: member.obj.fold_with(self),
// Don't visit member.prop so we avoid the ident visitor.
prop: member.prop,
}),
},
});
}
Expr::Member(member) => {
if !self.collect.should_wrap {
if match_member_expr(&member, vec!["module", "exports"], &self.collect.decls) {
Expand Down Expand Up @@ -585,7 +597,7 @@ impl<'a> Fold for Hoist<'a> {
return Expr::Ident(self.get_export_ident(member.span, &key));
}
}
Expr::Call(_call) => {
Expr::Call(_) => {
// require('foo').bar -> $id$import$foo$bar
if let Some(source) =
match_require(&member.obj, &self.collect.decls, self.collect.ignore_mark)
Expand Down Expand Up @@ -770,7 +782,7 @@ impl<'a> Fold for Hoist<'a> {
}
}

if let Some(exported) = self.collect.exports_locals.get(&node.sym) {
if let Some(exported) = self.collect.exports_locals.get(&id!(node)) {
// If wrapped, mark the original symbol as exported.
// Otherwise replace with an export identifier.
if self.collect.should_wrap {
Expand Down Expand Up @@ -1094,7 +1106,7 @@ pub struct Collect {
// exported name -> descriptor
pub exports: HashMap<JsWord, Export>,
// local name -> exported name
pub exports_locals: HashMap<JsWord, JsWord>,
pub exports_locals: HashMap<IdentId, JsWord>,
pub exports_all: HashMap<JsWord, SourceLocation>,
pub non_static_access: HashMap<IdentId, Vec<Span>>,
pub non_const_bindings: HashMap<IdentId, Vec<Span>>,
Expand Down Expand Up @@ -1373,7 +1385,7 @@ impl Visit for Collect {
if node.src.is_none() {
self
.exports_locals
.entry(match_export_name_ident(&named.orig).sym.clone())
.entry(id!(match_export_name_ident(&named.orig)))
.or_insert_with(|| exported.0.clone());
}
}
Expand All @@ -1389,7 +1401,7 @@ impl Visit for Collect {
if node.src.is_none() {
self
.exports_locals
.entry(default.exported.sym.clone())
.entry(id!(default.exported))
.or_insert_with(|| js_word!("default"));
}
}
Expand Down Expand Up @@ -1422,7 +1434,7 @@ impl Visit for Collect {
);
self
.exports_locals
.entry(class.ident.sym.clone())
.entry(id!(class.ident))
.or_insert_with(|| class.ident.sym.clone());
}
Decl::Fn(func) => {
Expand All @@ -1436,7 +1448,7 @@ impl Visit for Collect {
);
self
.exports_locals
.entry(func.ident.sym.clone())
.entry(id!(func.ident))
.or_insert_with(|| func.ident.sym.clone());
}
Decl::Var(var) => {
Expand Down Expand Up @@ -1468,7 +1480,7 @@ impl Visit for Collect {
);
self
.exports_locals
.entry(ident.sym.clone())
.entry(id!(ident))
.or_insert_with(|| js_word!("default"));
} else {
self.exports.insert(
Expand All @@ -1493,7 +1505,7 @@ impl Visit for Collect {
);
self
.exports_locals
.entry(ident.sym.clone())
.entry(id!(ident))
.or_insert_with(|| js_word!("default"));
} else {
self.exports.insert(
Expand Down Expand Up @@ -1555,7 +1567,7 @@ impl Visit for Collect {
);
self
.exports_locals
.entry(node.id.sym.clone())
.entry(id!(node.id))
.or_insert_with(|| node.id.sym.clone());
}

Expand All @@ -1580,7 +1592,7 @@ impl Visit for Collect {
);
self
.exports_locals
.entry(node.key.sym.clone())
.entry(id!(node.key))
.or_insert_with(|| node.key.sym.clone());
}

Expand Down