Preserve correct this for named/default imports (#7956)

parcel-bundler · May 17, 2022 · a1e9ce5 · a1e9ce5
1 parent fe58766
commit a1e9ce5
Show file tree

Hide file tree

Showing 9 changed files with 100 additions and 12 deletions.
diff --git a/packages/core/integration-tests/test/integration/js-import-this/index.js b/packages/core/integration-tests/test/integration/js-import-this/index.js
@@ -0,0 +1,17 @@
+import returnThisDefault, { returnThis } from "./other.js";
+import * as other from "./other.js";
+
+import returnThisWrappedDefault, { returnThis as returnThisWrapped } from "./other-wrapped.js";
+import * as otherWrapped from "./other-wrapped.js";
+
+let result = {
+  unwrappedNamed: returnThis(),
+  unwrappedDefault: returnThisDefault(),
+  unwrappedNamespace: other.returnThis(),
+  wrappedNamed: returnThisWrapped(),
+  wrappedDefault: returnThisWrappedDefault(),
+  wrappedNamespace: otherWrapped.returnThis(),
+};
+
+output = result;
+export default result;
diff --git a/packages/core/integration-tests/test/integration/js-import-this/other-wrapped.js b/packages/core/integration-tests/test/integration/js-import-this/other-wrapped.js
@@ -0,0 +1,13 @@
+import * as ns from "./other-wrapped.js";
+
+let y = typeof module !== "undefined" ? module : {};
+
+export function returnThis() {
+  if (y != null) {
+    return [this === undefined, this === ns];
+  } else {
+    throw new Error();
+  }
+}
+
+export default returnThis;
diff --git a/packages/core/integration-tests/test/integration/js-import-this/other.js b/packages/core/integration-tests/test/integration/js-import-this/other.js
@@ -0,0 +1,7 @@
+import * as ns from "./other.js";
+
+export function returnThis() {
+  return [this === undefined, this === ns];
+}
+
+export default returnThis;
diff --git a/packages/core/integration-tests/test/javascript.js b/packages/core/integration-tests/test/javascript.js
@@ -5267,6 +5267,21 @@ describe('javascript', function () {
     assert.deepEqual(res.default, 'x: 123');
   });
 
+  it('should call named imports without this context', async function () {
+    let b = await bundle(
+      path.join(__dirname, 'integration/js-import-this/index.js'),
+    );
+    let res = await run(b, {output: null}, {strict: true});
+    assert.deepEqual(res.default, {
+      unwrappedNamed: [true, false],
+      unwrappedDefault: [true, false],
+      unwrappedNamespace: [false, true],
+      wrappedNamed: [true, false],
+      wrappedDefault: [true, false],
+      wrappedNamespace: [false, true],
+    });
+  });
+
   it('should only replace free references to require', async () => {
     let b = await bundle(
       path.join(__dirname, 'integration/js-require-free/index.js'),

diff --git a/packages/core/integration-tests/test/output-formats.js b/packages/core/integration-tests/test/output-formats.js
@@ -191,7 +191,7 @@ describe('output formats', function () {
 
       let dist = await outputFS.readFile(b.getBundles()[0].filePath, 'utf8');
       assert(dist.includes('= require("lodash")'));
-      assert(dist.includes('= ($parcel$interopDefault('));
+      assert(dist.includes('= (0, ($parcel$interopDefault('));
       assert(/var {add: \s*\$.+?\$add\s*} = lodash/);
       assert.equal((await run(b)).bar, 6);
     });

diff --git a/packages/core/integration-tests/test/scope-hoisting.js b/packages/core/integration-tests/test/scope-hoisting.js
@@ -5284,6 +5284,23 @@ describe('scope hoisting', function () {
     assert.deepEqual(res, 'x: 123');
   });
 
+  it('should call named imports without this context', async function () {
+    let b = await bundle(
+      path.join(__dirname, 'integration/js-import-this/index.js'),
+    );
+    let res = await run(b, {output: null}, {strict: true});
+    assert.deepEqual(res, {
+      unwrappedNamed: [true, false],
+      unwrappedDefault: [true, false],
+      // TODO: unwrappedNamespace should actually be `[false, true]` but we optimize
+      // the `ns.foo` expression into a named import, so that namespace isn't available anymore.
+      unwrappedNamespace: [true, false],
+      wrappedNamed: [true, false],
+      wrappedDefault: [true, false],
+      wrappedNamespace: [false, true],
+    });
+  });
+
   it('should insert the prelude for sibling bundles referenced in HTML', async function () {
     let b = await bundle(
       path.join(

diff --git a/packages/core/integration-tests/test/transpilation.js b/packages/core/integration-tests/test/transpilation.js
@@ -121,7 +121,9 @@ describe('transpilation', function () {
       path.join(distDir, 'pure-comment.js'),
       'utf8',
     );
-    assert(file.includes('/*#__PURE__*/ _reactDefault.default.createElement'));
+    assert(
+      file.includes('/*#__PURE__*/ (0, _reactDefault.default).createElement'),
+    );
 
     let res = await run(b);
     assert(res.Foo());
@@ -190,7 +192,7 @@ describe('transpilation', function () {
 
     let file = await outputFS.readFile(b.getBundles()[0].filePath, 'utf8');
     assert(file.includes('react/jsx-dev-runtime'));
-    assert(file.includes('_jsxDevRuntime.jsxDEV("div"'));
+    assert(file.includes('(0, _jsxDevRuntime.jsxDEV)("div"'));
   });
 
   it('should support the automatic JSX runtime with preact >= 10.5', async function () {
@@ -200,7 +202,7 @@ describe('transpilation', function () {
 
     let file = await outputFS.readFile(b.getBundles()[0].filePath, 'utf8');
     assert(file.includes('preact/jsx-dev-runtime'));
-    assert(file.includes('_jsxDevRuntime.jsxDEV("div"'));
+    assert(file.includes('(0, _jsxDevRuntime.jsxDEV)("div"'));
   });
 
   it('should support the automatic JSX runtime with React ^16.14.0', async function () {
@@ -210,7 +212,7 @@ describe('transpilation', function () {
 
     let file = await outputFS.readFile(b.getBundles()[0].filePath, 'utf8');
     assert(file.includes('react/jsx-dev-runtime'));
-    assert(file.includes('_jsxDevRuntime.jsxDEV("div"'));
+    assert(file.includes('(0, _jsxDevRuntime.jsxDEV)("div"'));
   });
 
   it('should support the automatic JSX runtime with React 18 prereleases', async function () {
@@ -220,7 +222,7 @@ describe('transpilation', function () {
 
     let file = await outputFS.readFile(b.getBundles()[0].filePath, 'utf8');
     assert(file.includes('react/jsx-dev-runtime'));
-    assert(file.includes('_jsxDevRuntime.jsxDEV("div"'));
+    assert(file.includes('(0, _jsxDevRuntime.jsxDEV)("div"'));
   });
 
   it('should support the automatic JSX runtime with experimental React versions', async function () {
@@ -230,7 +232,7 @@ describe('transpilation', function () {
 
     let file = await outputFS.readFile(b.getBundles()[0].filePath, 'utf8');
     assert(file.includes('react/jsx-dev-runtime'));
-    assert(file.includes('_jsxDevRuntime.jsxDEV("div"'));
+    assert(file.includes('(0, _jsxDevRuntime.jsxDEV)("div"'));
   });
 
   it('should support the automatic JSX runtime with preact with alias', async function () {
@@ -243,7 +245,7 @@ describe('transpilation', function () {
 
     let file = await outputFS.readFile(b.getBundles()[0].filePath, 'utf8');
     assert(/\Wreact\/jsx-dev-runtime\W/.test(file));
-    assert(file.includes('_jsxDevRuntime.jsxDEV("div"'));
+    assert(file.includes('(0, _jsxDevRuntime.jsxDEV)("div"'));
   });
 
   it('should support the automatic JSX runtime with explicit tsconfig.json', async function () {
@@ -253,7 +255,7 @@ describe('transpilation', function () {
 
     let file = await outputFS.readFile(b.getBundles()[0].filePath, 'utf8');
     assert(file.includes('preact/jsx-dev-runtime'));
-    assert(file.includes('_jsxDevRuntime.jsxDEV("div"'));
+    assert(file.includes('(0, _jsxDevRuntime.jsxDEV)("div"'));
   });
 
   it('should support explicit JSX pragma in tsconfig.json', async function () {

diff --git a/packages/transformers/js/core/src/hoist.rs b/packages/transformers/js/core/src/hoist.rs
@@ -668,6 +668,17 @@ impl<'a> Fold for Hoist<'a> {
           }
         }
       }
+      Expr::Ident(ident) => {
+        if let Some(Import { specifier, .. }) = self.collect.imports.get(&id!(ident)) {
+          if specifier != "*" {
+            return Expr::Seq(SeqExpr {
+              span: ident.span,
+              exprs: vec![0.into(), Box::new(Expr::Ident(ident.fold_with(self)))],
+            });
+          }
+        }
+        return Expr::Ident(ident.fold_with(self));
+      }
       _ => {}
     }
 

diff --git a/packages/transformers/js/core/src/modules.rs b/packages/transformers/js/core/src/modules.rs
@@ -230,9 +230,15 @@ impl ESMFold {
       self.get_require_name(source, DUMMY_SP)
     };
 
-    Expr::Member(MemberExpr {
-      obj: Box::new(Expr::Ident(obj)),
-      prop: MemberProp::Ident(Ident::new(imported.clone(), DUMMY_SP)),
+    Expr::Seq(SeqExpr {
+      exprs: vec![
+        0.into(),
+        Box::new(Expr::Member(MemberExpr {
+          obj: Box::new(Expr::Ident(obj)),
+          prop: MemberProp::Ident(Ident::new(imported.clone(), DUMMY_SP)),
+          span,
+        })),
+      ],
       span,
     })
   }