Do not remove generator markers #463
Conversation
There was a problem hiding this comment.
Thank you! I guess I don't use generators much, so didn't catch this on my own code 😬.
I think I'd actually prefer to add it to isAccessModifier. Notice that async is there as well, so probably it's fair to say the method name is a misnomer at this point. I added a comment "Determine if this is any token that can go before the name in a method/field.", but probably it should just be renamed. Maybe isClassElementModifier would be a better name?
Mostly I worry about the further-down line while (isAccessModifier(tokens.tokenAtRelativeIndex(-1))) { needing to handle * as well. That's part of the logic to skip to the next method/field in a class, and ideally the * would be included in the range handled by the next iteration. I can't specifically think of a bug that would happen there, but it seems best to handle that right.
Codecov Report
@@ Coverage Diff @@
## master #463 +/- ##
==========================================
+ Coverage 81.56% 81.67% +0.11%
==========================================
Files 52 52
Lines 5429 5429
Branches 1207 1207
==========================================
+ Hits 4428 4434 +6
+ Misses 706 702 -4
+ Partials 295 293 -2
Continue to review full report at Codecov.
|
|
Moved it to isAccessModifier. Didn't rename the function, but can do that if you want that in this PR. |
| {transforms: ["jsx", "imports", "typescript"]}, | ||
| ); | ||
| }); | ||
|
|
There was a problem hiding this comment.
Can you also add a test for?
class C {
async *m() {
yield await 1;
}
}
|
Thanks |
Fixes #462
Previously, the star ended up in rangesToRemove.
Considered simply adding it to isAccessModifier, but I don't really think it is one.