From f5ab4677f0b01c317756b0bc0e8f759f1da49988 Mon Sep 17 00:00:00 2001 From: Devon Govett Date: Fri, 17 Dec 2021 16:17:53 -0500 Subject: [PATCH] Static analyze more exports patterns without scope hoisting (#7465) --- packages/transformers/js/core/src/hoist.rs | 327 +++++++++++++++++---- packages/transformers/js/core/src/lib.rs | 5 +- packages/transformers/js/core/src/utils.rs | 16 +- 3 files changed, 288 insertions(+), 60 deletions(-) diff --git a/packages/transformers/js/core/src/hoist.rs b/packages/transformers/js/core/src/hoist.rs index 3bec6b11c87..7bf8fafe5a4 100644 --- a/packages/transformers/js/core/src/hoist.rs +++ b/packages/transformers/js/core/src/hoist.rs @@ -1,3 +1,4 @@ +use crate::utils::match_property_name; use serde::{Deserialize, Serialize}; use std::collections::hash_map::DefaultHasher; use std::collections::{HashMap, HashSet}; @@ -885,16 +886,10 @@ impl<'a> Fold for Hoist<'a> { if is_cjs_exports { let key: JsWord = if self.collect.static_cjs_exports { - match &*member.prop { - Expr::Ident(ident) => { - if !member.computed { - ident.sym.clone() - } else { - unreachable!("Unexpected non-static CJS export"); - } - } - Expr::Lit(Lit::Str(str_)) => str_.value.clone(), - _ => unreachable!("Unexpected non-static CJS export"), + if let Some((name, _)) = match_property_name(&member) { + name + } else { + unreachable!("Unexpected non-static CJS export"); } } else { "*".into() @@ -1126,7 +1121,7 @@ pub struct Import { pub loc: SourceLocation, } -#[derive(Debug)] +#[derive(Debug, PartialEq)] pub struct Export { pub source: Option, pub specifier: JsWord, @@ -1228,6 +1223,41 @@ impl Collect { impl From for CollectResult { fn from(collect: Collect) -> CollectResult { + let mut exports: Vec = collect + .exports + .into_iter() + .map( + |( + exported, + Export { + source, + specifier, + loc, + }, + )| CollectExportedSymbol { + source, + local: specifier, + exported, + loc, + }, + ) + .collect(); + + // Add * symbol if there are any CJS exports so that unknown symbols don't cause errors (e.g. default interop). + if collect.has_cjs_exports { + exports.push(CollectExportedSymbol { + source: None, + exported: "*".into(), + local: "_".into(), + loc: SourceLocation { + start_line: 1, + start_col: 1, + end_line: 1, + end_col: 1, + }, + }) + } + CollectResult { imports: collect .imports @@ -1250,25 +1280,7 @@ impl From for CollectResult { }, ) .collect(), - exports: collect - .exports - .into_iter() - .map( - |( - exported, - Export { - source, - specifier, - loc, - }, - )| CollectExportedSymbol { - source, - local: specifier, - exported, - loc, - }, - ) - .collect(), + exports, exports_all: collect .exports_all .into_iter() @@ -1495,7 +1507,7 @@ impl Visit for Collect { DefaultDecl::Class(class) => { if let Some(ident) = &class.ident { self.exports.insert( - "default".into(), + js_word!("default"), Export { specifier: ident.sym.clone(), loc: SourceLocation::from(&self.source_map, node.span), @@ -1505,13 +1517,22 @@ impl Visit for Collect { self .exports_locals .entry(ident.sym.clone()) - .or_insert_with(|| "default".into()); + .or_insert_with(|| js_word!("default")); + } else { + self.exports.insert( + js_word!("default"), + Export { + specifier: js_word!("default"), + loc: SourceLocation::from(&self.source_map, node.span), + source: None, + }, + ); } } DefaultDecl::Fn(func) => { if let Some(ident) = &func.ident { self.exports.insert( - "default".into(), + js_word!("default"), Export { specifier: ident.sym.clone(), loc: SourceLocation::from(&self.source_map, node.span), @@ -1521,7 +1542,16 @@ impl Visit for Collect { self .exports_locals .entry(ident.sym.clone()) - .or_insert_with(|| "default".into()); + .or_insert_with(|| js_word!("default")); + } else { + self.exports.insert( + js_word!("default"), + Export { + specifier: js_word!("default"), + loc: SourceLocation::from(&self.source_map, node.span), + source: None, + }, + ); } } _ => { @@ -1532,6 +1562,19 @@ impl Visit for Collect { node.visit_children_with(self); } + fn visit_export_default_expr(&mut self, node: &ExportDefaultExpr) { + self.exports.insert( + js_word!("default"), + Export { + specifier: js_word!("default"), + loc: SourceLocation::from(&self.source_map, node.span), + source: None, + }, + ); + + node.visit_children_with(self); + } + fn visit_export_all(&mut self, node: &ExportAll) { self.exports_all.insert( node.src.value.clone(), @@ -1617,32 +1660,37 @@ impl Visit for Collect { return; } - let is_static = match &*node.prop { - Expr::Ident(_) => !node.computed, - Expr::Lit(Lit::Str(_)) => true, - _ => false, - }; + macro_rules! handle_export { + () => { + self.has_cjs_exports = true; + if let Some((name, span)) = match_property_name(&node) { + self.exports.insert( + name.clone(), + Export { + specifier: name, + source: None, + loc: SourceLocation::from(&self.source_map, span), + }, + ); + } else { + self.static_cjs_exports = false; + self.add_bailout(node.span, BailoutReason::NonStaticExports); + } + }; + } if let ExprOrSuper::Expr(expr) = &node.obj { match &**expr { Expr::Member(member) => { if match_member_expr(member, vec!["module", "exports"], &self.decls) { - self.has_cjs_exports = true; - if !is_static { - self.static_cjs_exports = false; - self.add_bailout(node.span, BailoutReason::NonStaticExports); - } + handle_export!(); } return; } Expr::Ident(ident) => { let exports: JsWord = "exports".into(); if ident.sym == exports && !self.decls.contains(&id!(ident)) { - self.has_cjs_exports = true; - if !is_static { - self.static_cjs_exports = false; - self.add_bailout(node.span, BailoutReason::NonStaticExports); - } + handle_export!(); } if ident.sym == js_word!("module") && !self.decls.contains(&id!(ident)) { @@ -1653,7 +1701,7 @@ impl Visit for Collect { } // `import` isn't really an identifier... - if !is_static && ident.sym != js_word!("import") { + if match_property_name(&node).is_none() && ident.sym != js_word!("import") { self .non_static_access .entry(id!(ident)) @@ -1664,11 +1712,7 @@ impl Visit for Collect { } Expr::This(_this) => { if self.in_module_this { - self.has_cjs_exports = true; - if !is_static { - self.static_cjs_exports = false; - self.add_bailout(node.span, BailoutReason::NonStaticExports); - } + handle_export!(); } return; } @@ -4068,4 +4112,177 @@ mod tests { "#} ); } + + #[test] + fn collect_exports() { + let (collect, _code, _hoist) = parse("export default function () {};"); + assert_eq!( + collect.exports, + map! { + w!("default") => Export { + source: None, + specifier: "default".into(), + loc: SourceLocation { + start_line: 1, + start_col: 1, + end_line: 1, + end_col: 29 + } + } + } + ); + + let (collect, _code, _hoist) = parse("export default function test () {};"); + assert_eq!( + collect.exports, + map! { + w!("default") => Export { + source: None, + specifier: "test".into(), + loc: SourceLocation { + start_line: 1, + start_col: 1, + end_line: 1, + end_col: 34 + } + } + } + ); + + let (collect, _code, _hoist) = parse("export default class {};"); + assert_eq!( + collect.exports, + map! { + w!("default") => Export { + source: None, + specifier: "default".into(), + loc: SourceLocation { + start_line: 1, + start_col: 1, + end_line: 1, + end_col: 23 + } + } + } + ); + + let (collect, _code, _hoist) = parse("export default class test {};"); + assert_eq!( + collect.exports, + map! { + w!("default") => Export { + source: None, + specifier: "test".into(), + loc: SourceLocation { + start_line: 1, + start_col: 1, + end_line: 1, + end_col: 28 + } + } + } + ); + + let (collect, _code, _hoist) = parse("export default foo;"); + assert_eq!( + collect.exports, + map! { + w!("default") => Export { + source: None, + specifier: "default".into(), + loc: SourceLocation { + start_line: 1, + start_col: 1, + end_line: 1, + end_col: 19 + } + } + } + ); + + let (collect, _code, _hoist) = parse("module.exports.foo = 2;"); + assert_eq!( + collect.exports, + map! { + w!("foo") => Export { + source: None, + specifier: "foo".into(), + loc: SourceLocation { + start_line: 1, + start_col: 16, + end_line: 1, + end_col: 18 + } + } + } + ); + + let (collect, _code, _hoist) = parse("module.exports['foo'] = 2;"); + assert_eq!( + collect.exports, + map! { + w!("foo") => Export { + source: None, + specifier: "foo".into(), + loc: SourceLocation { + start_line: 1, + start_col: 16, + end_line: 1, + end_col: 20 + } + } + } + ); + + let (collect, _code, _hoist) = parse("module.exports[`foo`] = 2;"); + assert_eq!( + collect.exports, + map! { + w!("foo") => Export { + source: None, + specifier: "foo".into(), + loc: SourceLocation { + start_line: 1, + start_col: 16, + end_line: 1, + end_col: 20 + } + } + } + ); + + let (collect, _code, _hoist) = parse("exports.foo = 2;"); + assert_eq!( + collect.exports, + map! { + w!("foo") => Export { + source: None, + specifier: "foo".into(), + loc: SourceLocation { + start_line: 1, + start_col: 9, + end_line: 1, + end_col: 11 + } + } + } + ); + + let (collect, _code, _hoist) = parse("this.foo = 2;"); + assert_eq!( + collect.exports, + map! { + w!("foo") => Export { + source: None, + specifier: "foo".into(), + loc: SourceLocation { + start_line: 1, + start_col: 6, + end_line: 1, + end_col: 8 + } + } + } + ); + } } diff --git a/packages/transformers/js/core/src/lib.rs b/packages/transformers/js/core/src/lib.rs index 6a2e131f32b..7f6626498c1 100644 --- a/packages/transformers/js/core/src/lib.rs +++ b/packages/transformers/js/core/src/lib.rs @@ -424,7 +424,10 @@ pub fn transform(config: Config) -> Result { } } } else { - result.symbol_result = Some(collect.into()); + // Bail if we could not statically analyze. + if collect.static_cjs_exports && !collect.should_wrap { + result.symbol_result = Some(collect.into()); + } let (module, needs_helpers) = esm2cjs(module, versions); result.needs_esm_helpers = needs_helpers; diff --git a/packages/transformers/js/core/src/utils.rs b/packages/transformers/js/core/src/utils.rs index 913a37fc0b3..0e8924ead1b 100644 --- a/packages/transformers/js/core/src/utils.rs +++ b/packages/transformers/js/core/src/utils.rs @@ -114,6 +114,14 @@ pub fn match_str_or_ident(node: &ast::Expr) -> Option<(JsWord, Span)> { match_str(node) } +pub fn match_property_name(node: &ast::MemberExpr) -> Option<(JsWord, Span)> { + if node.computed { + match_str(&*node.prop) + } else { + match_str_or_ident(&*node.prop) + } +} + pub fn match_require( node: &ast::Expr, decls: &HashSet<(JsWord, SyntaxContext)>, @@ -178,10 +186,10 @@ pub fn match_import(node: &ast::Expr, ignore_mark: Mark) -> Option { #[derive(Debug, Serialize, Deserialize, Clone, Eq, PartialEq)] pub struct SourceLocation { - start_line: usize, - start_col: usize, - end_line: usize, - end_col: usize, + pub start_line: usize, + pub start_col: usize, + pub end_line: usize, + pub end_col: usize, } impl SourceLocation {