Correctly transform spreads to use proper concat method (#9108)

* Correctly transform spreads to use proper concat method

* Add tests to ensure array spread clones elements

packages/babel-plugin-transform-destructuring/test/fixtures/destructuring/array-unpack-optimisation/output.js

@@ -21,7 +21,7 @@ var _ref4 = [a[1], a[0]];
 a[0] = _ref4[0];
 a[1] = _ref4[1];
 
-var _ref5 = babelHelpers.toConsumableArray(foo).concat([bar]),
+var _ref5 = [].concat(babelHelpers.toConsumableArray(foo), [bar]),
     a = _ref5[0],
     b = _ref5[1];
 

packages/babel-plugin-transform-spread/src/index.js

@@ -57,13 +57,29 @@ export default declare((api, options) => {
         if (!hasSpread(elements)) return;
 
         const nodes = build(elements, scope);
-        const first = nodes.shift();
+        let first = nodes[0];
 
-        if (nodes.length === 0 && first !== elements[0].argument) {
+        // If there is only one element in the ArrayExpression and
+        // the element was transformed (Array.prototype.slice.call or toConsumableArray)
+        // we know that the transformed code already takes care of cloning the array.
+        // So we can simply return that element.
+        if (nodes.length === 1 && first !== elements[0].argument) {
           path.replaceWith(first);
           return;
         }
 
+        // If the first element is a ArrayExpression we can directly call
+        // concat on it.
+        // `[..].concat(..)`
+        // If not then we have to use `[].concat(arr)` and not `arr.concat`
+        // because `arr` could be extended/modified (e.g. Immutable) and we do not know exactly
+        // what concat would produce.
+        if (!t.isArrayExpression(first)) {
+          first = t.arrayExpression([]);
+        } else {
+          nodes.shift();
+        }
+
         path.replaceWith(
           t.callExpression(
             t.memberExpression(first, t.identifier("concat")),

packages/babel-plugin-transform-spread/test/fixtures/regression/issue-8907/exec.js

@@ -0,0 +1,11 @@
+const arr = [];
+
+arr.concat = () => {
+    throw new Error('Should not be called');
+};
+let x;
+expect(() => {
+    x = [...arr];
+}).not.toThrow();
+
+expect(x).not.toBe(arr);

packages/babel-plugin-transform-spread/test/fixtures/regression/issue-8907/input.js

@@ -0,0 +1,7 @@
+const arr = [];
+
+arr.concat = () => {
+    throw new Error('Should not be called');
+};
+
+const x = [...arr];

packages/babel-plugin-transform-spread/test/fixtures/regression/issue-8907/output.js

@@ -0,0 +1,7 @@
+const arr = [];
+
+arr.concat = () => {
+  throw new Error('Should not be called');
+};
+
+const x = [].concat(arr);

packages/babel-plugin-transform-spread/test/fixtures/spread/arguments-array/exec.js

@@ -0,0 +1,9 @@
+// test that toConsumableArray clones the array.
+
+function foo() {
+  const x = [...arguments];
+
+  expect(x).not.toBe(arguments);
+}
+
+foo(1,2);

packages/babel-plugin-transform-spread/test/fixtures/spread/array-literal-first/output.js

@@ -1 +1 @@
-var lyrics = babelHelpers.toConsumableArray(parts).concat(["head", "and", "toes"]);
+var lyrics = [].concat(babelHelpers.toConsumableArray(parts), ["head", "and", "toes"]);

packages/babel-plugin-transform-spread/test/fixtures/spread/known-rest/output.js

@@ -3,5 +3,5 @@ function foo() {
     bar[_key] = arguments[_key];
   }
 
-  return bar.concat();
+  return [].concat(bar);
 }

packages/babel-plugin-transform-spread/test/fixtures/spread/single/exec.js

@@ -0,0 +1,7 @@
+// test that toConsumableArray clones the array.
+const arr = [];
+const foo = () => arr;
+
+const x = [...foo()];
+
+expect(x).not.toBe(arr);