-
Notifications
You must be signed in to change notification settings - Fork 44
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
WIP! Implement optional chaining and import.meta #28
Conversation
This is early draft and can only be finished when the ESTree specs are done. Added a few tests.
Should work the same way as in Babylon parser now.
This implementation should now match the official specs. I also added a bunch of new tests. @aladdin-add Could you follow up this with ESTree, so we can merge this PR soon as the specs are settled.
src/parser.ts
Outdated
@@ -3621,7 +3621,8 @@ export function parseMemberOrUpdateExpression( | |||
inGroup: 0 | 1, | |||
start: number, | |||
line: number, | |||
column: number | |||
column: number, | |||
chained: 0 | 1 = 0 |
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.
@aladdin-add chained: 0 | 1 = 0
is temporary because we don't know the ESTree specs yet. Added to be in line with the Babel parser :) When you figure it out with ESTree this has to either 1) be removed or 2) moved up to be in sync withinGroup
and = 0
removed so every call to parseMemberOrUpdateExpression
has to have a 0
as func arg.
Example: parseMemberOrUpdateExpression(parser, context, /* inGroup */ 0, /* chained */ 0, start, line, column)
If you figure out that the OptionalMemberExpression
shouldn't be there, well just remove it and use the original one and update estree.ts
as well.
@aladdin-add After studying the specs I think Babel's implementation is wrong and not how it will be. My current changes are more in line with what suggested in one of the ESTree discussion from 2017. I will push this changes soon, but here is an example. Note the new You parse this: Outputs: {
"body": [
{
"expression": {
"expr": {
"computed": false
"object": {
"computed": false
"object": {
"name": "a"
"type": "Identifier"
}
"property": {
"name": "b"
"type": "Identifier"
}
"type": "MemberExpression"
}
"optional": true
"property": {
"name": "c"
"type": "Identifier"
}
"type": "MemberExpression"
}
"type": "OptionalChain"
}
"type": "ExpressionStatement"
}
]
"sourceType": "script"
"type": "Program"
} |
This changes are more in line with the ECMA proposal specs (draft) https://tc39.es/proposal-optional-chaining/
…signmentTarget - see Test262 test suite
@aladdin-add and others. I enabled optional chaining and other new Stage 3 proposals available in the REPL so this PR can be tested live there. REPL: https://meriyah.github.io/meriyah/ Here is the benchmark results with this PR implemented. It looks like I got a performance regression :( @axefrog Ideas what can have caused this regression? |
@aladdin-add Fixed the performance regression. There was a regression while parsing larger libraries. Hard to see for people that doesn't know this code :) Here is the new benchmark results. And we are good to go 👍 |
Because Meriyah doesn't do any backtracking or lookahead this implementation is little different than what's done in other parsers. Shouldn't affect performance 👍
@aladdin-add Added |
@aladdin-add Looks like there are no progress with ESTree? Can you just merge this as it is and we adjust the AST later on? This PR kind of blocking me
This is still a WIP, and can't be finished before we know the ESTree specs for it.
For now this implementation is based on this ESTree discussion and how optional chaining is done in V8, SM and Webkit.
Import.meta added here to avoid conflicts later on.