From bce44b4380e0da6b6d5c32e7f676aa0fc9c7414c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hu=C3=A1ng=20J=C3=B9nli=C3=A0ng?= Date: Wed, 10 Nov 2021 11:35:25 -0500 Subject: [PATCH 1/4] chore: add typings to object-rest-spread --- .../src/{index.js => index.ts} | 128 +++++++++++------- ...s => shouldStoreRHSInTemporaryVariable.ts} | 20 ++- packages/babel-traverse/src/path/ancestry.ts | 1 + packages/babel-traverse/src/scope/index.ts | 2 +- tsconfig.json | 4 + 5 files changed, 102 insertions(+), 53 deletions(-) rename packages/babel-plugin-proposal-object-rest-spread/src/{index.js => index.ts} (85%) rename packages/babel-plugin-proposal-object-rest-spread/src/{shouldStoreRHSInTemporaryVariable.js => shouldStoreRHSInTemporaryVariable.ts} (62%) diff --git a/packages/babel-plugin-proposal-object-rest-spread/src/index.js b/packages/babel-plugin-proposal-object-rest-spread/src/index.ts similarity index 85% rename from packages/babel-plugin-proposal-object-rest-spread/src/index.js rename to packages/babel-plugin-proposal-object-rest-spread/src/index.ts index 2fa53a716d62..30b46e0ebff0 100644 --- a/packages/babel-plugin-proposal-object-rest-spread/src/index.js +++ b/packages/babel-plugin-proposal-object-rest-spread/src/index.ts @@ -1,21 +1,24 @@ import { declare } from "@babel/helper-plugin-utils"; import syntaxObjectRestSpread from "@babel/plugin-syntax-object-rest-spread"; import { types as t } from "@babel/core"; +import type { PluginPass } from "@babel/core"; +import type { NodePath, Visitor, Scope } from "@babel/traverse"; import { convertFunctionParams } from "@babel/plugin-transform-parameters"; import { isRequired } from "@babel/helper-compilation-targets"; import compatData from "@babel/compat-data/corejs2-built-ins"; import shouldStoreRHSInTemporaryVariable from "./shouldStoreRHSInTemporaryVariable"; -// TODO: Remove in Babel 8 // @babel/types <=7.3.3 counts FOO as referenced in var { x: FOO }. // We need to detect this bug to know if "unused" means 0 or 1 references. -const ZERO_REFS = (() => { - const node = t.identifier("a"); - const property = t.objectProperty(t.identifier("key"), node); - const pattern = t.objectPattern([property]); +const ZERO_REFS = process.env.BABEL_8_BREAKING + ? 0 + : (() => { + const node = t.identifier("a"); + const property = t.objectProperty(t.identifier("key"), node); + const pattern = t.objectPattern([property]); - return t.isReferenced(node, property, pattern) ? 1 : 0; -})(); + return t.isReferenced(node, property, pattern) ? 1 : 0; + })(); export default declare((api, opts) => { api.assertVersion(7); @@ -36,7 +39,9 @@ export default declare((api, opts) => { const pureGetters = api.assumption("pureGetters") ?? loose; const setSpreadProperties = api.assumption("setSpreadProperties") ?? loose; - function getExtendsHelper(file) { + function getExtendsHelper( + file: PluginPass, + ): t.MemberExpression | t.Identifier { return useBuiltIns ? t.memberExpression(t.identifier("Object"), t.identifier("assign")) : file.addHelper("extends"); @@ -51,7 +56,7 @@ export default declare((api, opts) => { return foundRestElement; } - function hasObjectPatternRestElement(path) { + function hasObjectPatternRestElement(path: NodePath): boolean { let foundRestElement = false; visitRestElements(path, restElement => { if (restElement.parentPath.isObjectPattern()) { @@ -62,7 +67,10 @@ export default declare((api, opts) => { return foundRestElement; } - function visitRestElements(path, visitor) { + function visitRestElements( + path: NodePath, + visitor: (path: NodePath) => any, + ) { path.traverse({ Expression(path) { const parentType = path.parent.type; @@ -79,7 +87,7 @@ export default declare((api, opts) => { }); } - function hasSpread(node) { + function hasSpread(node: t.ObjectExpression): boolean { for (const prop of node.properties) { if (t.isSpreadElement(prop)) { return true; @@ -92,9 +100,10 @@ export default declare((api, opts) => { // were converted to stringLiterals or not // e.g. extracts {keys: ["a", "b", "3", ++x], allLiteral: false } // from ast of {a: "foo", b, 3: "bar", [++x]: "baz"} - function extractNormalizedKeys(path) { - const props = path.node.properties; - const keys = []; + function extractNormalizedKeys(node: t.ObjectPattern) { + // RestElement has been removed in createObjectRest + const props = node.properties as t.ObjectProperty[]; + const keys: t.Expression[] = []; let allLiteral = true; let hasTemplateLiteral = false; @@ -106,7 +115,14 @@ export default declare((api, opts) => { keys.push(t.cloneNode(prop.key)); hasTemplateLiteral = true; } else if (t.isLiteral(prop.key)) { - keys.push(t.stringLiteral(String(prop.key.value))); + keys.push( + t.stringLiteral( + String( + //@ts-ignore prop.key can not be a NullLiteral + prop.key.value, + ), + ), + ); } else { keys.push(t.cloneNode(prop.key)); allLiteral = false; @@ -118,8 +134,11 @@ export default declare((api, opts) => { // replaces impure computed keys with new identifiers // and returns variable declarators of these new identifiers - function replaceImpureComputedKeys(properties, scope) { - const impureComputedPropertyDeclarators = []; + function replaceImpureComputedKeys( + properties: NodePath[], + scope: Scope, + ) { + const impureComputedPropertyDeclarators: t.VariableDeclarator[] = []; for (const propPath of properties) { const key = propPath.get("key"); if (propPath.node.computed && !key.isPure()) { @@ -132,7 +151,7 @@ export default declare((api, opts) => { return impureComputedPropertyDeclarators; } - function removeUnusedExcludedKeys(path) { + function removeUnusedExcludedKeys(path: NodePath): void { const bindings = path.getOuterBindingIdentifierPaths(); Object.keys(bindings).forEach(bindingName => { @@ -148,7 +167,11 @@ export default declare((api, opts) => { } //expects path to an object pattern - function createObjectRest(path, file, objRef) { + function createObjectRest( + path: NodePath, + file: PluginPass, + objRef: t.Identifier | t.MemberExpression, + ): [t.VariableDeclarator[], t.LVal, t.CallExpression] { const props = path.get("properties"); const last = props[props.length - 1]; t.assertRestElement(last.node); @@ -156,11 +179,12 @@ export default declare((api, opts) => { last.remove(); const impureComputedPropertyDeclarators = replaceImpureComputedKeys( - path.get("properties"), + path.get("properties") as NodePath[], path.scope, ); - const { keys, allLiteral, hasTemplateLiteral } = - extractNormalizedKeys(path); + const { keys, allLiteral, hasTemplateLiteral } = extractNormalizedKeys( + path.node, + ); if (keys.length === 0) { return [ @@ -210,7 +234,11 @@ export default declare((api, opts) => { ]; } - function replaceRestElement(parentPath, paramPath, container) { + function replaceRestElement( + parentPath: NodePath, + paramPath: NodePath, + container?: t.VariableDeclaration[], + ): void { if (paramPath.isAssignmentPattern()) { replaceRestElement(parentPath, paramPath.get("left"), container); return; @@ -299,7 +327,7 @@ export default declare((api, opts) => { for (let i = 0; i < params.length; ++i) { const param = params[i]; if (paramsWithRestElement.has(i)) { - replaceRestElement(param.parentPath, param); + replaceRestElement(path, param); } } } else { @@ -360,14 +388,15 @@ export default declare((api, opts) => { } let ref = originalPath.node.init; - const refPropertyPath = []; + const refPropertyPath: NodePath[] = []; let kind; - path.findParent(path => { + path.findParent((path: NodePath): boolean => { if (path.isObjectProperty()) { refPropertyPath.unshift(path); } else if (path.isVariableDeclarator()) { - kind = path.parentPath.node.kind; + kind = (path.parentPath as NodePath).node + .kind; return true; } }); @@ -385,12 +414,17 @@ export default declare((api, opts) => { ); }); - const objectPatternPath = path.findParent(path => - path.isObjectPattern(), + //@ts-expect-error: findParent can not apply assertions on result shape + const objectPatternPath: NodePath = path.findParent( + path => path.isObjectPattern(), ); const [impureComputedPropertyDeclarators, argument, callExpression] = - createObjectRest(objectPatternPath, file, ref); + createObjectRest( + objectPatternPath, + file, + ref as t.MemberExpression, + ); if (pureGetters) { removeUnusedExcludedKeys(objectPatternPath); @@ -402,11 +436,9 @@ export default declare((api, opts) => { insertionPath.insertBefore(impureObjRefComputedDeclarators); - insertionPath.insertAfter( + insertionPath = insertionPath.insertAfter( t.variableDeclarator(argument, callExpression), - ); - - insertionPath = insertionPath.getSibling(insertionPath.key + 1); + )[0] as NodePath; path.scope.registerBinding(kind, insertionPath); @@ -433,7 +465,7 @@ export default declare((api, opts) => { const specifiers = []; - for (const name of Object.keys(path.getOuterBindingIdentifiers(path))) { + for (const name of Object.keys(path.getOuterBindingIdentifiers(true))) { specifiers.push( t.exportSpecifier(t.identifier(name), t.identifier(name)), ); @@ -449,7 +481,7 @@ export default declare((api, opts) => { // try {} catch ({a, ...b}) {} CatchClause(path) { const paramPath = path.get("param"); - replaceRestElement(paramPath.parentPath, paramPath); + replaceRestElement(path, paramPath); }, // ({a, ...b} = c); @@ -511,14 +543,15 @@ export default declare((api, opts) => { ]); path.ensureBlock(); + const body = node.body as t.BlockStatement; - if (node.body.body.length === 0 && path.isCompletionRecord()) { - node.body.body.unshift( + if (body.body.length === 0 && path.isCompletionRecord()) { + body.body.unshift( t.expressionStatement(scope.buildUndefinedNode()), ); } - node.body.body.unshift( + body.body.unshift( t.expressionStatement( t.assignmentExpression("=", left, t.cloneNode(temp)), ), @@ -533,8 +566,9 @@ export default declare((api, opts) => { ]); path.ensureBlock(); + const body = node.body as t.BlockStatement; - node.body.body.unshift( + body.body.unshift( t.variableDeclaration(node.left.kind, [ t.variableDeclarator(pattern, t.cloneNode(key)), ]), @@ -565,11 +599,13 @@ export default declare((api, opts) => { if (objectPatterns.length > 0) { const statementPath = path.getStatementParent(); + const statementNode = statementPath.node; + const kind = + statementNode.type === "VariableDeclaration" + ? statementNode.kind + : "var"; statementPath.insertAfter( - t.variableDeclaration( - statementPath.node.kind || "var", - objectPatterns, - ), + t.variableDeclaration(kind, objectPatterns), ); } }, @@ -627,7 +663,7 @@ export default declare((api, opts) => { ]); } - for (const prop of (path.node.properties: Array)) { + for (const prop of path.node.properties) { if (t.isSpreadElement(prop)) { make(); exp.arguments.push(prop.argument); @@ -640,6 +676,6 @@ export default declare((api, opts) => { path.replaceWith(exp); }, - }, + } as Visitor, }; }); diff --git a/packages/babel-plugin-proposal-object-rest-spread/src/shouldStoreRHSInTemporaryVariable.js b/packages/babel-plugin-proposal-object-rest-spread/src/shouldStoreRHSInTemporaryVariable.ts similarity index 62% rename from packages/babel-plugin-proposal-object-rest-spread/src/shouldStoreRHSInTemporaryVariable.js rename to packages/babel-plugin-proposal-object-rest-spread/src/shouldStoreRHSInTemporaryVariable.ts index 99099c0515d9..0db0689a455f 100644 --- a/packages/babel-plugin-proposal-object-rest-spread/src/shouldStoreRHSInTemporaryVariable.js +++ b/packages/babel-plugin-proposal-object-rest-spread/src/shouldStoreRHSInTemporaryVariable.ts @@ -1,5 +1,6 @@ import { types as t } from "@babel/core"; +const { isObjectProperty } = t; /** * This is a helper function to determine if we should create an intermediate variable * such that the RHS of an assignment is not duplicated. @@ -7,17 +8,24 @@ import { types as t } from "@babel/core"; * See https://github.com/babel/babel/pull/13711#issuecomment-914388382 for discussion * on further optimizations. */ -export default function shouldStoreRHSInTemporaryVariable(node) { +export default function shouldStoreRHSInTemporaryVariable(node: t.LVal) { if (t.isArrayPattern(node)) { const nonNullElements = node.elements.filter(element => element !== null); if (nonNullElements.length > 1) return true; else return shouldStoreRHSInTemporaryVariable(nonNullElements[0]); } else if (t.isObjectPattern(node)) { - if (node.properties.length > 1) return true; - else if (node.properties.length === 0) return false; - else return shouldStoreRHSInTemporaryVariable(node.properties[0]); - } else if (t.isObjectProperty(node)) { - return shouldStoreRHSInTemporaryVariable(node.value); + const { properties } = node; + if (properties.length > 1) return true; + else if (properties.length === 0) return false; + else { + const firstProperty = properties[0]; + if (isObjectProperty(firstProperty)) { + // the value of the property must be an LVal + return shouldStoreRHSInTemporaryVariable(firstProperty.value as t.LVal); + } else { + return shouldStoreRHSInTemporaryVariable(firstProperty); + } + } } else if (t.isAssignmentPattern(node)) { return shouldStoreRHSInTemporaryVariable(node.left); } else if (t.isRestElement(node)) { diff --git a/packages/babel-traverse/src/path/ancestry.ts b/packages/babel-traverse/src/path/ancestry.ts index 0ecbcbc1f7b2..0381427724f7 100644 --- a/packages/babel-traverse/src/path/ancestry.ts +++ b/packages/babel-traverse/src/path/ancestry.ts @@ -12,6 +12,7 @@ import NodePath from "./index"; */ export function findParent( + this: NodePath, callback: (path: NodePath) => boolean, ): NodePath | null { let path = this; diff --git a/packages/babel-traverse/src/scope/index.ts b/packages/babel-traverse/src/scope/index.ts index 9baaec406c5a..056dc22078be 100644 --- a/packages/babel-traverse/src/scope/index.ts +++ b/packages/babel-traverse/src/scope/index.ts @@ -983,7 +983,7 @@ export default class Scope { init?: t.Expression; unique?: boolean; _blockHoist?: number | undefined; - kind?: "var" | "let"; + kind?: "var" | "let" | "const"; }) { let path = this.path; diff --git a/tsconfig.json b/tsconfig.json index 2718a354726f..edf6a89408a2 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -30,6 +30,7 @@ "./packages/babel-plugin-bugfix-safari-id-destructuring-collision-in-function-expression/src/**/*.ts", "./packages/babel-plugin-bugfix-v8-spread-parameters-in-optional-chaining/src/**/*.ts", "./packages/babel-plugin-proposal-async-do-expressions/src/**/*.ts", + "./packages/babel-plugin-proposal-object-rest-spread/src/**/*.ts", "./packages/babel-plugin-syntax-async-do-expressions/src/**/*.ts", "./packages/babel-plugin-transform-block-scoping/src/**/*.ts", "./packages/babel-plugin-transform-classes/src/**/*.ts", @@ -129,6 +130,9 @@ "@babel/plugin-proposal-async-do-expressions": [ "./packages/babel-plugin-proposal-async-do-expressions/src" ], + "@babel/plugin-proposal-object-rest-spread": [ + "./packages/babel-plugin-proposal-object-rest-spread/src" + ], "@babel/plugin-syntax-async-do-expressions": [ "./packages/babel-plugin-syntax-async-do-expressions/src" ], From ae2f95dd533598aa07eddbdaddc3fbf9e19b468c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hu=C3=A1ng=20J=C3=B9nli=C3=A0ng?= Date: Wed, 10 Nov 2021 11:40:40 -0500 Subject: [PATCH 2/4] chore: bundle object-rest-spread package --- Gulpfile.mjs | 1 + 1 file changed, 1 insertion(+) diff --git a/Gulpfile.mjs b/Gulpfile.mjs index 5cad0e62c8de..bffce65ecf0c 100644 --- a/Gulpfile.mjs +++ b/Gulpfile.mjs @@ -461,6 +461,7 @@ function copyDts(packages) { const libBundles = [ "packages/babel-parser", + "packages/babel-plugin-proposal-object-rest-spread", "packages/babel-plugin-proposal-optional-chaining", "packages/babel-preset-react", "packages/babel-preset-typescript", From d4d1b984885f62440f6c2bcbf59a1c369a4e0149 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hu=C3=A1ng=20J=C3=B9nli=C3=A0ng?= Date: Wed, 10 Nov 2021 12:05:12 -0500 Subject: [PATCH 3/4] improve type inference --- .../src/index.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/babel-plugin-proposal-object-rest-spread/src/index.ts b/packages/babel-plugin-proposal-object-rest-spread/src/index.ts index 30b46e0ebff0..234df694870f 100644 --- a/packages/babel-plugin-proposal-object-rest-spread/src/index.ts +++ b/packages/babel-plugin-proposal-object-rest-spread/src/index.ts @@ -8,6 +8,7 @@ import { isRequired } from "@babel/helper-compilation-targets"; import compatData from "@babel/compat-data/corejs2-built-ins"; import shouldStoreRHSInTemporaryVariable from "./shouldStoreRHSInTemporaryVariable"; +const { isAssignmentPattern, isObjectProperty } = t; // @babel/types <=7.3.3 counts FOO as referenced in var { x: FOO }. // We need to detect this bug to know if "unused" means 0 or 1 references. const ZERO_REFS = process.env.BABEL_8_BREAKING @@ -73,12 +74,10 @@ export default declare((api, opts) => { ) { path.traverse({ Expression(path) { - const parentType = path.parent.type; + const { parent, key } = path; if ( - (parentType === "AssignmentPattern" && path.key === "right") || - (parentType === "ObjectProperty" && - path.parent.computed && - path.key === "key") + (isAssignmentPattern(parent) && key === "right") || + (isObjectProperty(parent) && parent.computed && key === "key") ) { path.skip(); } From 46869b0403c6b91317be4d99a95ebda6eb5e6ded Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hu=C3=A1ng=20J=C3=B9nli=C3=A0ng?= Date: Fri, 19 Nov 2021 14:43:03 -0500 Subject: [PATCH 4/4] address review comments --- .../src/index.ts | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/babel-plugin-proposal-object-rest-spread/src/index.ts b/packages/babel-plugin-proposal-object-rest-spread/src/index.ts index 234df694870f..34602f18ba7b 100644 --- a/packages/babel-plugin-proposal-object-rest-spread/src/index.ts +++ b/packages/babel-plugin-proposal-object-rest-spread/src/index.ts @@ -11,15 +11,14 @@ import shouldStoreRHSInTemporaryVariable from "./shouldStoreRHSInTemporaryVariab const { isAssignmentPattern, isObjectProperty } = t; // @babel/types <=7.3.3 counts FOO as referenced in var { x: FOO }. // We need to detect this bug to know if "unused" means 0 or 1 references. -const ZERO_REFS = process.env.BABEL_8_BREAKING - ? 0 - : (() => { - const node = t.identifier("a"); - const property = t.objectProperty(t.identifier("key"), node); - const pattern = t.objectPattern([property]); +if (!process.env.BABEL_8_BREAKING) { + const node = t.identifier("a"); + const property = t.objectProperty(t.identifier("key"), node); + const pattern = t.objectPattern([property]); - return t.isReferenced(node, property, pattern) ? 1 : 0; - })(); + // eslint-disable-next-line no-var + var ZERO_REFS = t.isReferenced(node, property, pattern) ? 1 : 0; +} export default declare((api, opts) => { api.assertVersion(7); @@ -156,7 +155,8 @@ export default declare((api, opts) => { Object.keys(bindings).forEach(bindingName => { const bindingParentPath = bindings[bindingName].parentPath; if ( - path.scope.getBinding(bindingName).references > ZERO_REFS || + path.scope.getBinding(bindingName).references > + (process.env.BABEL_8_BREAKING ? 0 : ZERO_REFS) || !bindingParentPath.isObjectProperty() ) { return;