-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[Normative] Add Object Rest Spread #1048
Conversation
|
||
ArrayAssignmentPattern[Yield, Await] : | ||
`[` Elision? AssignmentRestElement[?Yield, ?Await]? `]` | ||
`[` AssignmentElementList[?Yield, ?Await] `]` | ||
`[` AssignmentElementList[?Yield, ?Await] `,` Elision? AssignmentRestElement[?Yield, ?Await]? `]` | ||
|
||
AssignmentRestProperty[Yield, Await] : | ||
`...` DestructuringAssignmentTarget[?Yield, ?Await] |
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.
In the spec text on the repo this AssignmentPropertyList
production has [Yield, Await]
but I'm sure it's meant to be [?Yield, ?Await]
. I've made it [?Yield, ?Await]
here to expedite things, but otherwise I'm happy to raise an issue in the repo and resolve it there before moving forward with this PR.
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.
Changing it here as you've done seems completely appropriate to me. Wouldn't hurt to make a matching PR there, but it's also not essential to do.
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.
Great work on this patch! Some comments, mostly editorial; there are a couple apparent typos. It's always hard to avoid errors here when you can't run the spec.
Minor: We generally use Normative:
rather than [Normative]
to describe the patch type.
spec.html
Outdated
@@ -4764,6 +4764,35 @@ <h1>GetFunctionRealm ( _obj_ )</h1> | |||
<p>Step 5 will only be reached if _obj_ is a non-standard function exotic object that does not have a [[Realm]] internal slot.</p> | |||
</emu-note> | |||
</emu-clause> | |||
|
|||
<!-- es6num="7.3.23" --> |
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.
es6num should not be placed on new things; it's only used for cross-referencing for things in ES2015.
spec.html
Outdated
1. ReturnIfAbrupt(_fromValue_). | ||
1. Let _excludedNames_ be a new empty List. | ||
1. Return ? CopyDataProperties(_object_, _fromValue_, _excludedNames_). | ||
</emu-alg> |
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.
Minor: I'd prefer if this new clause is added at the end of the list of productions, making it parallel with how the syntax is defined.
|
||
ArrayAssignmentPattern[Yield, Await] : | ||
`[` Elision? AssignmentRestElement[?Yield, ?Await]? `]` | ||
`[` AssignmentElementList[?Yield, ?Await] `]` | ||
`[` AssignmentElementList[?Yield, ?Await] `,` Elision? AssignmentRestElement[?Yield, ?Await]? `]` | ||
|
||
AssignmentRestProperty[Yield, Await] : | ||
`...` DestructuringAssignmentTarget[?Yield, ?Await] |
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.
Changing it here as you've done seems completely appropriate to me. Wouldn't hurt to make a matching PR there, but it's also not essential to do.
spec.html
Outdated
@@ -14306,14 +14344,18 @@ <h2>Supplemental Syntax</h2> | |||
|
|||
ObjectAssignmentPattern[Yield, Await] : | |||
`{` `}` | |||
`{` AssignmentRestProperty[?Yield, ?Await]? `}` |
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.
The ?
at the end seems a little redundant with the { }
in the previous line; I think you could choose one or the other. Looks like the proposal repository deletes line 14346; would that make sense to do here?
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.
I've kept the `{` `}`
line as this keeps the AssignmentRestProperty
syntax in line with the others.
spec.html
Outdated
@@ -14366,16 +14408,17 @@ <h1>Runtime Semantics: DestructuringAssignmentEvaluation</h1> | |||
<emu-grammar>ObjectAssignmentPattern : `{` `}`</emu-grammar> | |||
<emu-alg> | |||
1. Perform ? RequireObjectCoercible(_value_). | |||
1. Perform ? PropertyDestructuringAssignmentEvaluation for |AssignmentPropertyList| using _value_ as the argument. |
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.
I'm not sure what |AssignmentPropertyList|
refers to here. Are you sure you want to add this line?
spec.html
Outdated
ObjectAssignmentPattern : | ||
`{` AssignmentPropertyList `}` | ||
`{` AssignmentPropertyList `,` `}` | ||
<emu-grammar>ObjectAssignmentPattern : |
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.
Minor: Was deleting the newline here deliberate?
spec.html
Outdated
<h1>Runtime Semantics: PropertyDestructuringAssignmentEvaluation</h1> | ||
<p>With parameters _value_.</p> | ||
|
||
<emu-note>These collect a list of all destructured property names rather than just empty completion.</emu-note> |
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.
The "rather than" part of the note might not be necessary anymore, as it describes what changed from one version of the spec to the other.
6b57494
to
cc2fa78
Compare
@littledan I believe I've addressed all of your comments. Let me know if you have any more! |
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.
The fixed seem good AFAICT. Saw two more small issues I missed before.
spec.html
Outdated
<emu-grammar>AssignmentProperty : IdentifierReference Initializer?</emu-grammar> | ||
<emu-alg> | ||
1. Let _P_ be StringValue of |IdentifierReference|. | ||
1. Let _lref_ be ? ResolveBinding(_P_). | ||
1. Let _v_ be ? GetV(_value_, _P_). | ||
1. If |Initializer_opt| is present and _v_ is *undefined*, then | ||
1. Let _defaultValue_ be the result of evaluating |Initializer|. | ||
1. Set _v_ to ? GetValue(_defaultValue_). | ||
1. Let _v_ be ? GetValue(_defaultValue_). |
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.
I'm not sure why you made this change.
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.
I find this change confusing since it is unclear if this shadows or mutates this binding in the conditional.
spec.html
Outdated
@@ -15463,14 +15557,18 @@ <h2>Syntax</h2> | |||
|
|||
ObjectBindingPattern[Yield, Await] : | |||
`{` `}` | |||
`{` BindingRestProperty[?Yield, ?Await]? `}` |
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.
These two lines are ambiguous; it would be fine to either remove the ? Or delete the first of the two lines.
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.
Looks good to me other than the mentioned nits.
spec.html
Outdated
<emu-grammar>AssignmentProperty : IdentifierReference Initializer?</emu-grammar> | ||
<emu-alg> | ||
1. Let _P_ be StringValue of |IdentifierReference|. | ||
1. Let _lref_ be ? ResolveBinding(_P_). | ||
1. Let _v_ be ? GetV(_value_, _P_). | ||
1. If |Initializer_opt| is present and _v_ is *undefined*, then | ||
1. Let _defaultValue_ be the result of evaluating |Initializer|. | ||
1. Set _v_ to ? GetValue(_defaultValue_). | ||
1. Let _v_ be ? GetValue(_defaultValue_). |
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.
I find this change confusing since it is unclear if this shadows or mutates this binding in the conditional.
cc2fa78
to
73cd0e8
Compare
All nits addressed 😄 |
spec.html
Outdated
<emu-alg> | ||
1. Let _exprValue_ be the result of evaluating _AssignmentExpression_. | ||
1. Let _fromValue_ be GetValue(_exprValue_). | ||
1. ReturnIfAbrupt(_fromValue_). |
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.
This line should be removed, and the preceding GetValue
be prefixed with a ?
spec.html
Outdated
1. ReturnIfAbrupt(_lref_). | ||
1. Let _restObj_ be ObjectCreate(%ObjectPrototype%). | ||
1. Let _assignStatus_ be CopyDataProperties(_restObj_, _value_, _excludedNames_). | ||
1. ReturnIfAbrupt(_assignStatus_). |
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.
This line should be removed, and the preceding CopyDataProperties
be prefixed with a ?
spec.html
Outdated
<emu-alg> | ||
1. Let _restObj_ be ObjectCreate(%ObjectPrototype%). | ||
1. Let _assignStatus_ be CopyDataProperties(_restObj_, _value_, _excludedNames_). | ||
1. ReturnIfAbrupt(_assignStatus_). |
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.
This line should be removed, and the preceding CopyDataProperties
be prefixed with a ?
spec.html
Outdated
1. ReturnIfAbrupt(_assignStatus_). | ||
1. Let _bindingId_ be StringValue of |BindingIdentifier|. | ||
1. Let _lhs_ be ResolveBinding(_bindingId_, _environment_). | ||
1. ReturnIfAbrupt(_lhs_). |
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.
This line should be removed, and the preceding ResolveBinding
be prefixed with a ?
73cd0e8
to
854dd18
Compare
Done and done |
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.
Eesh, sorry, noticed these additional nits. Anyway, LGTM modulo having at least the first one (about Type) fixed.
spec.html
Outdated
<p>When the abstract operation CopyDataProperties is called with arguments _target_, _source_ and _excluded_, the following steps are taken:</p> | ||
<emu-alg> | ||
1. Assert: Type(_target_) is Object. | ||
1. Assert: Type(_excluded_) is List. |
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.
_excluded_
is not an ECMAScript object and so it doesn't have a type. A better wording is, Assert: _excluded_ is a List of property keys
.
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.
_excluded_
is not an ECMAScript object and so it doesn't have a type.
Not so: the definition of Type(x) specifically allows specification types as well as ECMAScript language types. However, it's true that this capability isn't used much, and never to identify Lists, so I agree with the suggested change.
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.
Oh, my mistake! Thanks for the background, but sounds like it's still a reasonable change.
spec.html
Outdated
1. Else, | ||
1. Let _from_ be ! ToObject(_source_). | ||
1. Let _keys_ be ? _from_.[[OwnPropertyKeys]](). | ||
1. Repeat for each element _nextKey_ of _keys_ in List order, |
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.
The wording of spec language has been getting regularized and changed a little since this proposal was written. The modern wording is, For each element _nextKey_ of _keys_, do
. For an example, see the current definition of Object.assign.
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.
It's also okay to say For each element _nextKey_ of _keys_ in List order, do
.
spec.html
Outdated
`{` AssignmentPropertyList `}` | ||
`{` AssignmentPropertyList `,` `}` | ||
`{` AssignmentPropertyList `}` | ||
`{` AssignmentPropertyList `,` `}` |
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.
What was the reason for removing this indentation?
spec.html
Outdated
<emu-grammar>BindingProperty : SingleNameBinding</emu-grammar> | ||
<emu-alg> | ||
1. Let _name_ be the string that is the only element of BoundNames of |SingleNameBinding|. | ||
1. Return the result of performing KeyedBindingInitialization for |SingleNameBinding| using _value_, _environment_, and _name_ as the arguments. | ||
1. Let _status_ be the result of performing KeyedBindingInitialization for |SingleNameBinding| using _value_, _environment_, and _name_ as the arguments. | ||
1. ReturnIfAbrupt(_status_). |
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.
If you're doing these conversions to ?
syntax, you might be able to apply it here too, and use Perform ? KeyBindingInitialization for |SingleNameBinding| using _value_, _environment_, and _name_ as the arguments.
There are some other uses of ReturnIfAbrupt in this PR that you could remove similarly. However, the current spec seems to have some uses of ?
and some ReturnIfAbrupt in a way that I don't really understand; maybe these uses are not allowed for some reason.
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.
I've changed any ReturnIfAbrupt
where the Let
value isn't used. In other words, now the only usage of ReturnIfAbrupt
is where the Let
is used later in the steps.
spec.html
Outdated
1. Let _keys_ be ? _from_.[[OwnPropertyKeys]](). | ||
1. Repeat for each element _nextKey_ of _keys_ in List order, | ||
1. Let _found_ be *false*. | ||
1. Repeat for each element _e_ of _excluded_, |
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.
Ditto the new wording here.
854dd18
to
057a36c
Compare
That should be all comments addressed again. cc @ljharb @littledan @bterlson |
spec.html
Outdated
<emu-alg> | ||
1. Assert: Type(_target_) is Object. | ||
1. Assert: _excluded_ is a List of property keys. | ||
1. If _source_ is *undefined* or *null*, let _keys_ be a new empty List. |
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.
Would it be cleaner to return target directly from here, instead of making an empty list and implicitly skipping the keys iteration?
spec.html
Outdated
1. Let _lref_ be the result of evaluating |DestructuringAssignmentTarget|. | ||
1. ReturnIfAbrupt(_lref_). | ||
1. Let _restObj_ be ObjectCreate(%ObjectPrototype%). | ||
1. Let _assignStatus_ be ? CopyDataProperties(_restObj_, _value_, _excludedNames_). |
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.
assignStatus is unused here; this should probably be Perform ? CopyDataProperties…
spec.html
Outdated
<emu-grammar>BindingRestProperty : `...` BindingIdentifier</emu-grammar> | ||
<emu-alg> | ||
1. Let _restObj_ be ObjectCreate(%ObjectPrototype%). | ||
1. Let _assignStatus_ be ? CopyDataProperties(_restObj_, _value_, _excludedNames_). |
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.
Also Perform here
I guess the following issues still apply to the current version and therefore should be fixed before the PR gets merged. tc39/proposal-object-rest-spread/issues/44 |
057a36c
to
432b866
Compare
spec.html
Outdated
1. For each element _nextKey_ of _keys_ in List order, do | ||
1. Let _found_ be *false*. | ||
1. For each element _e_ of _excluded_ in List order, do | ||
1. If SameValue(_e_, _nextKey_) is true, then |
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.
Hopefully this satisfies tc39/proposal-object-rest-spread#44 @anba
<emu-grammar>PropertyDefinition : `...` AssignmentExpression</emu-grammar> | ||
<emu-alg> | ||
1. Return ~empty~. | ||
</emu-alg> |
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.
Hopefully this satisfies tc39/proposal-object-rest-spread#42 @anba
spec.html
Outdated
|
||
<emu-grammar>BindingRestProperty : `...` BindingIdentifier</emu-grammar> | ||
<emu-alg> | ||
1. Let _lhs_ be ? ResolveBinding(StringValue of |BindingIdentifier|, _environment_). |
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.
Hopefully this satisfies tc39/proposal-object-rest-spread#36 @anba
spec.html
Outdated
1. Let _from_ be ! ToObject(_source_). | ||
1. Let _keys_ be ? _from_.[[OwnPropertyKeys]](). | ||
1. For each element _nextKey_ of _keys_ in List order, do | ||
1. Let _found_ be *false*. |
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.
I think _found_
should be _isExcluded_
or similar (little more clear what it's used for).
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.
Actually you could consider just excluded for the boolean, and use excludedNames for the list like you do further down.
spec.html
Outdated
|
||
<emu-clause id="Rest-RuntimeSemantics-PropertyDestructuringAssignmentEvaluation"> | ||
<h1>Runtime Semantics: PropertyDestructuringAssignmentEvaluation</h1> | ||
<p>With parameters _value_.</p> |
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.
Singular parameter here
spec.html
Outdated
<h1>Runtime Semantics: PropertyDestructuringAssignmentEvaluation</h1> | ||
<p>With parameters _value_.</p> | ||
|
||
<emu-note>These collect a list of all destructured property names.</emu-note> |
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.
This note isn't very descriptive to my eye. these
could be "The following operations" or "the operations in this clause". Also, can we say that collecting a list is in addition to performing evaluation semantics? This could probably just be folded into the normative clause intro prose.
I think this is great! Other than the minor comments previously, the only other issue here is the section IDs. It'll be annoying to find and replace but I'd like them to be consistent with the style used in 262 (kebab case prefixed with |
spec.html
Outdated
1. Assert: Type(_target_) is Object. | ||
1. Assert: _excluded_ is a List of property keys. | ||
1. If _source_ is *undefined* or *null*, return _target_. | ||
1. Else, |
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.
I think I'd prefer if this "Else" was just dropped because the if-branch always returns.
spec.html
Outdated
|
||
<emu-clause id="sec-copydataproperties" aoid="CopyDataProperties"> | ||
<h1>CopyDataProperties (_target_, _source_, _excluded_)</h1> | ||
<p>When the abstract operation CopyDataProperties is called with arguments _target_, _source_ and _excluded_, the following steps are taken:</p> |
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.
Comma before "and"
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.
That is a style choice I think 🤔
Is there an english grammar/style guide for ECMA?
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.
pretty sure we do use the Oxford comma, so definitely this needs to be fixed
spec.html
Outdated
1. Return _target_. | ||
</emu-alg> | ||
<emu-note> | ||
<p>The target passed is in here is always a newly created object which doesn't leak in case of an error being thrown. It's not observable when the actual object construction and initialization happens nor whether it is interleaved with reading the properties off the original object.</p> |
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.
s/is in/in/
"leak" is probably a bit too vague. The only time "leak" is used in the current spec is in the context of a resource leak. For CopyDataProperties we should instead use something along the lines of "directly accessible" wording used elsewhere in the current spec.
The second sentence could also easily be misinterpreted, because reading "properties off [sic] the original object" can't be interleaved with property evaluation of other properties in the object literal. Maybe just drop the second sentence?
@@ -11848,6 +11881,7 @@ <h1>Runtime Semantics: PropertyDefinitionEvaluation</h1> | |||
1. Perform ? PropertyDefinitionEvaluation of |PropertyDefinitionList| with arguments _object_ and _enumerable_. | |||
1. Return the result of performing PropertyDefinitionEvaluation of |PropertyDefinition| with arguments _object_ and _enumerable_. | |||
</emu-alg> | |||
<emu-grammar>PropertyDefinition : `...` AssignmentExpression</emu-grammar> |
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.
This should be next to the corresponding <emu-alg>
.
spec.html
Outdated
@@ -11868,6 +11902,12 @@ <h1>Runtime Semantics: PropertyDefinitionEvaluation</h1> | |||
1. Assert: _enumerable_ is *true*. | |||
1. Return CreateDataPropertyOrThrow(_object_, _propKey_, _propValue_). | |||
</emu-alg> | |||
<emu-alg> |
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.
Belongs after the following <emu-note>
spec.html
Outdated
<h1>Runtime Semantics: RestDestructuringAssignmentEvaluation</h1> | ||
<p>With parameters _value_ and _excludedNames_.</p> | ||
|
||
<emu-grammar>AssignmentRestProperty[Yield, Await] : `...` DestructuringAssignmentTarget</emu-grammar> |
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.
No [Yield, Await]
here.
spec.html
Outdated
<emu-alg> | ||
1. Let _excludedNames_ be the result of performing PropertyBindingInitialization of |BindingPropertyList| using _value_ and _environment_ as arguments. | ||
1. ReturnIfAbrupt(_excludedNames_). | ||
1. Return the result of performing RestBindingInitialization of _BindingRestProperty_ with _value_, _environment_ and _excludedNames_ as the arguments. |
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.
Comma before "and"
spec.html
Outdated
<emu-grammar>ObjectBindingPattern : `{` BindingRestProperty `}`</emu-grammar> | ||
<emu-alg> | ||
1. Let _excludedNames_ be a new empty List. | ||
1. Return the result of performing RestBindingInitialization of _BindingRestProperty_ with _value_, _environment_ and _excludedNames_ as the arguments. |
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.
Comma before "and".
spec.html
Outdated
</emu-alg> | ||
</emu-clause> | ||
|
||
<emu-clause id="Rest-RuntimeSemantics-PropertyBindingInitialization"> |
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.
As for destructuring assignment evaluation, I'd keep these semantics in the current <emu-clause>
.
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.
Could you clarify here please?
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.
I think he's suggesting that you can just add the new production under the existing clause rather than adding a new clause for it.
spec.html
Outdated
|
||
<emu-clause id="Rest-RuntimeSemantics-RestBindingInitialization"> | ||
<h1>Runtime Semantics: RestBindingInitialization</h1> | ||
<p>With parameters _value_, _environment_ and _excludedNames_.</p> |
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.
Comma before "and".
432b866
to
8e36950
Compare
All notes (bar #1048 (comment)) have been addressed. |
@sebmarkbage @ljharb Should this be merged? I see it already moved to Stage 4. tc39/proposals@7a04292#diff-79a53ca5c00fe0d90f18cd5237280da3 |
@styfle it will be merged when the editor is ready to do it :-) thanks for your patience! |
8e36950
to
705157d
Compare
Proposal repo: https://github.com/tc39/proposal-object-rest-spread/