-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
fix(parser): properly handle optional markers in generator class methods #17312
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
Conversation
magic-akari
commented
May 15, 2025
Q | A |
---|---|
Fixed Issues? | Fixes #17310 |
Patch: Bug Fix? | |
Major: Breaking Change? | |
Minor: New Feature? | |
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/59338 |
"SyntaxError: Class methods cannot have the 'readonly' modifier. (6:2)", | ||
"SyntaxError: Class methods cannot have the 'declare' modifier. (7:2)" |
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.
TS1024: 'readonly' modifier can only appear on a property declaration or index signature.
TS1031: 'declare' modifier cannot appear on class elements of this kind.
@@ -0,0 +1,9 @@ | |||
class A { | |||
*[a?.b]?() { }; |
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.
Should we report a recoverable error in cases like this, similarly to how report it for readonly
and declare
?
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.
Actually, this is valid TypeScript syntax.
Do you mean readonly *[a?.b]?() { };
?
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 I meant with just ?
, I didn't realize it's actually valid syntax.
readonly *[e?.e]?() { } | ||
declare *[f?.f]?() { } | ||
protected *[g?.g]?() { } | ||
} |
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.
Thank you. Can you split this test into two? one for the valid cases and the other for invalid ones.
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.
Done