From 0c457a82939dcf2bae02ebc2cee5b3bd66141de4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=B2=20Ribaudo?= Date: Tue, 8 Nov 2022 17:49:10 +0100 Subject: [PATCH 1/4] Add test --- .../test/fixtures/tdz/simple-assign-no-tdz/exec.js | 2 ++ .../test/fixtures/tdz/simple-assign-no-tdz/input.js | 2 ++ .../test/fixtures/tdz/simple-assign-no-tdz/output.js | 3 +++ 3 files changed, 7 insertions(+) create mode 100644 packages/babel-plugin-transform-block-scoping/test/fixtures/tdz/simple-assign-no-tdz/exec.js create mode 100644 packages/babel-plugin-transform-block-scoping/test/fixtures/tdz/simple-assign-no-tdz/input.js create mode 100644 packages/babel-plugin-transform-block-scoping/test/fixtures/tdz/simple-assign-no-tdz/output.js diff --git a/packages/babel-plugin-transform-block-scoping/test/fixtures/tdz/simple-assign-no-tdz/exec.js b/packages/babel-plugin-transform-block-scoping/test/fixtures/tdz/simple-assign-no-tdz/exec.js new file mode 100644 index 000000000000..6a90c9c28fbd --- /dev/null +++ b/packages/babel-plugin-transform-block-scoping/test/fixtures/tdz/simple-assign-no-tdz/exec.js @@ -0,0 +1,2 @@ +let i +i = 2; diff --git a/packages/babel-plugin-transform-block-scoping/test/fixtures/tdz/simple-assign-no-tdz/input.js b/packages/babel-plugin-transform-block-scoping/test/fixtures/tdz/simple-assign-no-tdz/input.js new file mode 100644 index 000000000000..6a90c9c28fbd --- /dev/null +++ b/packages/babel-plugin-transform-block-scoping/test/fixtures/tdz/simple-assign-no-tdz/input.js @@ -0,0 +1,2 @@ +let i +i = 2; diff --git a/packages/babel-plugin-transform-block-scoping/test/fixtures/tdz/simple-assign-no-tdz/output.js b/packages/babel-plugin-transform-block-scoping/test/fixtures/tdz/simple-assign-no-tdz/output.js new file mode 100644 index 000000000000..f26440f59d4f --- /dev/null +++ b/packages/babel-plugin-transform-block-scoping/test/fixtures/tdz/simple-assign-no-tdz/output.js @@ -0,0 +1,3 @@ +var i; +i; +i = 2; From 80c9c8790852ad403e503dd51a443107504496a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=B2=20Ribaudo?= Date: Tue, 8 Nov 2022 19:54:41 +0100 Subject: [PATCH 2/4] Only extract IDs for TDZ checks in assign when necessary --- .../src/tdz.ts | 156 +++++++++++------- .../tdz/simple-assign-no-tdz/output.js | 1 - 2 files changed, 92 insertions(+), 65 deletions(-) diff --git a/packages/babel-plugin-transform-block-scoping/src/tdz.ts b/packages/babel-plugin-transform-block-scoping/src/tdz.ts index 5f3bdcbcb0d2..53fc5ef90174 100644 --- a/packages/babel-plugin-transform-block-scoping/src/tdz.ts +++ b/packages/babel-plugin-transform-block-scoping/src/tdz.ts @@ -1,10 +1,7 @@ -import { types as t, template, type PluginPass } from "@babel/core"; +import { types as t, type PluginPass } from "@babel/core"; import type { NodePath, Scope, Visitor } from "@babel/traverse"; -function getTDZStatus( - refPath: NodePath, - bindingPath: NodePath, -) { +function getTDZStatus(refPath: NodePath, bindingPath: NodePath) { const executionStatus = bindingPath._guessExecutionStatusRelativeTo(refPath); if (executionStatus === "before") { @@ -16,15 +13,26 @@ function getTDZStatus( } } +const skipTDZChecks = new WeakSet(); + function buildTDZAssert( + status: "maybe" | "inside", node: t.Identifier | t.JSXIdentifier, state: TDZVisitorState, ) { - return t.callExpression(state.addHelper("temporalRef"), [ - // @ts-expect-error Fixme: we may need to handle JSXIdentifier - node, - t.stringLiteral(node.name), - ]); + if (status === "maybe") { + const clone = t.cloneNode(node); + skipTDZChecks.add(clone); + return t.callExpression(state.addHelper("temporalRef"), [ + // @ts-expect-error Fixme: we may need to handle JSXIdentifier + clone, + t.stringLiteral(node.name), + ]); + } else { + return t.callExpression(state.addHelper("tdz"), [ + t.stringLiteral(node.name), + ]); + } } function isReference( @@ -39,7 +47,41 @@ function isReference( return scope.getBindingIdentifier(node.name) === declared; } -const visitedMaybeTDZNodes = new WeakSet(); +type TDZReplacement = { status: "maybe" | "inside"; node: t.Expression }; +function getTDZReplacement( + path: NodePath, + state: TDZVisitorState, +): TDZReplacement | undefined; +function getTDZReplacement( + path: NodePath, + state: TDZVisitorState, + id: t.Identifier | t.JSXIdentifier, +): TDZReplacement | undefined; +function getTDZReplacement( + path: NodePath, + state: TDZVisitorState, + id: t.Identifier | t.JSXIdentifier = path.node as any, +): TDZReplacement | undefined { + if (!isReference(id, path.scope, state)) return; + + if (skipTDZChecks.has(id)) return; + skipTDZChecks.add(id); + + const bindingPath = path.scope.getBinding(id.name).path; + + if (bindingPath.isFunctionDeclaration()) return; + + const status = getTDZStatus(path, bindingPath); + if (status === "outside") return; + + if (status === "maybe") { + // add tdzThis to parent variable declarator so it's exploded + // @ts-expect-error todo(flow->ts): avoid mutations + bindingPath.parent._tdzThis = true; + } + + return { status, node: buildTDZAssert(status, id, state) }; +} export interface TDZVisitorState { tdzEnabled: boolean; @@ -50,72 +92,58 @@ export interface TDZVisitorState { export const visitor: Visitor = { ReferencedIdentifier(path, state) { if (!state.tdzEnabled) return; + if (path.parentPath.isUpdateExpression()) return; + // It will be handled after transforming the loop + if (path.parentPath.isFor({ left: path.node })) return; - const { node, parent, scope } = path; + const replacement = getTDZReplacement(path, state); + if (!replacement) return; - if (path.parentPath.isFor({ left: node })) return; - if (!isReference(node, scope, state)) return; + path.replaceWith(replacement.node); + }, - const bindingPath = scope.getBinding(node.name).path; + UpdateExpression(path, state) { + if (!state.tdzEnabled) return; - if (bindingPath.isFunctionDeclaration()) return; + const { node } = path; + // @ts-expect-error todo(flow->ts): avoid node mutations + if (node._ignoreBlockScopingTDZ) return; + // @ts-expect-error todo(flow->ts): avoid mutations + node._ignoreBlockScopingTDZ = true; - const status = getTDZStatus(path, bindingPath); - if (status === "outside") return; + const arg = path.get("argument"); + if (!arg.isIdentifier()) return; - if (status === "maybe") { - if (visitedMaybeTDZNodes.has(node)) { - return; - } - visitedMaybeTDZNodes.add(node); - const assert = buildTDZAssert(node, state); - - // add tdzThis to parent variable declarator so it's exploded - // @ts-expect-error todo(flow->ts): avoid mutations - bindingPath.parent._tdzThis = true; - - if (path.parentPath.isUpdateExpression()) { - // @ts-expect-error todo(flow->ts): avoid node mutations - if (parent._ignoreBlockScopingTDZ) return; - path.parentPath.replaceWith( - t.sequenceExpression([assert, parent as t.UpdateExpression]), - ); - } else { - path.replaceWith(assert); - } - } else if (status === "inside") { - path.replaceWith( - template.ast`${state.addHelper("tdz")}("${node.name}")` as t.Statement, - ); + const replacement = getTDZReplacement(path, state, arg.node); + if (!replacement) return; + + if (replacement.status === "maybe") { + path.insertBefore(replacement.node); + } else { + path.replaceWith(replacement.node); } }, - AssignmentExpression: { - exit(path, state) { - if (!state.tdzEnabled) return; - - const { node } = path; - - // @ts-expect-error todo(flow->ts): avoid node mutations - if (node._ignoreBlockScopingTDZ) return; + AssignmentExpression(path, state) { + if (!state.tdzEnabled) return; - const nodes = []; - const ids = path.getBindingIdentifiers(); + const { node } = path; + // @ts-expect-error todo(flow->ts): avoid node mutations + if (node._ignoreBlockScopingTDZ) return; + // @ts-expect-error todo(flow->ts): avoid mutations + node._ignoreBlockScopingTDZ = true; - for (const name of Object.keys(ids)) { - const id = ids[name]; + const nodes = []; + const ids = path.getBindingIdentifiers(); - if (isReference(id, path.scope, state)) { - nodes.push(id); - } + for (const name of Object.keys(ids)) { + const replacement = getTDZReplacement(path, state, ids[name]); + if (replacement) { + nodes.push(t.expressionStatement(replacement.node)); + if (replacement.status === "inside") break; } + } - if (nodes.length) { - // @ts-expect-error todo(flow->ts): avoid mutations - node._ignoreBlockScopingTDZ = true; - nodes.push(node); - path.replaceWithMultiple(nodes.map(n => t.expressionStatement(n))); - } - }, + if (nodes.length > 0) path.insertBefore(nodes); }, }; diff --git a/packages/babel-plugin-transform-block-scoping/test/fixtures/tdz/simple-assign-no-tdz/output.js b/packages/babel-plugin-transform-block-scoping/test/fixtures/tdz/simple-assign-no-tdz/output.js index f26440f59d4f..a39a71d895e8 100644 --- a/packages/babel-plugin-transform-block-scoping/test/fixtures/tdz/simple-assign-no-tdz/output.js +++ b/packages/babel-plugin-transform-block-scoping/test/fixtures/tdz/simple-assign-no-tdz/output.js @@ -1,3 +1,2 @@ var i; -i; i = 2; From c156ce5bd6a4a68cb37585388b6e4ccb9663661a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=B2=20Ribaudo?= Date: Tue, 8 Nov 2022 19:56:23 +0100 Subject: [PATCH 3/4] Avoid mutating nodes --- .../src/index.ts | 5 ++--- .../src/tdz.ts | 14 +++++--------- 2 files changed, 7 insertions(+), 12 deletions(-) diff --git a/packages/babel-plugin-transform-block-scoping/src/index.ts b/packages/babel-plugin-transform-block-scoping/src/index.ts index c1a42966e16c..d0cc2d707794 100644 --- a/packages/babel-plugin-transform-block-scoping/src/index.ts +++ b/packages/babel-plugin-transform-block-scoping/src/index.ts @@ -1,6 +1,6 @@ import { declare } from "@babel/helper-plugin-utils"; import type { NodePath, Visitor, Scope, Binding } from "@babel/traverse"; -import { visitor as tdzVisitor } from "./tdz"; +import { skipTDZChecks, visitor as tdzVisitor } from "./tdz"; import type { TDZVisitorState } from "./tdz"; import { traverse, template, types as t } from "@babel/core"; import type { PluginPass } from "@babel/core"; @@ -43,8 +43,7 @@ export default declare((api, opts: Options) => { t.cloneNode(decl.id), decl.init || scope.buildUndefinedNode(), ); - // @ts-expect-error todo(flow->ts): avoid mutations - assign._ignoreBlockScopingTDZ = true; + skipTDZChecks.add(assign); nodes.push(t.expressionStatement(assign)); decl.init = this.addHelper("temporalUndefined"); } diff --git a/packages/babel-plugin-transform-block-scoping/src/tdz.ts b/packages/babel-plugin-transform-block-scoping/src/tdz.ts index 53fc5ef90174..b329d4df6cd9 100644 --- a/packages/babel-plugin-transform-block-scoping/src/tdz.ts +++ b/packages/babel-plugin-transform-block-scoping/src/tdz.ts @@ -13,7 +13,7 @@ function getTDZStatus(refPath: NodePath, bindingPath: NodePath) { } } -const skipTDZChecks = new WeakSet(); +export const skipTDZChecks = new WeakSet(); function buildTDZAssert( status: "maybe" | "inside", @@ -106,10 +106,8 @@ export const visitor: Visitor = { if (!state.tdzEnabled) return; const { node } = path; - // @ts-expect-error todo(flow->ts): avoid node mutations - if (node._ignoreBlockScopingTDZ) return; - // @ts-expect-error todo(flow->ts): avoid mutations - node._ignoreBlockScopingTDZ = true; + if (skipTDZChecks.has(node)) return; + skipTDZChecks.add(node); const arg = path.get("argument"); if (!arg.isIdentifier()) return; @@ -128,10 +126,8 @@ export const visitor: Visitor = { if (!state.tdzEnabled) return; const { node } = path; - // @ts-expect-error todo(flow->ts): avoid node mutations - if (node._ignoreBlockScopingTDZ) return; - // @ts-expect-error todo(flow->ts): avoid mutations - node._ignoreBlockScopingTDZ = true; + if (skipTDZChecks.has(node)) return; + skipTDZChecks.add(node); const nodes = []; const ids = path.getBindingIdentifiers(); From db1db37ccf3504b6a0706ea7164a273fcc6bfdee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=B2=20Ribaudo?= Date: Tue, 8 Nov 2022 19:58:51 +0100 Subject: [PATCH 4/4] Add test for update expressions --- .../test/fixtures/tdz/update-expression/input.js | 13 +++++++++++++ .../test/fixtures/tdz/update-expression/output.js | 15 +++++++++++++++ 2 files changed, 28 insertions(+) create mode 100644 packages/babel-plugin-transform-block-scoping/test/fixtures/tdz/update-expression/input.js create mode 100644 packages/babel-plugin-transform-block-scoping/test/fixtures/tdz/update-expression/output.js diff --git a/packages/babel-plugin-transform-block-scoping/test/fixtures/tdz/update-expression/input.js b/packages/babel-plugin-transform-block-scoping/test/fixtures/tdz/update-expression/input.js new file mode 100644 index 000000000000..346d2bf0c6bb --- /dev/null +++ b/packages/babel-plugin-transform-block-scoping/test/fixtures/tdz/update-expression/input.js @@ -0,0 +1,13 @@ +maybeCallLater(function f() { + x++; + ++x; + x.p++; + ++x.p; +}); + +x++; +++x; +x.p++; +++x.p; + +let x; diff --git a/packages/babel-plugin-transform-block-scoping/test/fixtures/tdz/update-expression/output.js b/packages/babel-plugin-transform-block-scoping/test/fixtures/tdz/update-expression/output.js new file mode 100644 index 000000000000..8e99961fe95c --- /dev/null +++ b/packages/babel-plugin-transform-block-scoping/test/fixtures/tdz/update-expression/output.js @@ -0,0 +1,15 @@ +var x = babelHelpers.temporalUndefined; +maybeCallLater(function f() { + babelHelpers.temporalRef(x, "x") + x++; + babelHelpers.temporalRef(x, "x") + ++x; + babelHelpers.temporalRef(x, "x").p++; + ++babelHelpers.temporalRef(x, "x").p; +}); +babelHelpers.tdz("x"); +babelHelpers.tdz("x"); +babelHelpers.tdz("x").p++; +++babelHelpers.tdz("x").p; +x = void 0; +void 0;