From 6d4bbcbbec127de5ef5aa3031236ebe9715ccb16 Mon Sep 17 00:00:00 2001 From: liuxingbaoyu <30521560+liuxingbaoyu@users.noreply.github.com> Date: Thu, 19 May 2022 19:33:49 +0800 Subject: [PATCH 1/4] fix: Duplicate declaration in transformed for...of --- .../src/index.ts | 19 ++++++++++++++----- .../for-of-redeclaration-in-body/input.js | 4 ++++ .../for-of-redeclaration-in-body/output.js | 7 +++++++ .../for-of-redeclaration-in-body/input.js | 9 +++++++++ .../for-of-redeclaration-in-body/output.js | 15 +++++++++++++++ 5 files changed, 49 insertions(+), 5 deletions(-) create mode 100644 packages/babel-plugin-transform-for-of/test/fixtures/loose/for-of-redeclaration-in-body/input.js create mode 100644 packages/babel-plugin-transform-for-of/test/fixtures/loose/for-of-redeclaration-in-body/output.js diff --git a/packages/babel-plugin-transform-for-of/src/index.ts b/packages/babel-plugin-transform-for-of/src/index.ts index 732d5d2091f0..a76f39d7e083 100644 --- a/packages/babel-plugin-transform-for-of/src/index.ts +++ b/packages/babel-plugin-transform-for-of/src/index.ts @@ -234,10 +234,19 @@ export default declare((api, options: Options) => { ); } - // ensure that it's a block so we can take all its statements - path.ensureBlock(); - - (node.body as t.BlockStatement).body.unshift(declar); + let blockBody; + const body = path.get("body"); + if ( + body.isBlockStatement() && + Object.keys(path.getBindingIdentifiers()).some(id => + body.scope.hasOwnBinding(id), + ) + ) { + blockBody = t.blockStatement([declar, body.node]); + } else { + blockBody = t.toBlock(body.node); + blockBody.body.unshift(declar); + } const nodes = builder.build({ CREATE_ITERATOR_HELPER: state.addHelper(builder.helper), @@ -247,7 +256,7 @@ export default declare((api, options: Options) => { : null, STEP_KEY: t.identifier(stepKey), OBJECT: node.right, - BODY: node.body, + BODY: blockBody, }); const container = builder.getContainer(nodes); diff --git a/packages/babel-plugin-transform-for-of/test/fixtures/for-of-as-array/for-of-redeclaration-in-body/input.js b/packages/babel-plugin-transform-for-of/test/fixtures/for-of-as-array/for-of-redeclaration-in-body/input.js index ded052290862..3c695631ddf0 100644 --- a/packages/babel-plugin-transform-for-of/test/fixtures/for-of-as-array/for-of-redeclaration-in-body/input.js +++ b/packages/babel-plugin-transform-for-of/test/fixtures/for-of-as-array/for-of-redeclaration-in-body/input.js @@ -1,3 +1,7 @@ for (const elm of array) { const elm = 2; } + +for (let x of y) { + let x = 1; +} diff --git a/packages/babel-plugin-transform-for-of/test/fixtures/for-of-as-array/for-of-redeclaration-in-body/output.js b/packages/babel-plugin-transform-for-of/test/fixtures/for-of-as-array/for-of-redeclaration-in-body/output.js index f05d069d547d..33f0caa3ed53 100644 --- a/packages/babel-plugin-transform-for-of/test/fixtures/for-of-as-array/for-of-redeclaration-in-body/output.js +++ b/packages/babel-plugin-transform-for-of/test/fixtures/for-of-as-array/for-of-redeclaration-in-body/output.js @@ -4,3 +4,10 @@ for (let _i = 0, _array = array; _i < _array.length; _i++) { const elm = 2; } } + +for (let _i2 = 0, _y = y; _i2 < _y.length; _i2++) { + let x = _y[_i2]; + { + let x = 1; + } +} diff --git a/packages/babel-plugin-transform-for-of/test/fixtures/loose/for-of-redeclaration-in-body/input.js b/packages/babel-plugin-transform-for-of/test/fixtures/loose/for-of-redeclaration-in-body/input.js new file mode 100644 index 000000000000..cd2469acc16b --- /dev/null +++ b/packages/babel-plugin-transform-for-of/test/fixtures/loose/for-of-redeclaration-in-body/input.js @@ -0,0 +1,9 @@ +for (let x of y) { + let x = 1; + let y = 2; +} + +for (const x of y) { + const x = 1; + const y = 2; +} diff --git a/packages/babel-plugin-transform-for-of/test/fixtures/loose/for-of-redeclaration-in-body/output.js b/packages/babel-plugin-transform-for-of/test/fixtures/loose/for-of-redeclaration-in-body/output.js new file mode 100644 index 000000000000..d33003a65817 --- /dev/null +++ b/packages/babel-plugin-transform-for-of/test/fixtures/loose/for-of-redeclaration-in-body/output.js @@ -0,0 +1,15 @@ +for (var _iterator = babelHelpers.createForOfIteratorHelperLoose(y), _step; !(_step = _iterator()).done;) { + let x = _step.value; + { + let x = 1; + let y = 2; + } +} + +for (var _iterator2 = babelHelpers.createForOfIteratorHelperLoose(y), _step2; !(_step2 = _iterator2()).done;) { + const x = _step2.value; + { + const x = 1; + const y = 2; + } +} From e68b866610bdaf14fb8998b9b63d75e08c56ed67 Mon Sep 17 00:00:00 2001 From: liuxingbaoyu <30521560+liuxingbaoyu@users.noreply.github.com> Date: Thu, 19 May 2022 20:00:15 +0800 Subject: [PATCH 2/4] fix --- .../babel-plugin-transform-for-of/src/index.ts | 14 +++++++++++--- .../for-of-redeclaration-in-body/input.js | 2 +- .../for-of-redeclaration-in-body/output.js | 4 ++-- .../loose/for-of-redeclaration-in-body/input.js | 6 +++--- .../loose/for-of-redeclaration-in-body/output.js | 8 ++++---- 5 files changed, 21 insertions(+), 13 deletions(-) diff --git a/packages/babel-plugin-transform-for-of/src/index.ts b/packages/babel-plugin-transform-for-of/src/index.ts index a76f39d7e083..370dc40f14eb 100644 --- a/packages/babel-plugin-transform-for-of/src/index.ts +++ b/packages/babel-plugin-transform-for-of/src/index.ts @@ -167,7 +167,6 @@ export default declare((api, options: Options) => { }) as t.For; t.inherits(loop, node); - t.ensureBlock(loop); const iterationValue = t.memberExpression( t.cloneNode(right), @@ -175,13 +174,22 @@ export default declare((api, options: Options) => { true, ); + if ( + t.isBlockStatement(loop.body) && + Object.keys(path.getBindingIdentifiers()).some(id => + path.get("body").scope.hasOwnBinding(id), + ) + ) { + loop.body = t.blockStatement([loop.body]); + } else { + loop.body = t.toBlock(loop.body); + } + const left = node.left; if (t.isVariableDeclaration(left)) { left.declarations[0].init = iterationValue; - // @ts-expect-error todo(flow->ts): loop.body.body.unshift(left); } else { - // @ts-expect-error todo(flow->ts): loop.body.body.unshift( t.expressionStatement( t.assignmentExpression("=", left, iterationValue), diff --git a/packages/babel-plugin-transform-for-of/test/fixtures/for-of-as-array/for-of-redeclaration-in-body/input.js b/packages/babel-plugin-transform-for-of/test/fixtures/for-of-as-array/for-of-redeclaration-in-body/input.js index 3c695631ddf0..8073f40492f8 100644 --- a/packages/babel-plugin-transform-for-of/test/fixtures/for-of-as-array/for-of-redeclaration-in-body/input.js +++ b/packages/babel-plugin-transform-for-of/test/fixtures/for-of-as-array/for-of-redeclaration-in-body/input.js @@ -2,6 +2,6 @@ for (const elm of array) { const elm = 2; } -for (let x of y) { +for (let x of []) { let x = 1; } diff --git a/packages/babel-plugin-transform-for-of/test/fixtures/for-of-as-array/for-of-redeclaration-in-body/output.js b/packages/babel-plugin-transform-for-of/test/fixtures/for-of-as-array/for-of-redeclaration-in-body/output.js index 33f0caa3ed53..824a04ef70bd 100644 --- a/packages/babel-plugin-transform-for-of/test/fixtures/for-of-as-array/for-of-redeclaration-in-body/output.js +++ b/packages/babel-plugin-transform-for-of/test/fixtures/for-of-as-array/for-of-redeclaration-in-body/output.js @@ -5,8 +5,8 @@ for (let _i = 0, _array = array; _i < _array.length; _i++) { } } -for (let _i2 = 0, _y = y; _i2 < _y.length; _i2++) { - let x = _y[_i2]; +for (let _i2 = 0, _ref = []; _i2 < _ref.length; _i2++) { + let x = _ref[_i2]; { let x = 1; } diff --git a/packages/babel-plugin-transform-for-of/test/fixtures/loose/for-of-redeclaration-in-body/input.js b/packages/babel-plugin-transform-for-of/test/fixtures/loose/for-of-redeclaration-in-body/input.js index cd2469acc16b..abecc3bc5401 100644 --- a/packages/babel-plugin-transform-for-of/test/fixtures/loose/for-of-redeclaration-in-body/input.js +++ b/packages/babel-plugin-transform-for-of/test/fixtures/loose/for-of-redeclaration-in-body/input.js @@ -3,7 +3,7 @@ for (let x of y) { let y = 2; } -for (const x of y) { - const x = 1; - const y = 2; +for (let x of []) { + let x = 1; + let y = 2; } diff --git a/packages/babel-plugin-transform-for-of/test/fixtures/loose/for-of-redeclaration-in-body/output.js b/packages/babel-plugin-transform-for-of/test/fixtures/loose/for-of-redeclaration-in-body/output.js index d33003a65817..0947395ec708 100644 --- a/packages/babel-plugin-transform-for-of/test/fixtures/loose/for-of-redeclaration-in-body/output.js +++ b/packages/babel-plugin-transform-for-of/test/fixtures/loose/for-of-redeclaration-in-body/output.js @@ -6,10 +6,10 @@ for (var _iterator = babelHelpers.createForOfIteratorHelperLoose(y), _step; !(_s } } -for (var _iterator2 = babelHelpers.createForOfIteratorHelperLoose(y), _step2; !(_step2 = _iterator2()).done;) { - const x = _step2.value; +for (var _i = 0, _arr = []; _i < _arr.length; _i++) { + let x = _arr[_i]; { - const x = 1; - const y = 2; + let x = 1; + let y = 2; } } From 86f7931f3fe4d33cbcaa36e5e6c8fba9063285c7 Mon Sep 17 00:00:00 2001 From: liuxingbaoyu <30521560+liuxingbaoyu@users.noreply.github.com> Date: Fri, 20 May 2022 02:53:13 +0800 Subject: [PATCH 3/4] review --- .../src/index.ts | 72 +++++++------------ 1 file changed, 26 insertions(+), 46 deletions(-) diff --git a/packages/babel-plugin-transform-for-of/src/index.ts b/packages/babel-plugin-transform-for-of/src/index.ts index 370dc40f14eb..e9d85efbc668 100644 --- a/packages/babel-plugin-transform-for-of/src/index.ts +++ b/packages/babel-plugin-transform-for-of/src/index.ts @@ -9,6 +9,24 @@ export interface Options { loose?: boolean; } +function buildLoopBody(path, declar, newBody?) { + let block; + const bodyPath = path.get("body"); + const body = newBody ?? bodyPath.node; + if ( + t.isBlockStatement(body) && + Object.keys(path.getBindingIdentifiers()).some(id => + bodyPath.scope.hasOwnBinding(id), + ) + ) { + block = t.blockStatement([declar, body]); + } else { + block = t.toBlock(body); + block.body.unshift(declar); + } + return block; +} + export default declare((api, options: Options) => { api.assertVersion(7); @@ -90,20 +108,6 @@ export default declare((api, options: Options) => { ); } - let blockBody; - const body = path.get("body"); - if ( - body.isBlockStatement() && - Object.keys(path.getBindingIdentifiers()).some(id => - body.scope.hasOwnBinding(id), - ) - ) { - blockBody = t.blockStatement([assignment, body.node]); - } else { - blockBody = t.toBlock(body.node); - blockBody.body.unshift(assignment); - } - path.replaceWith( t.forStatement( t.variableDeclaration("let", inits), @@ -113,7 +117,7 @@ export default declare((api, options: Options) => { t.memberExpression(t.cloneNode(array), t.identifier("length")), ), t.updateExpression("++", t.cloneNode(i)), - blockBody, + buildLoopBody(path, assignment), ), ); }, @@ -174,29 +178,19 @@ export default declare((api, options: Options) => { true, ); - if ( - t.isBlockStatement(loop.body) && - Object.keys(path.getBindingIdentifiers()).some(id => - path.get("body").scope.hasOwnBinding(id), - ) - ) { - loop.body = t.blockStatement([loop.body]); - } else { - loop.body = t.toBlock(loop.body); - } - + let declar; const left = node.left; if (t.isVariableDeclaration(left)) { left.declarations[0].init = iterationValue; - loop.body.body.unshift(left); + declar = left; } else { - loop.body.body.unshift( - t.expressionStatement( - t.assignmentExpression("=", left, iterationValue), - ), + declar = t.expressionStatement( + t.assignmentExpression("=", left, iterationValue), ); } + loop.body = buildLoopBody(path, declar, loop.body); + return loop; } @@ -242,20 +236,6 @@ export default declare((api, options: Options) => { ); } - let blockBody; - const body = path.get("body"); - if ( - body.isBlockStatement() && - Object.keys(path.getBindingIdentifiers()).some(id => - body.scope.hasOwnBinding(id), - ) - ) { - blockBody = t.blockStatement([declar, body.node]); - } else { - blockBody = t.toBlock(body.node); - blockBody.body.unshift(declar); - } - const nodes = builder.build({ CREATE_ITERATOR_HELPER: state.addHelper(builder.helper), ITERATOR_HELPER: scope.generateUidIdentifier("iterator"), @@ -264,7 +244,7 @@ export default declare((api, options: Options) => { : null, STEP_KEY: t.identifier(stepKey), OBJECT: node.right, - BODY: blockBody, + BODY: buildLoopBody(path, declar), }); const container = builder.getContainer(nodes); From bfe2ccd20a24846c22b7b4ebecd463bd0ad1a8c2 Mon Sep 17 00:00:00 2001 From: liuxingbaoyu <30521560+liuxingbaoyu@users.noreply.github.com> Date: Fri, 20 May 2022 03:14:53 +0800 Subject: [PATCH 4/4] add type annotations --- packages/babel-plugin-transform-for-of/src/index.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/babel-plugin-transform-for-of/src/index.ts b/packages/babel-plugin-transform-for-of/src/index.ts index e9d85efbc668..b574640d7c5a 100644 --- a/packages/babel-plugin-transform-for-of/src/index.ts +++ b/packages/babel-plugin-transform-for-of/src/index.ts @@ -1,5 +1,6 @@ import { declare } from "@babel/helper-plugin-utils"; import { template, types as t } from "@babel/core"; +import type { NodePath } from "@babel/traverse"; import transformWithoutHelper from "./no-helper-implementation"; @@ -9,7 +10,11 @@ export interface Options { loose?: boolean; } -function buildLoopBody(path, declar, newBody?) { +function buildLoopBody( + path: NodePath, + declar: t.Statement, + newBody?: t.Statement | t.Expression, +) { let block; const bodyPath = path.get("body"); const body = newBody ?? bodyPath.node;