From 384346971549c05083217c1b8ad8ed6adb4da42c Mon Sep 17 00:00:00 2001 From: mattcompiles Date: Thu, 19 Oct 2023 19:34:51 +1100 Subject: [PATCH 1/6] Fix TSC check for CJS files --- .../integration-tests/test/scope-hoisting.js | 41 +++++++++++++++++++ packages/transformers/js/core/src/collect.rs | 5 ++- packages/transformers/js/core/src/fs.rs | 2 + packages/transformers/js/core/src/lib.rs | 3 ++ 4 files changed, 50 insertions(+), 1 deletion(-) diff --git a/packages/core/integration-tests/test/scope-hoisting.js b/packages/core/integration-tests/test/scope-hoisting.js index 5e7b871432f..348e93648ce 100644 --- a/packages/core/integration-tests/test/scope-hoisting.js +++ b/packages/core/integration-tests/test/scope-hoisting.js @@ -17,6 +17,7 @@ import { overlayFS, run, runBundle, + fsFixture, } from '@parcel/test-utils'; const bundle = (name, opts = {}) => { @@ -2487,6 +2488,23 @@ describe('scope hoisting', function () { assert.equal(await output, 42); }); + it('should handle TSC polyfills', async () => { + await fsFixture(overlayFS, __dirname)` + tsc-polyfill-es6 + library.js: + var __polyfill = (this && this.__polyfill) || function (a) {return a;}; + export default __polyfill('es6') + + index.js: + import value from './library'; + output = value;`; + + let b = await bundle(path.join(__dirname, 'tsc-polyfill-es6/index.js'), { + inputFS: overlayFS, + }); + assert.equal(await run(b), 'es6'); + }); + describe("considers an asset's closest package.json for sideEffects, not the package through which it found the asset", () => { it('handles redirects up the tree', async () => { let b = await bundle( @@ -5265,6 +5283,29 @@ describe('scope hoisting', function () { assert.deepEqual(await run(b), {test: 2}); }); + + it('should handle TSC polyfills', async () => { + await fsFixture(overlayFS, __dirname)` + tsc-polyfill-commonjs + library.js: + "use strict"; + var __polyfill = (this && this.__polyfill) || function (a) {return a;}; + exports.value = __polyfill('cjs') + + index.js: + const value = require('./library'); + output = value; + `; + + let b = await bundle( + path.join(__dirname, 'tsc-polyfill-commonjs/index.js'), + { + inputFS: overlayFS, + }, + ); + + assert.deepEqual(await run(b), {value: 'cjs'}); + }); }); it('should not throw with JS included from HTML', async function () { diff --git a/packages/transformers/js/core/src/collect.rs b/packages/transformers/js/core/src/collect.rs index 5abd717b0a0..f94f5664cfd 100644 --- a/packages/transformers/js/core/src/collect.rs +++ b/packages/transformers/js/core/src/collect.rs @@ -78,6 +78,7 @@ pub struct Collect { in_function: bool, in_assign: bool, in_class: bool, + is_module: bool, } #[derive(Debug, Serialize)] @@ -120,12 +121,14 @@ impl Collect { ignore_mark: Mark, global_mark: Mark, trace_bailouts: bool, + is_module: bool, ) -> Self { Collect { source_map, decls, ignore_mark, global_mark, + is_module, static_cjs_exports: true, has_cjs_exports: false, is_esm: false, @@ -768,7 +771,7 @@ impl Visit for Collect { } } Expr::Bin(bin_expr) => { - if self.in_module_this { + if self.is_module & self.in_module_this { // Some TSC polyfills use a pattern like below. // We want to avoid marking these modules as CJS // e.g. var _polyfill = (this && this.polyfill) || function () {} diff --git a/packages/transformers/js/core/src/fs.rs b/packages/transformers/js/core/src/fs.rs index 0bb3d001579..83cc9b030c4 100644 --- a/packages/transformers/js/core/src/fs.rs +++ b/packages/transformers/js/core/src/fs.rs @@ -17,6 +17,7 @@ pub fn inline_fs<'a>( global_mark: Mark, project_root: &'a str, deps: &'a mut Vec, + is_module: bool, ) -> impl Fold + 'a { InlineFS { filename: Path::new(filename).to_path_buf(), @@ -26,6 +27,7 @@ pub fn inline_fs<'a>( Mark::fresh(Mark::root()), global_mark, false, + is_module, ), global_mark, project_root, diff --git a/packages/transformers/js/core/src/lib.rs b/packages/transformers/js/core/src/lib.rs index b7fa321ef34..234ef1e1ebe 100644 --- a/packages/transformers/js/core/src/lib.rs +++ b/packages/transformers/js/core/src/lib.rs @@ -252,6 +252,7 @@ pub fn transform(config: Config) -> Result { ), )); + let is_module = module.is_module(); // If it's a script, convert into module. This needs to happen after // the resolver (which behaves differently for non-/strict mode). let module = match module { @@ -342,6 +343,7 @@ pub fn transform(config: Config) -> Result { global_mark, &config.project_root, &mut fs_deps, + is_module ), should_inline_fs ), @@ -448,6 +450,7 @@ pub fn transform(config: Config) -> Result { ignore_mark, global_mark, config.trace_bailouts, + is_module, ); module.visit_with(&mut collect); if let Some(bailouts) = &collect.bailouts { From 993fc0a5276a9ac6a0d0cbc7889752943da3191b Mon Sep 17 00:00:00 2001 From: mattcompiles Date: Thu, 19 Oct 2023 21:19:40 +1100 Subject: [PATCH 2/6] Fix unit tests --- packages/transformers/js/core/src/hoist.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/transformers/js/core/src/hoist.rs b/packages/transformers/js/core/src/hoist.rs index 40ce3cc1719..d61acc4ab46 100644 --- a/packages/transformers/js/core/src/hoist.rs +++ b/packages/transformers/js/core/src/hoist.rs @@ -1155,6 +1155,7 @@ mod tests { Mark::fresh(Mark::root()), global_mark, true, + true, ); module.visit_with(&mut collect); From 710354399d8f01f1a2be8f9362c02f809d43fdf9 Mon Sep 17 00:00:00 2001 From: mattcompiles Date: Fri, 20 Oct 2023 11:40:17 +1100 Subject: [PATCH 3/6] Remove this tracking for files with module syntax --- packages/transformers/js/core/src/collect.rs | 28 +++++--------------- packages/transformers/js/core/src/hoist.rs | 17 +++++++++--- 2 files changed, 20 insertions(+), 25 deletions(-) diff --git a/packages/transformers/js/core/src/collect.rs b/packages/transformers/js/core/src/collect.rs index f94f5664cfd..45b077d8456 100644 --- a/packages/transformers/js/core/src/collect.rs +++ b/packages/transformers/js/core/src/collect.rs @@ -692,6 +692,11 @@ impl Visit for Collect { return; } Expr::This(_this) => { + if self.is_module { + // Don't track this bindings if ESM syntax is present + return; + } + if self.in_module_this { handle_export!(); } else if !self.in_class { @@ -770,27 +775,6 @@ impl Visit for Collect { self.used_imports.insert(id!(ident)); } } - Expr::Bin(bin_expr) => { - if self.is_module & self.in_module_this { - // Some TSC polyfills use a pattern like below. - // We want to avoid marking these modules as CJS - // e.g. var _polyfill = (this && this.polyfill) || function () {} - if matches!(bin_expr.op, BinaryOp::LogicalAnd) && matches!(*bin_expr.left, Expr::This(..)) - { - match &*bin_expr.right { - Expr::Member(member_expr) => { - if matches!(*member_expr.obj, Expr::This(..)) - && matches!(member_expr.prop, MemberProp::Ident(..)) - { - return; - } - } - _ => {} - } - } - } - node.visit_children_with(self); - } _ => { node.visit_children_with(self); } @@ -823,7 +807,7 @@ impl Visit for Collect { } fn visit_this_expr(&mut self, node: &ThisExpr) { - if self.in_module_this { + if !self.is_module && self.in_module_this { self.has_cjs_exports = true; self.static_cjs_exports = false; self.add_bailout(node.span, BailoutReason::FreeExports); diff --git a/packages/transformers/js/core/src/hoist.rs b/packages/transformers/js/core/src/hoist.rs index d61acc4ab46..b248815a2de 100644 --- a/packages/transformers/js/core/src/hoist.rs +++ b/packages/transformers/js/core/src/hoist.rs @@ -1140,11 +1140,21 @@ mod tests { ); let mut parser = Parser::new_from(lexer); - match parser.parse_module() { - Ok(module) => swc_core::common::GLOBALS.set(&Globals::new(), || { + match parser.parse_program() { + Ok(program) => swc_core::common::GLOBALS.set(&Globals::new(), || { swc_core::ecma::transforms::base::helpers::HELPERS.set( &swc_core::ecma::transforms::base::helpers::Helpers::new(false), || { + let is_module = program.is_module(); + let module = match program { + Program::Module(module) => module, + Program::Script(script) => Module { + span: script.span, + shebang: None, + body: script.body.into_iter().map(ModuleItem::Stmt).collect(), + }, + }; + let unresolved_mark = Mark::fresh(Mark::root()); let global_mark = Mark::fresh(Mark::root()); let module = module.fold_with(&mut resolver(unresolved_mark, global_mark, false)); @@ -1155,7 +1165,7 @@ mod tests { Mark::fresh(Mark::root()), global_mark, true, - true, + is_module, ); module.visit_with(&mut collect); @@ -1445,6 +1455,7 @@ mod tests { // We want to avoid marking these modules as CJS let (collect, _code, _hoist) = parse( r#" + import 'something'; var __classPrivateFieldSet = (this && this.__classPrivateFieldSet) || function () {} "#, ); From 4ab7e558c7e7c18d5b9b9bd0179a867027a70497 Mon Sep 17 00:00:00 2001 From: mattcompiles Date: Fri, 20 Oct 2023 15:58:30 +1100 Subject: [PATCH 4/6] Address PR feedback --- packages/transformers/js/core/src/collect.rs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/packages/transformers/js/core/src/collect.rs b/packages/transformers/js/core/src/collect.rs index 45b077d8456..426a81fce36 100644 --- a/packages/transformers/js/core/src/collect.rs +++ b/packages/transformers/js/core/src/collect.rs @@ -692,12 +692,7 @@ impl Visit for Collect { return; } Expr::This(_this) => { - if self.is_module { - // Don't track this bindings if ESM syntax is present - return; - } - - if self.in_module_this { + if !self.is_module && self.in_module_this { handle_export!(); } else if !self.in_class { if let MemberProp::Ident(prop) = &node.prop { From 508b74d5a56715fbb447d29deba97db4a9b52f40 Mon Sep 17 00:00:00 2001 From: mattcompiles Date: Mon, 23 Oct 2023 15:36:20 +1100 Subject: [PATCH 5/6] Update packages/transformers/js/core/src/collect.rs Co-authored-by: Devon Govett --- packages/transformers/js/core/src/collect.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/transformers/js/core/src/collect.rs b/packages/transformers/js/core/src/collect.rs index 426a81fce36..e3d51423d1e 100644 --- a/packages/transformers/js/core/src/collect.rs +++ b/packages/transformers/js/core/src/collect.rs @@ -692,8 +692,10 @@ impl Visit for Collect { return; } Expr::This(_this) => { - if !self.is_module && self.in_module_this { - handle_export!(); + if self.in_module_this { + if !self.is_module { + handle_export!(); + } } else if !self.in_class { if let MemberProp::Ident(prop) = &node.prop { self.this_exprs.insert(id!(prop), (prop.clone(), node.span)); From c9647a5b35a5f946e7e8c9cd790d3b4e05406438 Mon Sep 17 00:00:00 2001 From: mattcompiles Date: Mon, 23 Oct 2023 15:39:53 +1100 Subject: [PATCH 6/6] Fix formatting --- packages/transformers/js/core/src/collect.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/transformers/js/core/src/collect.rs b/packages/transformers/js/core/src/collect.rs index e3d51423d1e..804d0a5c427 100644 --- a/packages/transformers/js/core/src/collect.rs +++ b/packages/transformers/js/core/src/collect.rs @@ -693,7 +693,7 @@ impl Visit for Collect { } Expr::This(_this) => { if self.in_module_this { - if !self.is_module { + if !self.is_module { handle_export!(); } } else if !self.in_class {