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

Reinterpret << when parsing TS type arguments #14145

Merged
merged 6 commits into from Jan 24, 2022
Merged

Conversation

JLHwung
Copy link
Contributor

@JLHwung JLHwung commented Jan 12, 2022

Q                       A
Fixed Issues? Fixes #11969
Patch: Bug Fix? Y
Tests Added + Pass? Yes
Documentation PR Link
Any Dependency Changes?
License MIT

I didn't port the fixe to Flow because Flow does not support f<<T>(v: T) => void>(), but it does support f< <T>(v: T) => void>(). I will open an issue to Flow later. Before Flow supports it let's focus on the TS.

In this PR we split tt.bitShift into tt.bitShiftL (<<) and tt.bitShiftR (>>). Then we reinterpret << as < when parsing type arguments from ES productions. We don't need to handle << when we are parsing from TS productions because getTokenFromType ensures that < is always tokenized into tt.lt when state.inType is true.

@JLHwung JLHwung added PR: Bug Fix 🐛 A type of pull request used for our changelog categories area: typescript labels Jan 12, 2022
@babel-bot
Copy link
Collaborator

babel-bot commented Jan 12, 2022

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

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.

It looks like it already works, but can you also add a test for <<<? f<<<T>(x) is f << (<T> x) (with JSX disabled)

And also a test for when << is a real bit shift: f<<T>((x)=>string)>(x);

@@ -2818,8 +2829,9 @@ export default (superClass: Class<Parser>): Class<Parser> =>

parseClassSuper(node: N.Class): void {
super.parseClassSuper(node);
if (node.superClass && this.match(tt.lt)) {
node.superTypeParameters = this.tsParseTypeArguments();
// handle `extends f()<<T>
Copy link
Member

Choose a reason for hiding this comment

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

Q: Why f()<<T> and not f<<T>?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. It can be simplified.

@JLHwung JLHwung merged commit 5861002 into babel:main Jan 24, 2022
@JLHwung JLHwung deleted the fix-11969 branch January 24, 2022 04:47
fisker added a commit to prettier/prettier that referenced this pull request Feb 7, 2022
@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 Apr 26, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: typescript outdated A closed issue/PR that is archived due to age. Recommended to make a new issue PR: Bug Fix 🐛 A type of pull request used for our changelog categories
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[@babel/preset-typescript] Parse error when an explicit type parameter is a generic arrow function
4 participants