Port babel-parser changes from 2020-04-12 to 2020-07-22 #556
Conversation
There was a problem hiding this comment.
Great, thanks! The one that you ported looks good. For the three where you had questions, I think the private fields ones should be ported as well so that the parser stays up-to-date with future JS.
66b86 4 months ago added basic support for module attributes and tests updated (#10962)
β Skipped as this is the implementation of a stage 1 proposal - the proposal is now stage 3 though, but is perhaps a separate effort? https://github.com/tc39/proposal-import-assertions
Makes sense to skip given that the syntax has changed since that implementation. Whenever babel-parser implements the new assert syntax, the port of that commit would be a good opportunity to implement the syntax in Sucrase.
74590 4 months ago Add private-property-in-object support (#11372)
β Skipped this one as I didn't quite understand the aim of the existing private fields support - they seem to be removed completely?
The current state of private class fields isn't great right now. Theoretically, they're "passed through" so that they're supported as long as the runtime supports them, but the class fields transform removes them, which basically causes any uses to break. I think ideally this issue would be either fixed or documented.
Longer-term (or maybe actually pretty soon), I think Sucrase should have a semver-major release to drop the class fields transform since it's fairly well-supported by node and browsers at this point. At that point, supporting private class fields is just a matter of passing it through to the compiled JS.
So I think we should actually port this change, with the goal of just passing through the tokens without crashing. From the parser perspective, it should properly emit the token without breaking the parser state.
bda75 4 months ago Handle private access chained on an optional chain (#11248)
β Skipped for the same reason as 74590 above
Like above, I think we should port this change so that it works in the parser, but not worry about the runtime implementation. In the future, when Sucrase removes the class fields and optional chaining transforms, it should be able to parse and pass through the syntax so that the underlying JS runtime can evaluate it.
|
@alangpierce Thanks! Makes sense to forward the private field tokens, will have a look. Would that just be a |
Yep! A reasonable alternative is a separate
I'd call it out of scope, yes. I looked at the Babel source code to confirm what it does, and it parses a |
|
@alangpierce Updated to include support for |
src/parser/tokenizer/index.ts
Outdated
| @@ -218,6 +218,12 @@ export function lookaheadTypeAndKeyword(): TypeAndKeyword { | |||
| return new TypeAndKeyword(type, contextualKeyword); | |||
| } | |||
|
|
|||
| // Returns true if the next character is a valid start for an identifier | |||
| export function nextIsIdentifier(): boolean { | |||
There was a problem hiding this comment.
Opted for this instead of accessing input in the expression traverser, which is essentially what babel does. Should be simple to switch around if you prefer something different.
There was a problem hiding this comment.
Never mind, ended up syncing more from babel-parser, which moves over to accessing input in the traverser
Codecov Report
@@ Coverage Diff @@
## master #556 +/- ##
==========================================
- Coverage 81.64% 81.60% -0.04%
==========================================
Files 55 55
Lines 5503 5758 +255
Branches 1299 1303 +4
==========================================
+ Hits 4493 4699 +206
- Misses 721 768 +47
- Partials 289 291 +2
Continue to review full report at Codecov.
|
Rugvip
changed the title
Rugvip
changed the title
|
@alangpierce Added a couple more commits, should hopefully now have all that we need in the parser to support TS4. |
There was a problem hiding this comment.
π really great work, thank you! I have a few nitpicks, but none particularly important, so I'll merge as-is, then do a release.
02c8f 3 months ago fix: add optional: false to chained optional call expression (#11814)
β Sets node.optional, assuming this can be skipped since optional chaining is handled differently in Sucrase?
Yep, that sounds right to me! It was just an issue of AST format (optional missing vs explicitly false).
| @@ -408,6 +408,7 @@ export function parseExprAtom(): boolean { | |||
| case tt.regexp: | |||
| case tt.num: | |||
| case tt.bigint: | |||
| case tt.decimal: | |||
There was a problem hiding this comment.
FYI this doesn't need to be kept in sync since it's just a benchmark. But either way is fine.
| ++state.pos; | ||
| isBigInt = true; | ||
| } else if (nextChar === charCodes.lowercaseM) { | ||
| unexpected("Invalid decimal", start); |
There was a problem hiding this comment.
I think I'd prefer to leave this validation off for the sake of simplicity.
| return pos + skip![0].length; | ||
| } | ||
|
|
||
| export function lookaheadCharCode(): number { |
There was a problem hiding this comment.
Thanks, this makes sense to me! Both usages do feel a little ugly, but I guess that's to be expected for some parser cases.
One thought: it would be nice to avoid the skipWhiteSpace regex, at least in the long run. A while back, I did a lot of work to make Sucrase more compatible with AssemblyScript, and it didn't quite get there, but generally high-powered JS features like regexes aren't going to work nicely in WebAssembly. Rather than a regex, I'd prefer just a plain function that walks the string indices until it finds non-whitespace. I think that would execute faster as well, though I'm not 100% sure. (Not that this is a common code path anyway.) I'll merge this as-is, since it's not really a big deal, but might be nice to rework it a little in the future.
| @@ -367,6 +367,7 @@ function tsParseMappedType(): void { | |||
| function tsParseTupleType(): void { | |||
| expect(tt.bracketL); | |||
| while (!eat(tt.bracketR) && !state.error) { | |||
| // Do not validate presence of either none or only labeled elements | |||
There was a problem hiding this comment.
This comment doesn't seem super-necessary; across the parser code, all validation is skipped.
There was a problem hiding this comment.
Put it here because it replaces a large chunk of code in babel, but agree that it's pretty useless outside the context of porting babel changes
a466f 5 months ago fix: report missing plugins on type exports (#11417)
π« Looks like this is already covered by the existing isTS and isFlow cases
fba64 5 months ago fix: disallow expression after binding identifier
of(#11355)π« Sucrase doesn't use token contexts.
40c51 5 months ago Set exprAllowed to false for star token (#11449)
π« Sucrase doesn't use token contexts.
2e4f18 5 months ago Add some parser missing plugins errors (#11478)
π« Validation related to babel plugins only.
fa98a 5 months ago docs: update AST spec (#11492)
π« Docs update
9c284 5 months ago (tag: v7.9.6) v7.9.6
π« Release only
90a91 5 months ago Update Flow to 0.123.0 (#11500)
π« Flow version bump with one change to the tokenizer that isn't present in sucrase
31b36 5 months ago Use ?. where it represents the intended semantics (#11512)
π« Refactor to make use of ?. in the all of babel, separate effort in sucrase if needed
2f31e 4 months ago fix: allow bigInt in method name and TSLiteralType (#11547)
β Skipped refactor of isLiteralPropertyName in utils, but ported bigint type literal support + added test
62e68 4 months ago Fix comments for smartPipeline topic-forbidding contexts (#11597)
π« Comment update related to contexts
5dd7f 4 months ago Enable
import.metaby default in@babel/parser(#11406)π« Import meta is already always parsed
66b86 4 months ago added basic support for module attributes and tests updated (#10962)
π« Skipped as this is the implementation of a stage 1 proposal - the proposal is now stage 3 though, but is perhaps a separate effort? https://github.com/tc39/proposal-import-assertions
74590 4 months ago Add private-property-in-object support (#11372)
β Ported
bda75 4 months ago Handle private access chained on an optional chain (#11248)
β Ported
5da24 4 months ago (tag: v7.10.0) v7.10.0
π« Release only
242d9 4 months ago Use
repository.directoryfield inpackage.jsonfiles (#11625)π« Just babel package.json stuff
88f57 4 months ago (tag: v7.10.1) v7.10.1
π« Release only
b5c4a 4 months ago refactor: split locationParser into ParserErrors and error message (#11653)
π« Skipped, error message related refactoring
b0350 4 months ago (tag: v7.10.2) v7.10.2
π« Release only
71d352 4 months ago Properly parse
export default fromwhenexportDefaultFromis not enabled (#11676)β Ported the change, but was likely working already.
41085 4 months ago Update prettier to v2 (#11579)
π« Skipped, formatting change
b27ab 3 months ago fix: add optional: false to MemberExpression (#11709)
π« estree not supported.
e15a5 3 months ago Fix innercomments (#11697)
π« Sucrase skips comments
eea15 3 months ago Migrate from "master" branch to "main" (#11715)
π« Branch rename
2787e 3 months ago (tag: v7.10.3) v7.10.3
π« Release only
30835 3 months ago fix: implement early errors for record and tuple (#11652)
π« Error handling only
beca7 3 months ago Add better parser error when using jsx (#11722)
π« Error handling only
75c23 3 months ago Add @babel/eslint-plugin-development-internal (#11376)
π« Linter changes only
7fd40d 3 months ago (tag: v7.10.4) v7.10.4
π« Release only
b1b21 3 months ago docs: add AST spec on optional chain [skip ci] (#11729)
π« Documentation change only
d6762 3 months ago fix: throw expect jsx plugin error when an idStart or > is seen (#11774)
π« Error handling only
02c8f 3 months ago fix: add optional: false to chained optional call expression (#11814)
β Sets node.optional, assuming this can be skipped since optional chaining is handled differently in Sucrase?
f7964 2 months ago (tag: v7.10.5) v7.10.5
π« Release only
8f191 10 weeks ago chore: fix typo in codebase (#11846)
π« These typos aren't present in Sucrase
f4eeff 10 weeks ago fix: correctly set innerEndPos in CoverParenthesizedExpressionAndArrowParameterList (#11847)
π« Fix for cone that's not present in Sucrase
3680f 9 weeks ago fix: allow 09.1_1 and 09e1_1 in sloppy mode (#11854)
π« Sucrase doesn't have separate handling of octals so this was not needed
2bf38 9 weeks ago fix: disallow \8, \9 in strict mode string (#11852)
π« Error handling only
059e91 8 weeks ago Add decimal parsing support (#11640)
β Ported
d7347f 8 weeks ago eslint-parser: ES2020 features (#11815)
π« estree not supported.
5b4b3 3 months ago TypeScript 4.0: Allow spread in the middle of tuples (#11753)
β Added tests, but the change did not need to be ported
8a1d7e4 3 months ago Allow unknown/any in TS catch clause param (#11755)
β Ported + added TypeScript tests, only change needed to support catch clause params
9e666 3 months ago Follow-up on initial TS4 catch param support (#11767)
π« Skipped, this detail is not included in the TypeScript parser in sucrase
eba4c 2 months ago TypeScript 4.0: Support labeled tuple elements (#11754)
β Ported, excluding validation logic
0e985 9 weeks ago feat: enable numericSeparator parsing support (#11863)
π« Skipped, already supported
b651a 9 weeks ago Enable logical assignment by default in @babel/parser (#11860) (#11869)
π« Skipped, already supported