fix: mark var declarations in loops as not constant (#15027)

* fix: babel-traverse constant flag in bindings

flag bindings in loops as not constant.

Fixes: #13760

* Only mark `var`s directly inside loops

* Fix CI error

Co-authored-by: Stephane Rufer <stephane@arista.com>

packages/babel-plugin-transform-destructuring/test/fixtures/assumption-iterableIsArray/for-in/output.js

@@ -5,7 +5,8 @@ for (var _ref in obj) {
 }
 
 for (var _ref2 in obj) {
-  name = _ref2[0];
-  value = _ref2[1];
+  var _ref3 = _ref2;
+  name = _ref3[0];
+  value = _ref3[1];
   print("Name: " + name + ", Value: " + value);
 }

packages/babel-plugin-transform-destructuring/test/fixtures/destructuring/for-in/output.js

@@ -7,9 +7,11 @@ for (var _ref in obj) {
 }
 
 for (var _ref3 in obj) {
-  var _ref4 = babelHelpers.slicedToArray(_ref3, 2);
+  var _ref4 = _ref3;
 
-  name = _ref4[0];
-  value = _ref4[1];
+  var _ref5 = babelHelpers.slicedToArray(_ref4, 2);
+
+  name = _ref5[0];
+  value = _ref5[1];
   print("Name: " + name + ", Value: " + value);
 }

packages/babel-plugin-transform-destructuring/test/fixtures/destructuring/for-of-shadowed-block-scoped/output.js

@@ -13,7 +13,8 @@ for (const _ref of [O]) {
 var _;
 
 for (var _ref2 of [O]) {
-  _ = _ref2[a];
+  var _ref3 = _ref2;
+  _ = _ref3[a];
   {
     const a = "A";
   }

packages/babel-plugin-transform-destructuring/test/fixtures/destructuring/for-of/output.js

@@ -7,10 +7,12 @@ for (var _ref of test.expectation.registers) {
 }
 
 for (var _ref3 of test.expectation.registers) {
-  var _ref4 = babelHelpers.slicedToArray(_ref3, 3);
+  var _ref4 = _ref3;
 
-  name = _ref4[0];
-  before = _ref4[1];
-  after = _ref4[2];
+  var _ref5 = babelHelpers.slicedToArray(_ref4, 3);
+
+  name = _ref5[0];
+  before = _ref5[1];
+  after = _ref5[2];
   void 0;
 }

packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/for-loop-2/input.js

@@ -0,0 +1,11 @@
+function render() {
+  const nodes = [];
+
+  for (let i = 0; i < 5; i++) {
+    const o = "foo";
+    const n = i;
+    nodes.push(<div>{n}</div>);
+  }
+
+  return nodes;
+}

packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/for-loop-2/output.js

@@ -0,0 +1,11 @@
+function render() {
+  const nodes = [];
+
+  for (let i = 0; i < 5; i++) {
+    const o = "foo";
+    const n = i;
+    nodes.push(<div>{n}</div>);
+  }
+
+  return nodes;
+}

packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/for-loop-3/input.js

@@ -0,0 +1,9 @@
+function render() {
+  const nodes = [];
+
+  for (const node of nodes) {
+    nodes.push(<div>{node}</div>);
+  }
+
+  return nodes;
+}

packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/for-loop-3/output.js

@@ -0,0 +1,9 @@
+function render() {
+  const nodes = [];
+
+  for (const node of nodes) {
+    nodes.push(<div>{node}</div>);
+  }
+
+  return nodes;
+}

packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/for-loop-4/input.js

@@ -0,0 +1,10 @@
+function render() {
+  const nodes = [];
+
+  for (const node of nodes) {
+    const n = node;
+    nodes.push(<div>{n}</div>);
+  }
+
+  return nodes;
+}

packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/for-loop-4/output.js

@@ -0,0 +1,10 @@
+function render() {
+  const nodes = [];
+
+  for (const node of nodes) {
+    const n = node;
+    nodes.push(<div>{n}</div>);
+  }
+
+  return nodes;
+}

packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/for-loop-5/input.js

@@ -0,0 +1,11 @@
+function render() {
+  const nodes = [];
+
+  for (let i = 0; i < 5; i++) {
+    const o = "foo";
+    const n = i;
+    nodes.push(<div>{o}</div>);
+  }
+
+  return nodes;
+}

packages/babel-plugin-transform-react-constant-elements/test/fixtures/constant-elements/for-loop-5/output.js

@@ -0,0 +1,11 @@
+function render() {
+  const nodes = [];
+
+  for (let i = 0; i < 5; i++) {
+    const o = "foo";
+    const n = i;
+    nodes.push(<div>{o}</div>);
+  }
+
+  return nodes;
+}

packages/babel-traverse/src/scope/binding.ts

@@ -44,6 +44,24 @@ export default class Binding {
     this.path = path;
     this.kind = kind;
 
+    if (
+      (kind === "var" || kind === "hoisted") &&
+      // https://github.com/rollup/rollup/issues/4654
+      // Rollup removes the path argument from this call. Add an
+      // unreachable IIFE (that rollup doesn't know is unreachable)
+      // with side effects, to prevent it from messing up with arguments.
+      // You can reproduce this with
+      //   BABEL_8_BREAKING=true make prepublish-build
+      isDeclaredInLoop(
+        path ||
+          (() => {
+            throw new Error("Internal Babel error: unreachable ");
+          })(),
+      )
+    ) {
+      this.reassign(path);
+    }
+
     this.clearValue();
   }
 
@@ -109,3 +127,21 @@ export default class Binding {
     this.referenced = !!this.references;
   }
 }
+
+function isDeclaredInLoop(path: NodePath) {
+  for (
+    let { parentPath, key } = path;
+    parentPath;
+    { parentPath, key } = parentPath
+  ) {
+    if (parentPath.isFunctionParent()) return false;
+    if (
+      parentPath.isWhile() ||
+      parentPath.isForXStatement() ||
+      (parentPath.isForStatement() && key === "body")
+    ) {
+      return true;
+    }
+  }
+  return false;
+}

packages/babel-traverse/test/scope.js

@@ -391,6 +391,44 @@ describe("scope", () => {
       ).toBe(false);
     });
 
+    it("variable constantness in loops", () => {
+      let scopePath = null;
+      const isAConstant = code => {
+        let path = getPath(code);
+        if (scopePath) path = path.get(scopePath);
+        return path.scope.getBinding("a").constant;
+      };
+
+      expect(isAConstant("for (_ of ns) { var a = 1; }")).toBe(false);
+      expect(isAConstant("for (_ in ns) { var a = 1; }")).toBe(false);
+      expect(isAConstant("for (;;) { var a = 1; }")).toBe(false);
+      expect(isAConstant("while (1) { var a = 1; }")).toBe(false);
+      expect(isAConstant("do { var a = 1; } while (1)")).toBe(false);
+
+      expect(isAConstant("for (var a of ns) {}")).toBe(false);
+      expect(isAConstant("for (var a in ns) {}")).toBe(false);
+      expect(isAConstant("for (var a;;) {}")).toBe(true);
+
+      scopePath = "body.0.body.expression";
+      expect(isAConstant("for (_ of ns) () => { var a = 1; }")).toBe(true);
+      expect(isAConstant("for (_ in ns) () => { var a = 1; }")).toBe(true);
+      expect(isAConstant("for (;;) () => { var a = 1; }")).toBe(true);
+      expect(isAConstant("while (1) () => { var a = 1; }")).toBe(true);
+      expect(isAConstant("do () => { var a = 1; }; while (1)")).toBe(true);
+
+      scopePath = "body.0.body";
+      expect(isAConstant("for (_ of ns) { let a = 1; }")).toBe(true);
+      expect(isAConstant("for (_ in ns) { let a = 1; }")).toBe(true);
+      expect(isAConstant("for (;;) { let a = 1; }")).toBe(true);
+      expect(isAConstant("while (1) { let a = 1; }")).toBe(true);
+      expect(isAConstant("do { let a = 1; } while (1)")).toBe(true);
+
+      scopePath = "body.0";
+      expect(isAConstant("for (let a of ns) {}")).toBe(true);
+      expect(isAConstant("for (let a in ns) {}")).toBe(true);
+      expect(isAConstant("for (let a;;) {}")).toBe(true);
+    });
+
     test("label", function () {
       expect(getPath("foo: { }").scope.getBinding("foo")).toBeUndefined();
       expect(getPath("foo: { }").scope.getLabel("foo").type).toBe(