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

Convert @babel/parser to TypeScript #14783

Merged
merged 9 commits into from Jul 23, 2022
Merged

Conversation

JLHwung
Copy link
Contributor

@JLHwung JLHwung commented Jul 22, 2022

Q                       A
Fixed Issues? Closes #13370
Patch: Bug Fix?
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass? Yes
Documentation PR Link
Any Dependency Changes?
License MIT

In this PR we converts babel-parser to TypeScript, which marks an end of the transition from Flow (but not the end of improving the codebase typings). I tried to make this PR as small as possible. For those typing errors which can't be fixed at my first glance, I suppress them with @ts-expect-error. Hopefully we will remove most of them when we further improve the parser typings.

Significant changes:

  • An Undone type marker is introduced for those intermediate AST nodes created by startNode
type Undone<T extends NodeType> = Omit<T, "type">;

Accessing the type property is a typing error so we are sure that we didn't do any type differentiation on unfinished AST nodes, otherwise the other properties are typed as the AST definitions in "babel-parser".

  • For parser plugins, if we are calling this.someParserMethod which is not overridden in the plugin, we use super.someParserMethod instead.

Further Improvements that be landed in separate PRs:

  • Cleanup the legacy Flow infrastructure
  • Migrate @babel/parser AST types to @babel/types AST types
  • Cleanup unnecessary @ts-expect-error comments

This PR revives #13370. I should also thank @zxbodya, who maintains the flowts which helps the transition process.

@JLHwung JLHwung added Flow -> TS Tracking repository migration from Flow to TS PR: Internal 🏠 A type of pull request used for our changelog categories labels Jul 22, 2022
@babel-bot
Copy link
Collaborator

babel-bot commented Jul 22, 2022

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

@liuxingbaoyu
Copy link
Member

Amazing PR and at the same time CI is completely green, I like it very much!

@liuxingbaoyu liuxingbaoyu self-requested a review July 22, 2022 17:26
?{ code?: ParseErrorCode, reasonCode?: string } | boolean,
function toParseErrorCredentials(
b: string,
a?:
Copy link
Member

Choose a reason for hiding this comment

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

export was added before the comment, which seems to be a problem with the automatic conversion.

update: I tried to pick the code block and it failed, see the code before and after here.

Copy link
Member

@nicolo-ribaudo nicolo-ribaudo left a comment

Choose a reason for hiding this comment

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

Thanks, awesome! I pushed a commit undoing some minor format changes.

@JLHwung JLHwung merged commit a483aa2 into babel:main Jul 23, 2022
@JLHwung JLHwung deleted the ts-babel-parser branch July 23, 2022 03:14
@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 Oct 22, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Flow -> TS Tracking repository migration from Flow to TS outdated A closed issue/PR that is archived due to age. Recommended to make a new issue PR: Internal 🏠 A type of pull request used for our changelog categories
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants