Skip to content

Support RestElement#236

Merged
Constellation merged 1 commit intoestools:masterfrom
ef4:rest-element
May 27, 2015
Merged

Support RestElement#236
Constellation merged 1 commit intoestools:masterfrom
ef4:rest-element

Conversation

@ef4
Copy link
Copy Markdown
Contributor

@ef4 ef4 commented May 27, 2015

Closes #225.

@Constellation
Copy link
Copy Markdown
Member

LGTM

@Constellation
Copy link
Copy Markdown
Member

After seeing the test is pased, i'll merge it.

Constellation added a commit that referenced this pull request May 27, 2015
@Constellation Constellation merged commit f845b2c into estools:master May 27, 2015
@Constellation
Copy link
Copy Markdown
Member

Thank you!

@ArtemGovorov
Copy link
Copy Markdown

Shouldn't it be generatePattern as opposed to generateIdentifier? Spec allows the argument to be a pattern https://github.com/estree/estree/blob/master/es6.md#restelement

@ef4
Copy link
Copy Markdown
Contributor Author

ef4 commented May 28, 2015

@ArtemGovorov I see you're correct, I will submit another PR that expands coverage to all patterns.

ef4 added a commit to ef4/escodegen that referenced this pull request May 28, 2015
This is a followup to estools#236 that brings RestElement into full spec
compliance.
ef4 added a commit to ef4/escodegen that referenced this pull request May 28, 2015
This is a followup to estools#236 that brings RestElement into full spec
compliance.
@michaelficarra
Copy link
Copy Markdown
Member

That's a spec bug. I will submit an estree issue.

edit: Oops, I was thinking only of ArrayBindingPattern, not of ArrayAssignmentPattern.

@ef4
Copy link
Copy Markdown
Contributor Author

ef4 commented May 28, 2015

If it's a bug, it seems to already be implemented in multiple places. esprima supports it, and babel (uses acorn?) supports it too.

@michaelficarra
Copy link
Copy Markdown
Member

See my edit above. I made a mistake.

ArtemGovorov pushed a commit to wallabyjs/escodegen that referenced this pull request May 18, 2016
This is a followup to estools#236 that brings RestElement into full spec
compliance.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support RestElement

4 participants