-
-
Notifications
You must be signed in to change notification settings - Fork 258
Conversation
src/util/identifier.js
Outdated
@@ -31,7 +31,7 @@ export const reservedWords = { | |||
// And the keywords | |||
|
|||
export const isKeyword = makePredicate( | |||
"break case catch continue debugger default do else finally for function if return switch throw try var while with null true false instanceof typeof void delete new in this let const class extends export import yield super", | |||
"break case catch continue debugger default do else finally for function if return switch throw try var while with null true false instanceof typeof void delete new in this let const class extends export import yield super match", |
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.
We'll need to somehow fork this with the hasPlugin check as well right?
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.
You mean makePredicate([...] + this.hasPlugin('match') ? 'match' : '')
? Yes because match
shoudln't be a keyword without the plugin.
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.
@hzoo @xtuc I wonder if we should make match
a keyword at all (or at least part of this array)?
If we do it then we need to tweak for all situations where match is currently being used as a valid identifier. Of course only when the plugin is enabled but when it is, it can break a lot of code out there.
What I imagined instead as a simplest implementation, is to tokenize it as a identifier and use lookup to determine if it should be a keyword in a given context (part of match expression or just a ex. function call match(something);
).
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.
Yes, that makes sense. Or consider it as a keyword only in a certain context, but more if
s would be spread across the codebase I guess.
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.
We should start a discussion on the AST structure then (spec.md change) |
@hzoo what a about an AST repo with the Babel AST spec? We could PR and discuss there. I want to avoid mixing AST and implementation discussions. |
I don't think we need an AST repo, babylon is already that repo at the very least 😛. Or just discuss in the proposal issue, I think it's fine, otherwise it would be an estree issue. |
What’s the status of this PR? What’s left to be done? Any help needed? |
@alexeyraspopov There is a small todo list in the ticket description. Unfortunately, I don't have time right now to continue work on this issue but I will come back to it in some time. One of the first steps now should be re-aligning it with spec proposal - I am sure that it developed further. Feel free to continue from where I left off ;) |
We moved babylon back into the monorepo at https://github.com/babel/babel/tree/master/packages/babylon. Unfortunately pull requests cannot be migrated between repositories automatically. If this PR is still valid please reopen it there. |
Work in progress. I am creating this PR so other people can see progress and contribute. Related issues: babel/proposals#6
I will align to all contributing guidelines in a very near future ;)
What's missing:
match
function (currently match is considered just another keyword which is obviously far from perfect)[...]
) doesnt work