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

feat: add new parser babel-ts to parse TypeScript syntaxes via Babel #6400

Merged
merged 21 commits into from
Jan 29, 2020
Merged

feat: add new parser babel-ts to parse TypeScript syntaxes via Babel #6400

merged 21 commits into from
Jan 29, 2020

Conversation

JounQin
Copy link
Member

@JounQin JounQin commented Aug 18, 2019

  • I’ve added tests to confirm my change works.
  • (If changing the API or CLI) I’ve documented the changes I’ve made (in the docs/ directory)
  • (If the change is user-facing) I’ve added my changes to the CHANGELOG.unreleased.md file following the template.
  • I’ve read the contributing guidelines.

Try the playground for this PR

Motivation: Babel itself is able to parse TypeScript, but we haven't enabled it, which makes something like babel-preset-proposal-typescript can not integrate with Prettier perfectly, but it should be easy to support it.

Copy link
Member

@j-f1 j-f1 left a comment

Choose a reason for hiding this comment

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

Can you add some more thorough tests (i.e. by adding the babel-ts parser to a few existing TypeScript test cases) to make sure that the printer can handle the Babel-TypeScript AST just as well as the ESTree-Typescript AST?

docs/options.md Outdated Show resolved Hide resolved
docs/options.md Outdated Show resolved Hide resolved
@JounQin
Copy link
Member Author

JounQin commented Aug 18, 2019

Can you add some more thorough tests (i.e. by adding the babel-ts parser to a few existing TypeScript test cases) to make sure that the printer can handle the Babel-TypeScript AST just as well as the ESTree-Typescript AST?

Done.

Copy link
Member

@alexander-akait alexander-akait left a comment

Choose a reason for hiding this comment

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

Not sure we need 2 parsers for typescript

@JounQin
Copy link
Member Author

JounQin commented Aug 19, 2019

Not sure we need 2 parsers for typescript

@evilebottnawi With babel-ts we will be able to integrate something like babel-preset-proposal-typescript as like in my PR description.

And we do have two parsers for flow. :)

@alexander-akait
Copy link
Member

@JounQin 👍

@JounQin

This comment has been minimized.

@alexander-akait

This comment has been minimized.

@JounQin

This comment has been minimized.

@JounQin

This comment has been minimized.

@alexander-akait

This comment has been minimized.

@JounQin

This comment has been minimized.

@alexander-akait

This comment has been minimized.

@JounQin

This comment has been minimized.

@alexander-akait alexander-akait added this to the 1.19 milestone Sep 12, 2019
@alexander-akait

This comment has been minimized.

@alexander-akait

This comment has been minimized.

src/language-js/parser-babylon.js Outdated Show resolved Hide resolved
@lipis

This comment has been minimized.

@JounQin

This comment has been minimized.

@alexander-akait

This comment has been minimized.

@alexander-akait alexander-akait mentioned this pull request Sep 30, 2019
@alexander-akait

This comment has been minimized.

@JounQin

This comment has been minimized.

docs/options.md Outdated Show resolved Hide resolved
@JounQin
Copy link
Member Author

JounQin commented Oct 2, 2019

@duailibe Can you approve this PR? So that we merge it...

@duailibe
Copy link
Member

duailibe commented Oct 2, 2019

@JounQin I'm aware. I still disagree with the combinations, there are still too many

I'm trying to fix it myself, and I'll update the PR today

@thorn0
Copy link
Member

thorn0 commented Jan 23, 2020

Tested --parser babel-ts on the following repos: Microsoft/TypeScript, ReactiveX/rxjs, and angular/angular. The results are identical to formatting with --parser typescript. Does anyone want to help with this and test it on some big TypeScript codebases? Remember that it can be installed directly from GitHub with yarn add JounQin/prettier#feat/babel_ts.

@alexander-akait
Copy link
Member

@thorn0 I think we if we find some problems we can send a PR with fix, it is not easy to find all edge cases

Copy link
Member

@alexander-akait alexander-akait left a comment

Choose a reason for hiding this comment

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

I think we can merge ⭐

@thorn0 thorn0 merged commit c1937cb into prettier:next Jan 29, 2020
@JounQin JounQin deleted the feat/babel_ts branch January 29, 2020 12:13
@lock lock bot added the locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. label May 5, 2020
@lock lock bot locked as resolved and limited conversation to collaborators May 5, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet