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
[ts]Fix syntax error for modifier name class methods with type parameters #12356
Conversation
sosukesuzuki
commented
Nov 13, 2020
•
edited by gitpod-io
bot
edited by gitpod-io
bot
Q | A |
---|---|
Fixed Issues? | Fixes #12353 |
Patch: Bug Fix? | Y |
Major: Breaking Change? | N |
Minor: New Feature? | N |
Tests Added + Pass? | Yes |
Documentation PR Link | |
Any Dependency Changes? | |
License | MIT |
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/32858/ |
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 92f8321:
|
@@ -166,7 +166,8 @@ export default (superClass: Class<Parser>): Class<Parser> => | |||
!this.match(tt.colon) && | |||
!this.match(tt.eq) && | |||
!this.match(tt.question) && | |||
!this.match(tt.bang) | |||
!this.match(tt.bang) && | |||
!this.match(tt.relational) |
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 think we could align to TS implementation here:
An allowlist instead of denylist is better to reason about and avoid any surprise cases that we did not rule out.
class C { | ||
declare<T>() {} | ||
readonly<T>() {} | ||
abstract<T>() {} |
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.
Can you also add test cases for private
, public
and protected
?
0574a07
to
f4fba0b
Compare
@@ -0,0 +1,4 @@ | |||
class Foo { | |||
constructor(private, public, static) { |
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 should be an error, because private
, public
and static
are reserved words in strict mode.
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.
babel-parser
doesn't check reserved word for TypeScript?
babel/packages/babel-parser/src/plugins/typescript/index.js
Lines 1962 to 1972 in b564368
checkReservedWord( | |
word: string, // eslint-disable-line no-unused-vars | |
startLoc: number, // eslint-disable-line no-unused-vars | |
checkKeywords: boolean, // eslint-disable-line no-unused-vars | |
// eslint-disable-next-line no-unused-vars | |
isBinding: boolean, | |
): void { | |
// Don't bother checking for TypeScript code. | |
// Strict mode words may be allowed as in `declare namespace N { const static: number; }`. | |
// And we have a type checker anyway, so don't bother having the parser do it. | |
} |
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.
Oh right, I didn't remember about that.
Thanks! |
(I reworded the commit title a bit otherwise it was too long when adding |