Improve traverse typings by JLHwung · Pull Request #14648 · babel/babel

JLHwung · 2022-06-08T16:59:48Z

Q	A
License	MIT

See #14601 for more info. This PR improves @babel/traverse typings and fixes other package typing errors due to more effective traverse typings.

babel-bot · 2022-06-08T17:02:34Z

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/52316/

JLHwung · 2022-06-08T17:08:26Z

packages/babel-traverse/src/path/modification.ts

+    : // todo: refine to t.Node[]
+      //  ? E extends t.Node
+      //    ? E | E[]
+      //    : never


unshiftContainer now enforces stricter typings. I tried refining further but TS fails to infer statements: Statement[] can be passed to NodePath<t.BlockStatement>.unshiftContainer("body", statements)

JLHwung · 2022-06-08T17:09:19Z

packages/babel-traverse/src/path/modification.ts

    parentPath: this,
    parent: this.node,
-    container: container,
+    container: container as unknown as t.Node | t.Node[],


I have to cast it as unknown first because NodePath.get does not have template typings.

JLHwung · 2022-06-08T17:12:34Z

packages/babel-traverse/src/path/index.ts

+      | t.CatchClause,
+  >(
+    this: NodePath<T>,
+  ): asserts this is NodePath<T & { body: t.BlockStatement }>;


ensureBlock now asserts the shape { body: t.BlockStatement }, however TS requires explicit type annotations for assertions so I added some type annotations in other plugins.

We could also change it to return undefined in Babel 8 maybe.

JLHwung · 2022-06-08T17:14:33Z

packages/babel-traverse/src/path/inference/index.ts

-    let inferer = inferers[node.type];
+    let inferer =
+      // @ts-expect-error inferers do not cover all AST types
+      inferers[node.type];


The inferers could be refined to Record<node["type"], Inferer> but then we have to convert the namespace exports in inferers to a plain object export.

JLHwung · 2022-06-08T17:35:08Z

packages/babel-traverse/src/scope/index.ts


    case "JSXOpeningElement":
-      parts.push(node.name);
+      gatherNodeParts(node.name, parts);


This is a bug fix: node.name is an AST node.

JLHwung · 2022-06-13T18:14:42Z

packages/babel-types/package.json

  "dependencies": {
    "@babel/helper-validator-identifier": "workspace:^",
-    "to-fast-properties": "condition:BABEL_8_BREAKING ? ^4.0.0 : ^2.0.0"
+    "to-fast-properties": "condition:BABEL_8_BREAKING ? ^3.0.0 : ^2.0.0"


Per #14601 (comment)

liuxingbaoyu · 2022-06-14T11:20:07Z

packages/babel-traverse/src/path/evaluation.ts

+const VALID_CALLEES = ["String", "Number", "Math"] as const;
+const INVALID_METHODS = ["random"] as const;
+
+function isValidCalless(val: string): val is typeof VALID_CALLEES[number] {


isValidCalless
Is it a typo?

nicolo-ribaudo · 2022-06-15T10:23:32Z

packages/babel-helper-split-export-declaration/src/index.ts


    const updatedDeclaration = standaloneDeclaration
-      ? declaration
+      ? declaration.node


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

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.

nicolo-ribaudo · 2022-06-15T10:25:25Z

packages/babel-plugin-proposal-object-rest-spread/src/index.ts


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


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

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).

JLHwung · 2022-06-20T15:29:37Z

I have rebased noImplicitAny and this branch. Hopefully CI will be green again.

JLHwung · 2022-06-20T15:38:31Z

packages/babel-helper-transform-fixture-test-runner/src/index.ts

-        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)) {


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

It works well enough for what we need 😛

JLHwung · 2022-06-20T15:42:05Z

packages/babel-generator/src/generators/statements.ts

-    )
-  ) {
+function getLastStatement(statement: t.Statement): t.Statement {
+  // @ts-ignore: If statement.body is empty or not a Node, isStatement will return false


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

JLHwung · 2022-06-20T15:58:32Z

On the failing Babel 8 test: we should probably run make tscheck after stripping Babel_8_BREAKING flags.

nicolo-ribaudo · 2022-06-20T22:23:33Z

packages/babel-traverse/src/scope/lib/renamer.ts

-    if (!path.isFunctionDeclaration() && !path.isClassDeclaration()) return;
-    if (this.binding.kind !== "hoisted") return;
-
-    path.node.id = identifier(this.oldName);
-    path.node._blockHoist = 3;
-
-    path.replaceWith(
-      variableDeclaration("let", [
-        variableDeclarator(identifier(this.newName), toExpression(path.node)),
-      ]),
-    );


Is it possible that we'll need this code in the future? If yes, can we keep it commented out?

I make it an identity function so TS will not complain unused params.

The AST typings are generated from defineType calls, so defineType should accept a string as type.

* export removeProperties options * traverse * fix other packages typings * fix gatherNodeParts * downgrade to-fast-properties to v3 * fix typo * babel-core/normalize-file * add ignore comment property used by flow plugin * refine getLastStatement typings * fix babel-standalone rebase typing error * fix assert.rejects polyfill * simplify _param typings * supress Babel 7 AST errors * loosen defineType typings The AST typings are generated from defineType calls, so defineType should accept a string as type. * restore legacy code * Suppress data/core-js-compat importing errors

* enable noImplicitAny * codemod (#14602) * Improve preset/plugin-typescript typings (#14603) * preset-typescript * transform-typescript * Update packages/babel-plugin-transform-typescript/src/enum.ts Co-authored-by: Nicolò Ribaudo <nicolo.ribaudo@gmail.com> * prettier Co-authored-by: Nicolò Ribaudo <nicolo.ribaudo@gmail.com> * Improve transform-runtime typings (#14605) * enable noImplicitAny * codemod (#14602) * Improve preset/plugin-typescript typings (#14603) * preset-typescript * transform-typescript * Update packages/babel-plugin-transform-typescript/src/enum.ts Co-authored-by: Nicolò Ribaudo <nicolo.ribaudo@gmail.com> * prettier Co-authored-by: Nicolò Ribaudo <nicolo.ribaudo@gmail.com> * improve transform-runtime typings Co-authored-by: Nicolò Ribaudo <nicolo.ribaudo@gmail.com> * jsx (#14607) * Improve module transform packages typings (#14608) * improve babel-template typings (#14621) * Improve @babel/helper-* typings (#14620) * improve helper-function-name typings * helper hoist variables * helper-member-expression-to-functions * helper-simple-access * remap-async-to-generator * helper-annotate-as-pure * helper-builder-binary-assignment-operator-visitor * helper-check-duplicate-nodes * early return when export declaration is export all * split-export-declaration * helper-define-map * define-map * Update packages/babel-helper-builder-binary-assignment-operator-visitor/src/index.ts Co-authored-by: Nicolò Ribaudo <nicolo.ribaudo@gmail.com> * review comments Co-authored-by: Nicolò Ribaudo <nicolo.ribaudo@gmail.com> * improve helper-module-imports typings (#14623) * refactor: simplify ImportInjector._applyDefaults * helper-module-imports * map globals to the one used in Babel 8 * Improve fixture-test-runner typings (#14625) * plugin-test-runner * fixture-test-runner * Update packages/babel-helper-fixtures/src/index.ts Co-authored-by: Nicolò Ribaudo <nicolo.ribaudo@gmail.com> Co-authored-by: Nicolò Ribaudo <nicolo.ribaudo@gmail.com> * Improve preset-env typings (#14606) * refactor: move shipped-proposals to ts * preset-env * address review comments * Improve pipeline-operator typings (#14629) * refactor: simplify buildOptimizedSequenceExpression * proposal-pipeline-operator * address review comments * improve standalone typings (#14630) * babel-standalone * address review comment * Improve plugins typings (Part 1) (#14633) * change AssumptionFunction return type * helper-create-regexp-features-plugin * create-class-features-plugin * transform-for-of * improve unicode-escapes typings * transform-object-super * transform-react-constant-elements * proto-to-assign * function-name * flow-comments TS cannot infer Flow visitor type because we have both Flow type and Flow virtual type. * Improve code-frame/hightlight typings (#14643) * code-frame * map js-tokens to Babel 8 * highlight * Improve `@babel/types` typings (#14645) * bump to-fast-properties to v4 * types * mark node comments as mutable * Improve babel-core typings (#14622) Co-authored-by: Nicolò Ribaudo <nicolo.ribaudo@gmail.com> * improve transform-classes typings (#14624) * Improve `@babel/generator` typings (#14644) * generator * refactor: merge {start,end}Terminatorless to printTerminatorless * inline buildYieldAwait * inline ExportDeclaration * also export Pos * let getPossibleRaw return string | void * Apply suggestions from code review Co-authored-by: Nicolò Ribaudo <nicolo.ribaudo@gmail.com> Co-authored-by: liuxingbaoyu <30521560+liuxingbaoyu@users.noreply.github.com> * address review comments * do not export internal printer method * simplify needsWhitespace Co-authored-by: Nicolò Ribaudo <nicolo.ribaudo@gmail.com> Co-authored-by: liuxingbaoyu <30521560+liuxingbaoyu@users.noreply.github.com> * Improve plugins typings (Part 2) (#14639) * exponentiation-operator * duplicate-keys * computed-properties * replace-supers * block-scoping * block-scoped-functions * syntax-typescript * record-and-tuple * private-property-in-object * partial-application * json-strings * function-sent * function-bind * class-static-block * async-generator-functions * Update packages/babel-plugin-transform-block-scoping/src/index.ts * address review comments * helper-replace-supers * Improve plugins typings (Part 3) (#14642) * external-helpers * bugfix * transform-parameters * object-rest-spread * destructuring-private * transform-destructuring * proposal-decorators * optional-chaining * helper-wrap-function * explode-assignable-expression * helper-compilation-targets * helper-plugin-utils * helpers * helper-validator-option * fix: allow "+" and "-" in MappedType .readonly/.optional * fixture-test-runner * remove charcodes from dependencies the charcodes transform will inline charCodes.* so we can remove it from dependencies * Improve traverse typings (#14648) * export removeProperties options * traverse * fix other packages typings * fix gatherNodeParts * downgrade to-fast-properties to v3 * fix typo * babel-core/normalize-file * add ignore comment property used by flow plugin * refine getLastStatement typings * fix babel-standalone rebase typing error * fix assert.rejects polyfill * simplify _param typings * supress Babel 7 AST errors * loosen defineType typings The AST typings are generated from defineType calls, so defineType should accept a string as type. * restore legacy code * Suppress data/core-js-compat importing errors Co-authored-by: Nicolò Ribaudo <nicolo.ribaudo@gmail.com> Co-authored-by: liuxingbaoyu <30521560+liuxingbaoyu@users.noreply.github.com>

JLHwung commented Jun 8, 2022

View reviewed changes

JLHwung force-pushed the noImplicitAny-traverse branch from 2060d95 to 0cc6cb9 Compare June 13, 2022 18:14

JLHwung commented Jun 13, 2022

View reviewed changes

liuxingbaoyu reviewed Jun 14, 2022

View reviewed changes

JLHwung force-pushed the noImplicitAny-traverse branch from 658bb72 to 5a13fec Compare June 14, 2022 13:30

liuxingbaoyu approved these changes Jun 14, 2022

View reviewed changes

nicolo-ribaudo reviewed Jun 19, 2022

View reviewed changes

JLHwung force-pushed the noImplicitAny branch from c22823e to bceef21 Compare June 20, 2022 15:15

JLHwung force-pushed the noImplicitAny-traverse branch from 5a13fec to ec2eb42 Compare June 20, 2022 15:28

JLHwung force-pushed the noImplicitAny-traverse branch 2 times, most recently from 8160a4b to f3cf592 Compare June 20, 2022 15:36

JLHwung commented Jun 20, 2022

View reviewed changes

JLHwung requested a review from liuxingbaoyu June 20, 2022 15:42

liuxingbaoyu approved these changes Jun 20, 2022

View reviewed changes

JLHwung force-pushed the noImplicitAny-traverse branch from b5f457a to 626983b Compare June 20, 2022 18:04

liuxingbaoyu requested a review from nicolo-ribaudo June 20, 2022 22:15

nicolo-ribaudo reviewed Jun 20, 2022

View reviewed changes

nicolo-ribaudo approved these changes Jun 20, 2022

View reviewed changes

JLHwung force-pushed the noImplicitAny branch from bceef21 to 18291cb Compare June 21, 2022 02:35

JLHwung added 7 commits June 20, 2022 22:35

export removeProperties options

6d42e2e

traverse

15279e7

fix other packages typings

b4cad86

fix gatherNodeParts

3f5cebe

downgrade to-fast-properties to v3

4258928

fix typo

68137e3

babel-core/normalize-file

382ba1e

JLHwung added 8 commits June 20, 2022 22:35

add ignore comment property used by flow plugin

f9c0457

refine getLastStatement typings

3551433

fix babel-standalone rebase typing error

a209629

fix assert.rejects polyfill

d6ec941

simplify _param typings

038eb2c

supress Babel 7 AST errors

a504cec

loosen defineType typings

fc97108

The AST typings are generated from defineType calls, so defineType should accept a string as type.

restore legacy code

6b3ae16

JLHwung force-pushed the noImplicitAny-traverse branch from a920dc3 to 6b3ae16 Compare June 21, 2022 02:38

Suppress data/core-js-compat importing errors

3e9f7fa

JLHwung force-pushed the noImplicitAny-traverse branch from 2867f2e to 3e9f7fa Compare June 21, 2022 02:50

JLHwung merged commit 03880bf into babel:noImplicitAny Jun 21, 2022

JLHwung deleted the noImplicitAny-traverse branch June 21, 2022 15:43

JLHwung mentioned this pull request Aug 10, 2022

[Bug]: generateUidIdentifierBasedOnNode with JSX returning _objectObject #14418

Closed

1 task

github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Sep 21, 2022

github-actions bot locked as resolved and limited conversation to collaborators Sep 21, 2022

Uh oh!

Conversation

JLHwung commented Jun 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

babel-bot commented Jun 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JLHwung commented Jun 20, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JLHwung commented Jun 20, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

JLHwung commented Jun 8, 2022 •

edited

Loading

babel-bot commented Jun 8, 2022 •

edited

Loading