From 67bb820904d53480fa37536fc3cb4109a4c6d3e2 Mon Sep 17 00:00:00 2001 From: Evan You Date: Mon, 5 Aug 2024 14:07:30 +0800 Subject: [PATCH] fix(compiler-core): properly handle for loop variable declarations in expression transforms ref https://github.com/vuejs/core/pull/11467#issuecomment-2263069794 --- .../transformExpressions.spec.ts.snap | 18 +++++++ .../transforms/transformExpressions.spec.ts | 26 ++++++++-- packages/compiler-core/src/babelUtils.ts | 49 ++++++++++++++----- 3 files changed, 78 insertions(+), 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 index 9ea502132..5a94de5a6 100644 --- 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 index af14265e2..c92814089 100644 --- 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( + `
`, + ) + 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 index 3144ed595..9b0a5ede6 100644 --- 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 }, 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) } } }