fix(compiler-core): properly handle for loop variable declarations in…

… expression transforms ref #11467 (comment)
vuejs · Aug 5, 2024 · 67bb820 · 67bb820
1 parent f3efff1
commit 67bb820
Show file tree

Hide file tree

Showing 3 changed files with 78 additions and 15 deletions.
diff --git a/packages/compiler-core/__tests__/transforms/__snapshots__/transformExpressions.spec.ts.snap b/packages/compiler-core/__tests__/transforms/__snapshots__/transformExpressions.spec.ts.snap
@@ -14,6 +14,21 @@ return function render(_ctx, _cache, $props, $setup, $data, $options) {
 }"
 `;
 
+exports[`compiler: expression transform > should allow leak of var declarations in for loop 1`] = `
+"const { openBlock: _openBlock, createElementBlock: _createElementBlock } = Vue
+
+return function render(_ctx, _cache) {
+  return (_openBlock(), _createElementBlock("div", {
+    onClick: () => {
+        for (var i = 0; i < _ctx.list.length; i++) {
+          _ctx.log(i)
+        }
+        _ctx.error(i)
+      }
+  }, null, 8 /* PROPS */, ["onClick"]))
+}"
+`;
+
 exports[`compiler: expression transform > should not prefix catch block param 1`] = `
 "const { openBlock: _openBlock, createElementBlock: _createElementBlock } = Vue
 
@@ -53,6 +68,7 @@ return function render(_ctx, _cache) {
         for (let i = 0; i < _ctx.list.length; i++) {
           _ctx.log(i)
         }
+        _ctx.error(_ctx.i)
       }
   }, null, 8 /* PROPS */, ["onClick"]))
 }"
@@ -67,6 +83,7 @@ return function render(_ctx, _cache) {
         for (const x in _ctx.list) {
           _ctx.log(x)
         }
+        _ctx.error(_ctx.x)
       }
   }, null, 8 /* PROPS */, ["onClick"]))
 }"
@@ -81,6 +98,7 @@ return function render(_ctx, _cache) {
         for (const x of _ctx.list) {
           _ctx.log(x)
         }
+        _ctx.error(_ctx.x)
       }
   }, null, 8 /* PROPS */, ["onClick"]))
 }"

diff --git a/packages/compiler-core/__tests__/transforms/transformExpressions.spec.ts b/packages/compiler-core/__tests__/transforms/transformExpressions.spec.ts
@@ -468,9 +468,11 @@ describe('compiler: expression transform', () => {
         for (const x in list) {
           log(x)
         }
+        error(x)
       }"/>`,
     )
-    expect(code).not.toMatch(`_ctx.x`)
+    expect(code).not.toMatch(`log(_ctx.x)`)
+    expect(code).toMatch(`error(_ctx.x)`)
     expect(code).toMatchSnapshot()
   })
 
@@ -480,9 +482,11 @@ describe('compiler: expression transform', () => {
         for (const x of list) {
           log(x)
         }
+        error(x)
       }"/>`,
     )
-    expect(code).not.toMatch(`_ctx.x`)
+    expect(code).not.toMatch(`log(_ctx.x)`)
+    expect(code).toMatch(`error(_ctx.x)`)
     expect(code).toMatchSnapshot()
   })
 
@@ -492,9 +496,25 @@ describe('compiler: expression transform', () => {
         for (let i = 0; i < list.length; i++) {
           log(i)
         }
+        error(i)
       }"/>`,
     )
-    expect(code).not.toMatch(`_ctx.i`)
+    expect(code).not.toMatch(`log(_ctx.i)`)
+    expect(code).toMatch(`error(_ctx.i)`)
+    expect(code).toMatchSnapshot()
+  })
+
+  test('should allow leak of var declarations in for loop', () => {
+    const { code } = compile(
+      `<div @click="() => {
+        for (var i = 0; i < list.length; i++) {
+          log(i)
+        }
+        error(i)
+      }"/>`,
+    )
+    expect(code).not.toMatch(`log(_ctx.i)`)
+    expect(code).not.toMatch(`error(_ctx.i)`)
     expect(code).toMatchSnapshot()
   })
 

diff --git a/packages/compiler-core/src/babelUtils.ts b/packages/compiler-core/src/babelUtils.ts
@@ -2,6 +2,9 @@
 // do not import runtime methods
 import type {
   BlockStatement,
+  ForInStatement,
+  ForOfStatement,
+  ForStatement,
   Function,
   Identifier,
   Node,
@@ -81,6 +84,10 @@ export function walkIdentifiers(
         for (const id of extractIdentifiers(node.param)) {
           markScopeIdentifier(node, id, knownIds)
         }
+      } else if (isForStatement(node)) {
+        walkForStatement(node, false, id =>
+          markScopeIdentifier(node, id, knownIds),
+        )
       }
     },
     leave(node: Node & { scopeIds?: Set<string> }, parent: Node | null) {
@@ -196,18 +203,36 @@ export function walkBlockDeclarations(
     ) {
       if (stmt.declare || !stmt.id) continue
       onIdent(stmt.id)
-    } else if (
-      stmt.type === 'ForOfStatement' ||
-      stmt.type === 'ForInStatement' ||
-      stmt.type === 'ForStatement'
-    ) {
-      const variable = stmt.type === 'ForStatement' ? stmt.init : stmt.left
-      if (variable && variable.type === 'VariableDeclaration') {
-        for (const decl of variable.declarations) {
-          for (const id of extractIdentifiers(decl.id)) {
-            onIdent(id)
-          }
-        }
+    } else if (isForStatement(stmt)) {
+      walkForStatement(stmt, true, onIdent)
+    }
+  }
+}
+
+function isForStatement(
+  stmt: Node,
+): stmt is ForStatement | ForOfStatement | ForInStatement {
+  return (
+    stmt.type === 'ForOfStatement' ||
+    stmt.type === 'ForInStatement' ||
+    stmt.type === 'ForStatement'
+  )
+}
+
+function walkForStatement(
+  stmt: ForStatement | ForOfStatement | ForInStatement,
+  isVar: boolean,
+  onIdent: (id: Identifier) => void,
+) {
+  const variable = stmt.type === 'ForStatement' ? stmt.init : stmt.left
+  if (
+    variable &&
+    variable.type === 'VariableDeclaration' &&
+    (variable.kind === 'var' ? isVar : !isVar)
+  ) {
+    for (const decl of variable.declarations) {
+      for (const id of extractIdentifiers(decl.id)) {
+        onIdent(id)
       }
     }
   }