Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve traverse typings #14648

Merged
merged 16 commits into from Jun 21, 2022
4 changes: 2 additions & 2 deletions packages/babel-core/src/transformation/normalize-file.ts
Expand Up @@ -107,9 +107,9 @@ const EXTERNAL_SOURCEMAP_REGEX =

function extractCommentsFromList(
regex: RegExp,
comments: ReadonlyArray<t.Comment>,
comments: t.Comment[],
lastComment: string | null,
): [ReadonlyArray<t.Comment>, string | null] {
): [t.Comment[], string | null] {
if (comments) {
comments = comments.filter(({ value }) => {
if (regex.test(value)) {
Expand Down
8 changes: 1 addition & 7 deletions packages/babel-generator/src/generators/methods.ts
Expand Up @@ -37,13 +37,7 @@ export function _parameters(

export function _param(
this: Printer,
parameter:
| t.Function["params"][number]
| t.TSIndexSignature["parameters"][number]
| t.TSDeclareMethod["params"][number]
| t.TSDeclareFunction["params"][number]
| t.TSFunctionType["parameters"][number]
| t.TSConstructorType["parameters"][number],
parameter: t.Identifier | t.RestElement | t.Pattern | t.TSParameterProperty,
parent?:
| t.Function
| t.TSIndexSignature
Expand Down
18 changes: 5 additions & 13 deletions packages/babel-generator/src/generators/statements.ts
Expand Up @@ -50,22 +50,14 @@ export function IfStatement(this: Printer, node: t.IfStatement) {
}

// Recursively get the last statement.
function getLastStatement(
statement: Exclude<t.Statement, t.BreakStatement>,
): t.Statement {
if (
!isStatement(
// @ts-ignore body is not in BreakStatement
statement.body,
)
) {
function getLastStatement(statement: t.Statement): t.Statement {
// @ts-ignore: If statement.body is empty or not a Node, isStatement will return false
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have reverted some changes suggested before since getLastStatement does accept a BreakStatement, it will return the statement itself as the "last statement".

const { body } = statement;
if (isStatement(body) === false) {
return statement;
}

return getLastStatement(
// @ts-ignore body is not in BreakStatement
statement.body,
);
return getLastStatement(body);
}

export function ForStatement(this: Printer, node: t.ForStatement) {
Expand Down
10 changes: 6 additions & 4 deletions packages/babel-generator/src/generators/typescript.ts
Expand Up @@ -239,9 +239,10 @@ export function tsPrintFunctionOrConstructorType(
) {
const { typeParameters } = node;
const parameters = process.env.BABEL_8_BREAKING
? // @ts-expect-error Babel 8 AST shape
? // @ts-ignore Babel 8 AST shape
node.params
: node.parameters;
: // @ts-ignore Babel 7 AST shape
node.parameters;
this.print(typeParameters, node);
this.token("(");
this._parameters(parameters, node);
Expand All @@ -250,9 +251,10 @@ export function tsPrintFunctionOrConstructorType(
this.token("=>");
this.space();
const returnType = process.env.BABEL_8_BREAKING
? // @ts-expect-error Babel 8 AST shape
? // @ts-ignore Babel 8 AST shape
node.returnType
: node.typeAnnotation;
: // @ts-ignore Babel 7 AST shape
node.typeAnnotation;
this.print(returnType.typeAnnotation, node);
}

Expand Down
3 changes: 1 addition & 2 deletions packages/babel-generator/src/printer.ts
Expand Up @@ -669,9 +669,8 @@ class Printer {
_printComment(comment: t.Comment, skipNewLines?: boolean) {
if (!this.format.shouldPrintComment(comment.value)) return;

// Some plugins use this to mark comments as removed using the AST-root 'comments' property,
// Some plugins (such as flow-strip-types) use this to mark comments as removed using the AST-root 'comments' property,
// where they can't manually mutate the AST node comment lists.
// @ts-ignore todo: which plugin?
if (comment.ignore) return;

if (this._printedComments.has(comment)) return;
Expand Down
Expand Up @@ -107,6 +107,7 @@ interface PrivateNameVisitorState {
function privateNameVisitorFactory<S>(
visitor: Visitor<PrivateNameVisitorState & S>,
) {
// @ts-ignore Fixme: TS complains _exploded: boolean does not satisfy visitor functions
const privateNameVisitor: Visitor<PrivateNameVisitorState & S> = {
...visitor,

Expand Down
Expand Up @@ -580,7 +580,9 @@ function removeModuleDeclarations(programPath: NodePath<t.Program>) {
) {
// @ts-expect-error todo(flow->ts): avoid mutations
declaration._blockHoist = child.node._blockHoist;
child.replaceWith(declaration);
child.replaceWith(
declaration as NodePath<t.FunctionDeclaration | t.ClassDeclaration>,
);
} else {
// These should have been removed by the nameAnonymousExports() call.
throw declaration.buildCodeFrameError(
Expand Down
Expand Up @@ -51,7 +51,7 @@ export default function splitExportDeclaration(
}

const updatedDeclaration = standaloneDeclaration
? declaration
? declaration.node
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a bugfix, did it change any observable behavior?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not a bug fix. path.replaceWith accepts either Node or NodePath, but the current signature

replaceWith<R extends t.Node>(
  this: NodePath,
  replacementPath: R | NodePath<R>,
): [NodePath<R>]

does not support NodePath<A> | B. So here I ensure updatedDeclaration is always a Node.

: variableDeclaration("var", [
variableDeclarator(
cloneNode(id),
Expand Down
Expand Up @@ -33,14 +33,16 @@ type Module = {
};

if (!process.env.BABEL_8_BREAKING) {
// Introduced in Node.js 8
// Introduced in Node.js 10
if (!assert.rejects) {
assert.rejects = async function (block, validateError) {
try {
await block();
await (typeof block === "function" ? block() : block);
return Promise.reject(new Error("Promise not rejected"));
} catch (error) {
if (!validateError(error)) {
// @ts-ignore Fixme: validateError can be a string | object
// see https://nodejs.org/api/assert.html#assertrejectsasyncfn-error-message
if (typeof validateError === "function" && !validateError(error)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TS caught some bugs in our polyfill. We can fix this in a separate PR or leave it as-is.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It works well enough for what we need 😛

return Promise.reject(
new Error("Promise rejected with invalid error"),
);
Expand Down
Expand Up @@ -3,7 +3,7 @@ import remapAsyncToGenerator from "@babel/helper-remap-async-to-generator";
import syntaxAsyncGenerators from "@babel/plugin-syntax-async-generators";
import { types as t } from "@babel/core";
import type { PluginPass } from "@babel/core";
import type { Visitor } from "@babel/traverse";
import type { NodePath, Visitor } from "@babel/traverse";
import rewriteForAwait from "./for-await";

export default declare(api => {
Expand All @@ -29,7 +29,7 @@ export default declare(api => {
path.skip();
},

ForOfStatement(path, { file }) {
ForOfStatement(path: NodePath<t.ForOfStatement>, { file }) {
const { node } = path;
if (!node.await) return;

Expand All @@ -49,7 +49,7 @@ export default declare(api => {
}

// push the rest of the original loop body onto our new body
block.body.push(...(node.body as t.BlockStatement).body);
block.body.push(...path.node.body.body);

t.inherits(loop, node);
t.inherits(loop.body, node.body);
Expand Down
Expand Up @@ -143,7 +143,10 @@ function replaceClassWithVar(
t.sequenceExpression([newClassExpr, varId]),
);

return [t.cloneNode(varId), newPath.get("expressions.0")];
return [
t.cloneNode(varId),
newPath.get("expressions.0") as NodePath<t.ClassExpression>,
];
}
}

Expand Down
Expand Up @@ -10,7 +10,8 @@ import { convertFunctionParams } from "@babel/plugin-transform-parameters";
import { unshiftForXStatementBody } from "@babel/plugin-transform-destructuring";

import type { PluginPass } from "@babel/core";
import type { Visitor } from "@babel/traverse";
import type { NodePath, Visitor } from "@babel/traverse";
import type * as t from "@babel/types";

export default declare(function ({ assertVersion, assumption, types: t }) {
assertVersion("^7.17.0");
Expand Down Expand Up @@ -51,7 +52,10 @@ export default declare(function ({ assertVersion, assumption, types: t }) {
const { params: transformedParams, variableDeclaration } =
buildVariableDeclarationFromParams(paramsAfterIndex, scope);

path.get("body").unshiftContainer("body", variableDeclaration);
(path.get("body") as NodePath<t.BlockStatement>).unshiftContainer(
"body",
variableDeclaration,
);
params.push(...transformedParams);
// preserve function.length
// (b, p1) => {}
Expand Down
Expand Up @@ -540,7 +540,7 @@ export default declare((api, opts: Options) => {
},

// taken from transform-destructuring/src/index.js#visitor
ForXStatement(path) {
ForXStatement(path: NodePath<t.ForXStatement>) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be automatically inferred given the visitor name? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, TS already inferred the path type, however, the TS assertions in ensureBlock requires that the path has an explicit type annotation. I think it is a TS limitation. See also #14648 (comment).

const { node, scope } = path;
const leftPath = path.get("left");
const left = node.left;
Expand All @@ -558,7 +558,7 @@ export default declare((api, opts: Options) => {
]);

path.ensureBlock();
const body = node.body as t.BlockStatement;
const body = path.node.body;

if (body.body.length === 0 && path.isCompletionRecord()) {
body.body.unshift(
Expand Down
5 changes: 3 additions & 2 deletions packages/babel-plugin-transform-block-scoping/src/index.ts
Expand Up @@ -62,12 +62,12 @@ export default declare((api, opts: Options) => {
}
},

Loop(path, state) {
Loop(path: NodePath<t.Loop>, state) {
const { parent, scope } = path;
path.ensureBlock();
const blockScoping = new BlockScoping(
path,
path.get("body") as NodePath<t.BlockStatement>,
path.get("body"),
parent,
scope,
throwIfClosureRequired,
Expand Down Expand Up @@ -534,6 +534,7 @@ class BlockScoping {
]),
);
} else if (violation.isForXStatement()) {
// @ts-expect-error TS requires explicit annotation of "violation"
violation.ensureBlock();
violation
.get("left")
Expand Down
5 changes: 3 additions & 2 deletions packages/babel-plugin-transform-destructuring/src/index.ts
Expand Up @@ -8,6 +8,7 @@ import {
type DestructuringTransformerNode,
} from "./util";
export { buildObjectExcludingKeys, unshiftForXStatementBody } from "./util";
import type { NodePath } from "@babel/traverse";

/**
* Test if a VariableDeclaration's declarations contains any Patterns.
Expand Down Expand Up @@ -68,7 +69,7 @@ export default declare((api, options: Options) => {
path.scope.crawl();
},

ForXStatement(path) {
ForXStatement(path: NodePath<t.ForXStatement>) {
const { node, scope } = path;
const left = node.left;

Expand All @@ -82,7 +83,7 @@ export default declare((api, options: Options) => {
]);

path.ensureBlock();
const statementBody = (node.body as t.BlockStatement).body;
const statementBody = path.node.body.body;
const nodes = [];
// todo: the completion of a for statement can only be observed from
// a do block (or eval that we don't support),
Expand Down
6 changes: 3 additions & 3 deletions packages/babel-plugin-transform-modules-amd/src/index.ts
Expand Up @@ -32,9 +32,9 @@ function injectWrapper(
const { body, directives } = path.node;
path.node.directives = [];
path.node.body = [];
const amdFactoryCall = (
path.pushContainer("body", wrapper)[0] as NodePath<t.ExpressionStatement>
).get("expression") as NodePath<t.CallExpression>;
const amdFactoryCall = path
.pushContainer("body", wrapper)[0]
.get("expression") as NodePath<t.CallExpression>;
const amdFactoryCallArgs = amdFactoryCall.get("arguments");
const amdFactory = (
amdFactoryCallArgs[
Expand Down
2 changes: 1 addition & 1 deletion packages/babel-plugin-transform-modules-umd/src/index.ts
Expand Up @@ -264,7 +264,7 @@ export default declare((api, options: Options) => {
])[0] as NodePath<t.ExpressionStatement>;
const umdFactory = (
umdWrapper.get("expression.arguments")[1] as NodePath<t.Function>
).get("body");
).get("body") as NodePath<t.BlockStatement>;
umdFactory.pushContainer("directives", directives);
umdFactory.pushContainer("body", body);
},
Expand Down
10 changes: 5 additions & 5 deletions packages/babel-plugin-transform-parameters/src/params.ts
@@ -1,25 +1,25 @@
import { template, types as t } from "@babel/core";
import type { NodePath, Scope, Visitor } from "@babel/traverse";

const buildDefaultParam = template(`
const buildDefaultParam = template.statement(`
let VARIABLE_NAME =
arguments.length > ARGUMENT_KEY && arguments[ARGUMENT_KEY] !== undefined ?
arguments[ARGUMENT_KEY]
:
DEFAULT_VALUE;
`);

const buildLooseDefaultParam = template(`
const buildLooseDefaultParam = template.statement(`
if (ASSIGNMENT_IDENTIFIER === UNDEFINED) {
ASSIGNMENT_IDENTIFIER = DEFAULT_VALUE;
}
`);

const buildLooseDestructuredDefaultParam = template(`
const buildLooseDestructuredDefaultParam = template.statement(`
let ASSIGNMENT_IDENTIFIER = PARAMETER_NAME === UNDEFINED ? DEFAULT_VALUE : PARAMETER_NAME ;
`);

const buildSafeArgumentsAccess = template(`
const buildSafeArgumentsAccess = template.statement(`
let $0 = arguments.length > $1 ? arguments[$1] : undefined;
`);

Expand Down Expand Up @@ -238,7 +238,7 @@ export default function convertFunctionParams(
// throws, it must reject asynchronously.
path.node.generator = false;
} else {
path.get("body").unshiftContainer("body", body as t.Statement[]);
path.get("body").unshiftContainer("body", body);
}

return true;
Expand Down
3 changes: 3 additions & 0 deletions packages/babel-preset-env/src/normalize-options.ts
@@ -1,5 +1,8 @@
import semver, { type SemVer } from "semver";
import corejs2Polyfills from "@babel/compat-data/corejs2-built-ins";
// @ts-ignore Fixme: TS can not infer types from ../data/core-js-compat.js
// but we can't import core-js-compat/data.json because JSON imports does
// not work on Node 14
import corejs3Polyfills from "../data/core-js-compat";
import { plugins as pluginsList } from "./plugins-compat-data";
import moduleTransformations from "./module-transformations";
Expand Down
2 changes: 1 addition & 1 deletion packages/babel-standalone/src/index.ts
Expand Up @@ -106,7 +106,7 @@ export function transform(code: string, options: InputOptions) {
}

export function transformFromAst(
ast: Parameters<typeof babelTransformFromAst>[0],
ast: Parameters<typeof babelTransformFromAstSync>[0],
code: string,
options: InputOptions,
) {
Expand Down