Closed Bug 1377349 Opened 6 years ago Closed 6 years ago

Remove support for nested destructuring in object rest properties

Categories

(Core :: JavaScript Engine, enhancement)

Product:
Core
Component:
JavaScript Engine
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla57
Tracking Flags:
Tracking Status
firefox57 --- fixed

People

(Reporter: anba, Assigned: anba)

Details

Attachments

(1 file)

bug1377349.patch
28.29 KB, patch
shu
: review+
Details | Diff | Splinter Review 
Nested destructuring patterns are no longer allowed in object rest/spread per https://github.com/tc39/tc39-notes/blob/master/es8/2017-05/may-23.md#conclusionresolution-10
Summary: Remove support for nested destructuring in object rest/spread → Remove support for nested destructuring in object rest properties
Attached patch bug1377349.patch — — Splinter Review 
Patch to forbid patterns after rest-properties in destructuring contexts. 

The new if-condition in checkDestructuringAssignmentTarget() checks for unparenthesised and parenthesised patterns to avoid displaying the "destructuring patterns in assignments can't be parenthesized" error message.

With the next test262 update we should get new tests to ensure destructuring patterns cannot be used with object rest-properties.

I plan to request uplift for this patch to beta, to ensure our object rest-properties implementation is compliant to the latest spec proposal when we ship this feature in moz55.
Attachment #8884854 - Flags: review?(shu)
Comment on attachment 8884854 [details] [diff] [review]
bug1377349.patch

Review of attachment 8884854 [details] [diff] [review]:
-----------------------------------------------------------------

Looks great. Really sorry about the huge review delay.
Attachment #8884854 - Flags: review?(shu) → review+
(In reply to Shu-yu Guo [:shu] from comment #2)
> Looks great. Really sorry about the huge review delay.

TBH I'm not too happy we've now shipped a version which is no longer compliant with the latest spec proposal, because the whole process from raising the issue, getting an agreement on TC39, having a fixed draft, updating the implementation, and getting it reviewed took quite some time. But I guess there shouldn't be much users' outrage because of this. :-)
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/074fb831cd7b
Remove support for nested destructuring in object rest properties. r=shu
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/074fb831cd7b
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox57: --- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.