Skip to content
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

RestElement should be a Pattern? #10143

Open
tolmasky opened this issue Jun 28, 2019 · 3 comments

Comments

Projects
None yet
3 participants
@tolmasky
Copy link
Contributor

commented Jun 28, 2019

ALIAS_KEYS["RestElement"] === ["LVal", "PatternLike"]

Notably missing is Pattern. Having looked through all the uses of RestElement, I cannot figure out why this would be except as possibly a holdover from when SpreadElement and RestElement were one (if I recall correctly?). RestElement only seems to be used in Patterns and as part of TS declarations, but the TS declarations themselves don't seem like they would suffer from RestElement being a pattern (since they seem to manually pick and choose). Alternatively, it would seem that maybe there should be a RestPattern vs a TSRestDeclaration or something -- but again, I think this wouldn't really be necessary.

@babel-bot

This comment has been minimized.

Copy link
Collaborator

commented Jun 28, 2019

Hey @tolmasky! We really appreciate you taking the time to report an issue. The collaborators
on this project attempt to help as many people as possible, but we're a limited number of volunteers,
so it's possible this won't be addressed swiftly.

If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack
community
that typically always has someone willing to help. You can sign-up here
for an invite.

@nicolo-ribaudo

This comment has been minimized.

Copy link
Member

commented Jun 28, 2019

I think that there should be some sort of distinction between nodes which can be "top-level patterns" and node which can be "nested patterns".
i.e. In var X = y;, X could be an ArrayPattern, ObjectPattern. RestElement and AssignmentPattern should have the same "patterness", since they can only be nested inside other patterns.

Currently there is PatternLike, which is "everything you can assign to using a declaration": Identifier, RestElement, ArrayPattern, ObjectPattern and AssignmentPattern.

@tolmasky

This comment has been minimized.

Copy link
Contributor Author

commented Jun 28, 2019

So, if that is the case, then Identifier incorrectly does not list itself as a Pattern as it is both a PatternLike and can appear at the "top level" of a Pattern.

My original thought was that the whole idea behind PatternLike actually came from the fact that Identifier is sometimes a pattern but sometimes not, due to it's overloaded nature as both "declaration" and "reference" - sometimes it is an expression, sometimes it is not. (For example, imagine if instead of ArrayPattern, we simply used ArrayExpression and SpreadElement, and the transformer "knew" to apply Expression when it was on the right-hand side, but not on the left -- this seems to be the behavior of Identifier, which despite listing itself as an Expression in ALIAS_KEYS, does not erroneously call a visitor's Expression handler when it is playing the role of a Pattern). With this understanding, I figured that RestElement must have fallen under the same "dual" role category before the split between RestElement and SpreadElement, and could thus now be safely labeled a Pattern.

Either way, it seems like either RestElement is incorrectly not a Pattern or Identifier is incorrectly not a Pattern.

I've mentioned this before, but I would much prefer a world where Identifier was simply split into more expressive types. In this case, it would be nice to have an IdentifierPattern to distinguish between Identifiers (the same way we distinguish between ArrayPatterns and ArrayExpressions). The nice thing about this is that you could know what a node is simply by holding it, unlike the situation today where the nature of a node is dependent not only on it's type, but also it's parent. As far as I know this is only true of Identifiers, which may be:

  1. Non-computed property access
  2. Referenced values
  3. Patterns
  4. Property declarations

All of which imply different things: non-computed property accesses have no affect on scope, referenced values add free variables to the scope, patterns add bound variables to the scope, and property declarations also have no effect.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.