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 module transform packages typings #14608

Merged
merged 5 commits into from May 30, 2022

Conversation

JLHwung
Copy link
Contributor

@JLHwung JLHwung commented May 26, 2022

Q                       A
License MIT

See #14601 for more info.

}
} else if (path.isExportNamedDeclaration()) {
const declar = path.get("declaration");
const declar = path.node.declaration;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

declar has been changed from a NodePath to Node for better type inference.

Copy link
Member

Choose a reason for hiding this comment

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

Probably it's also faster

@babel-bot
Copy link
Collaborator

babel-bot commented May 26, 2022

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

Comment on lines 33 to 35
const amdFactory = (
amdWrapper.get("expression.arguments") as NodePath<t.Expression>[]
)
Copy link
Member

Choose a reason for hiding this comment

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

We should annotate the wrapper parameter as t.ExpressionStatement (so that this type cast is guaranteed to be correct).

Also, it would probably be better to do const amdWrapper: NodePath<t.ExpressionStatement> = path.pushContainer("body", wrapper)[0] instead of type-casting here: it feels safer because we are making less assumptions about the code (we are just helping it to know that pushContainer returns its argument wrapped).

.get("expression.arguments")
const amdFactory = (
amdWrapper.get("expression.arguments") as NodePath<t.Expression>[]
)
.filter(arg => arg.isFunctionExpression())[0]
Copy link
Member

Choose a reason for hiding this comment

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

Nit (not related to this PR, but we can fix it here since it's small): .find() instead of .filter()[0].

.filter(arg => arg.isFunctionExpression())[0]
.get("body");
.get("body") as NodePath;
Copy link
Member

Choose a reason for hiding this comment

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

This should be NodePath<t.BlockStatement>, or NodePath<t.FunctionExpression["body"]>. Or maybe the .find change above already helps it inferring this type.

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 split out expression.argumetns. The find is not sufficient because TS doesn't known the find must succeed. I end up querying the last argument and cast it to a FunctionExpression.

Comment on lines 345 to 350
| t.ImportSpecifier
| t.ImportDefaultSpecifier
| t.ImportNamespaceSpecifier
| t.ExportSpecifier
| t.ExportDefaultSpecifier
| t.ExportNamespaceSpecifier
Copy link
Member

Choose a reason for hiding this comment

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

Does t.ModuleSpecifier work here?

Comment on lines +40 to +42
amdFactoryCallArgs[
amdFactoryCallArgs.length - 1
] as NodePath<t.FunctionExpression>
Copy link
Member

Choose a reason for hiding this comment

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

Is it possible to use .at(-1) here?

Copy link
Member

Choose a reason for hiding this comment

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

No, we still need to support older node versions.

@JLHwung JLHwung merged commit 0f63772 into babel:noImplicitAny May 30, 2022
@JLHwung JLHwung deleted the noImplicitAny-module-transform branch May 30, 2022 17:02
JLHwung added a commit that referenced this pull request Jun 21, 2022
* 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>
@github-actions github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Aug 30, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants