Don't throw when destructuring into a var named as an import (#10628)

nicolo-ribaudo · web-flow · commit 43aa7e262c6e · 2019-11-04T22:50:36.000+01:00
diff --git a/packages/babel-helper-module-transforms/src/rewrite-live-references.js b/packages/babel-helper-module-transforms/src/rewrite-live-references.js
@@ -224,6 +224,10 @@ const rewriteReferencesVisitor = {
       seen.add(path.node);
 
       const left = path.get("left");
+
+      // No change needed
+      if (left.isMemberExpression()) return;
+
       if (left.isIdentifier()) {
         // Simple update-assign foo += 1; export { foo };
         // =>   exports.foo =  (foo += 1);
@@ -234,9 +238,9 @@ const rewriteReferencesVisitor = {
           return;
         }
 
-        const exportedNames = exported.get(localName) || [];
+        const exportedNames = exported.get(localName);
         const importData = imported.get(localName);
-        if (exportedNames.length > 0 || importData) {
+        if (exportedNames?.length > 0 || importData) {
           assert(path.node.operator === "=", "Path was not simplified");
 
           const assignment = path.node;
@@ -259,13 +263,14 @@ const rewriteReferencesVisitor = {
           );
           requeueInParent(path);
         }
-      } else if (left.isMemberExpression()) {
-        // No change needed
       } else {
         const ids = left.getOuterBindingIdentifiers();
-        const id = Object.keys(ids)
-          .filter(localName => imported.has(localName))
-          .pop();
+        const programScopeIds = Object.keys(ids).filter(
+          localName =>
+            scope.getBinding(localName) === path.scope.getBinding(localName),
+        );
+        const id = programScopeIds.find(localName => imported.has(localName));
+
         if (id) {
           path.node.right = t.sequenceExpression([
             path.node.right,
@@ -276,14 +281,7 @@ const rewriteReferencesVisitor = {
         // Complex ({a, b, c} = {}); export { a, c };
         // =>   ({a, b, c} = {}), (exports.a = a, exports.c = c);
         const items = [];
-        Object.keys(ids).forEach(localName => {
-          // redeclared in this scope
-          if (
-            scope.getBinding(localName) !== path.scope.getBinding(localName)
-          ) {
-            return;
-          }
-
+        programScopeIds.forEach(localName => {
           const exportedNames = exported.get(localName) || [];
           if (exportedNames.length > 0) {
             items.push(
diff --git a/packages/babel-plugin-transform-modules-commonjs/test/fixtures/misc/import-shadowed-assign/input.mjs b/packages/babel-plugin-transform-modules-commonjs/test/fixtures/misc/import-shadowed-assign/input.mjs
@@ -0,0 +1,12 @@
+import { foo } from "x";
+
+function f(foo) {
+  foo = 2;
+  [foo] = [];
+  ({ foo } = {});
+}
+
+
+foo = 2;
+[foo] = [];
+({ foo } = {});
diff --git a/packages/babel-plugin-transform-modules-commonjs/test/fixtures/misc/import-shadowed-assign/output.js b/packages/babel-plugin-transform-modules-commonjs/test/fixtures/misc/import-shadowed-assign/output.js
@@ -0,0 +1,23 @@
+"use strict";
+
+var _x = require("x");
+
+function f(foo) {
+  foo = 2;
+  [foo] = [];
+  ({
+    foo
+  } = {});
+}
+
+_x.foo = (2, function () {
+  throw new Error('"' + "foo" + '" is read-only.');
+}());
+[foo] = ([], function () {
+  throw new Error('"' + "foo" + '" is read-only.');
+}());
+({
+  foo
+} = ({}, function () {
+  throw new Error('"' + "foo" + '" is read-only.');
+}()));
