Closed calebmer closed 8 years ago
Are there any comments? Could we merge this? I’d like to see this functionality adopted in parsers, so it’s important to see this extension in the language.
Seems fine to me. Merged.
@calebmer @sebmarkbage Does anyone remember why we introduced a separate node type for this and not reused SpreadElement
from ESTree which has the same structure and visual syntax?
@RReverser I don’t think there was a specific reason. I defaulted to creating a new type which may be better for extensibility in the future.
What does ESTree do for spread properties (object spread)?
@sebmarkbage
interface SpreadElement <: Node {
type: "SpreadElement";
argument: Expression;
}
@sebmarkbage Ah sorry, just realised that I misunderstood your question. See https://github.com/estree/estree/blob/master/experimental/rest-spread-properties.md - it also reuses SpreadElement
/ RestElement
, hence my suggestion to use the same one in JSX for uniformity.
I'm skeptical of seeking reuse for things that are specified as separate things in the syntax spec. It seems like a fragile future compatibility issue and convenience issue. E.g. for matchers in Babel or for implementing evaluators.
According to the last TC39 meeting, rest properties will no longer allow a nested pattern - unlike rest element. Only identifiers. So they've already diverged and might diverge more in the future.
shrugs AST is mostly for syntactic representation of things, there are better IRs for evaluators and checkers. And syntactically this is indeed ...
+ expression in both places, so can be used by parsers and serializers as such.
As per discussions in https://github.com/facebook/jsx/issues/57 and https://github.com/babel/babylon/pull/42.
This syntax is the same one I used in writing https://github.com/babel/babylon/pull/42
cc @syranide, @sebmarkbage, @kittens