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
Improve traverse typings #14648
Conversation
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/52316/ |
: // todo: refine to t.Node[] | ||
// ? E extends t.Node | ||
// ? E | E[] | ||
// : never |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)
@@ -338,8 +373,9 @@ export function pushContainer( | |||
const path = NodePath.get({ | |||
parentPath: this, | |||
parent: this.node, | |||
container: container, | |||
container: container as unknown as t.Node | t.Node[], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have to cast it as unknown first because NodePath.get
does not have template typings.
| t.CatchClause, | ||
>( | ||
this: NodePath<T>, | ||
): asserts this is NodePath<T & { body: t.BlockStatement }>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could also change it to return undefined in Babel 8 maybe.
let inferer = inferers[node.type]; | ||
let inferer = | ||
// @ts-expect-error inferers do not cover all AST types | ||
inferers[node.type]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
@@ -204,7 +205,7 @@ function gatherNodeParts(node: t.Node, parts: any[]) { | |||
break; | |||
|
|||
case "JSXOpeningElement": | |||
parts.push(node.name); | |||
gatherNodeParts(node.name, parts); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a bug fix: node.name
is an AST node.
2060d95
to
0cc6cb9
Compare
@@ -25,7 +25,7 @@ | |||
}, | |||
"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" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Per #14601 (comment)
const VALID_CALLEES = ["String", "Number", "Math"] as const; | ||
const INVALID_METHODS = ["random"] as const; | ||
|
||
function isValidCalless(val: string): val is typeof VALID_CALLEES[number] { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isValidCalless
Is it a typo?
658bb72
to
5a13fec
Compare
@@ -51,7 +51,7 @@ export default function splitExportDeclaration( | |||
} | |||
|
|||
const updatedDeclaration = standaloneDeclaration | |||
? declaration | |||
? declaration.node |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
@@ -531,7 +531,7 @@ export default declare((api, opts: Options) => { | |||
}, | |||
|
|||
// taken from transform-destructuring/src/index.js#visitor | |||
ForXStatement(path) { | |||
ForXStatement(path: NodePath<t.ForXStatement>) { |
There was a problem hiding this comment.
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? 🤔
There was a problem hiding this comment.
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).
5a13fec
to
ec2eb42
Compare
I have rebased |
8160a4b
to
f3cf592
Compare
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)) { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 😛
) | ||
) { | ||
function getLastStatement(statement: t.Statement): t.Statement { | ||
// @ts-ignore: If statement.body is empty or not a Node, isStatement will return false |
There was a problem hiding this comment.
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".
On the failing Babel 8 test: we should probably run |
b5f457a
to
626983b
Compare
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)), | ||
]), | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible that we'll need this code in the future? If yes, can we keep it commented out?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
a920dc3
to
6b3ae16
Compare
2867f2e
to
3e9f7fa
Compare
* 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>
See #14601 for more info. This PR improves
@babel/traverse
typings and fixes other package typing errors due to more effective traverse typings.