Discussion:
[chromium-dev] ES6 in Chromium proposal: destructing
m***@chromium.org
2018-12-03 18:04:30 UTC
Permalink
Originally posted as a comment in the spread & rest proposal
<https://groups.google.com/a/chromium.org/forum/#!searchin/chromium-dev/proposing$20spread$24rest/chromium-dev/LqP4AniIs8c/GcFyY8N0AwAJ>,
but re-posting as it's own thread per suggestion.

Relevant to note, dpapad's CL for updating the style guide
<https://chromium-review.googlesource.com/c/chromium/src/+/1271915>

Proposing to move destructuring
<https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Destructuring_assignment> to
the "allowed features" section of the es style guide
<https://chromium.googlesource.com/chromium/src/+/master/styleguide/web/es.md#Destructuring-Assignment>
.

*Syntax*

let [one, two] = [1, 2];
let {ten, eleven} = {ten: 10, eleven: 11};


// skipping
let [one, two, , four] = [1, 2, 3, 4];
let {ten, eleven, thirteen} = {ten: 10, eleven: 11, twelve: 12, thirteen: 13
};


// renaming
let {ten, x: eleven} = {ten: 10, x: 11, twelve: 12};


// unwrapping nested objects
let {wrap: {wrap2: {fifty}}} = {wrap: {wrap2: {fifty: 50}}};


// default values
let [zero = 0, xnull = 0, xfalse = 0] = [undefined, null, false];
let {cats = 0, dogs = 0, firstName = '', lastName = ''} = {dogs: 3,
firstName: 'x'};
let {surname: lastName = ''} = {surname: 'false'};

*Benefits*

*1. Conciseness*

Before

people.forEach(person => {
processFirstName(person.name.first);
processLastName(person.name.last);
processAge(person.age);
processFull(`${person.name.first} ${person.name.last}`, person.age);
});

Or

people.forEach(person => {
let first = person.name.first;
let last = person.name.last;
let age = person.age;
processFirstName(first);
processLastName(last);
processAge(age);
processFull(`${first} ${last}`, age);
});

After

people.forEach(({name: {first, last}, age}) => {
processFirstName(first);
processLastName(last);
processAge(age);
processFull(`${first} ${last}`, age);
});

*2. Explicit function interfaces*

Before

// unclear what the parameter `settings` should look like without looking
at sample invocations.
function updatePage(settings) {};

After

function updatePage({bgColor, fontColor, fontSize}) {};

This issue is mitigated when type annotations are available.

*3. Explicit naming of array items*

Before

let htmlText = '<p>The blue fox ate the moon grass.</p>';
let textRegex = /\>([A-Z]\w+) ((?:\w|\s)*)([.!?])?\</;
let lineMatch = htmlText.match(textRegex);

printLarge(lineMatch[1]);
print(lineMatch[2]);
printOrDefault(lineMatch[3], '.');

Or

let htmlText = '<p>The blue fox ate the moon grass.</p>';
let textRegex = /\>([A-Z]\w+) ((?:\w|\s)*)([.!?])?\</;
let lineMatch = htmlText.match(textRegex);
let firstWord = lineMatch[1];
let remainingSentence = lineMatch[2];
let punctuation = lineMatch[3];

printLarge(firstWord);
print(remainingSentence);;
printOrDefault(punctuation, '.');

After

let htmlText = '<p>The blue fox ate the moon grass.</p>';
let textRegex = /\>([A-Z]\w+) ((?:\w|\s)*)([.!?])?\</;
let [, firstWord, remainingSentence, punctuation] = htmlText.match(textRegex
);


printLarge(firstWord);
print(remainingSentence);
printOrDefault(punctuation, '.');


*With Rest operator*

Worth noting, destructuring and rest operator can be used together.
let [first, second, third, ...honorableMentions] = [0, 1, 2, 3, 4, 5, 6, 7];
// see [1]
let {president, vicePresident, ...otherImportantPeople} = {president: 0,
vicePresident: 1, depStateSec: 2, depDefenseSec: 3, depTreasurySec: 4} //
see [2]

*Compatibility:*
iOS 10.0-10.2 <https://kangax.github.io/compat-table/es6/>
Works with eslint, clang, and Closure [see below]
[1] Rest operator in array destructuring is not supported by iOS.
[2] Rest operators in object destructuring is supported by iOS 11.1+ and
closure compiler with `ECMASCRIPT8` flag.

*Closure support*

*1. Rest operator in object destructuring.*

As noted above, this requires the `ECMASCRIPT8` flag.

*2. Typed arrays*

The below snippet fails to throw a compiler error.

/** @type number */ let one;
[one] = [false];


Similarly, for default values during array deconstruction, the below does
not throw an error.

/** @type string */ let zero;
/** @type string */ let xnull;
/** @type string */ let xfalse;
[zero = 0, xnull = 0, xfalse = 0] = [undefined, null, false];


But this is a broader issue with typed arrays in general and not unique to
destructuring expressions, e.g. the below also fails to throw an error.

/** @type Array<number> */ let array;
array = [false];


Therefore, I don't think this is an argument against allowing
destructuring, but still something worth mentioning.
--
--
Chromium Developers mailing list: chromium-***@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev+***@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/ba5985dd-a289-4cec-882c-389d41e98aa5%40chromium.org.
Dan Beam
2018-12-04 02:19:46 UTC
Permalink
Post by m***@chromium.org
Originally posted as a comment in the spread & rest proposal
<https://groups.google.com/a/chromium.org/forum/#!searchin/chromium-dev/proposing$20spread$24rest/chromium-dev/LqP4AniIs8c/GcFyY8N0AwAJ>,
but re-posting as it's own thread per suggestion.
Relevant to note, dpapad's CL for updating the style guide
<https://chromium-review.googlesource.com/c/chromium/src/+/1271915>
Proposing to move destructuring
<https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Destructuring_assignment> to
the "allowed features" section of the es style guide
<https://chromium.googlesource.com/chromium/src/+/master/styleguide/web/es.md#Destructuring-Assignment>
.
Preface: thank you for wanting to participate in this process and make web
development in Chromium better :).

I guess my only 2 cents to add here is: many of the simple examples on MDN
are great and easy to read:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Destructuring_assignment#Browser_compatibility
but some of the more complex ones (i.e. defaults and destructuring and
rest, possibly with objects) seem maybe *harder *to read than just ...
"structured" versions. So maybe add a cautionary note about "don't go nuts
/ cause code be to be less readable because of this" when/if we allow this.
Post by m***@chromium.org
*Syntax*
let [one, two] = [1, 2];
let {ten, eleven} = {ten: 10, eleven: 11};
// skipping
let [one, two, , four] = [1, 2, 3, 4];
13};
// renaming
let {ten, x: eleven} = {ten: 10, x: 11, twelve: 12};
// unwrapping nested objects
let {wrap: {wrap2: {fifty}}} = {wrap: {wrap2: {fifty: 50}}};
// default values
let [zero = 0, xnull = 0, xfalse = 0] = [undefined, null, false];
let {cats = 0, dogs = 0, firstName = '', lastName = ''} = {dogs: 3,
firstName: 'x'};
let {surname: lastName = ''} = {surname: 'false'};
*Benefits*
*1. Conciseness*
Before
people.forEach(person => {
processFirstName(person.name.first);
processLastName(person.name.last);
processAge(person.age);
processFull(`${person.name.first} ${person.name.last}`, person.age);
});
Or
people.forEach(person => {
let first = person.name.first;
let last = person.name.last;
let age = person.age;
processFirstName(first);
processLastName(last);
processAge(age);
processFull(`${first} ${last}`, age);
});
After
people.forEach(({name: {first, last}, age}) => {
processFirstName(first);
processLastName(last);
processAge(age);
processFull(`${first} ${last}`, age);
});
*2. Explicit function interfaces*
Before
// unclear what the parameter `settings` should look like without looking
at sample invocations.
function updatePage(settings) {};
After
function updatePage({bgColor, fontColor, fontSize}) {};
This issue is mitigated when type annotations are available.
*3. Explicit naming of array items*
Before
let htmlText = '<p>The blue fox ate the moon grass.</p>';
let textRegex = /\>([A-Z]\w+) ((?:\w|\s)*)([.!?])?\</;
let lineMatch = htmlText.match(textRegex);
printLarge(lineMatch[1]);
print(lineMatch[2]);
printOrDefault(lineMatch[3], '.');
Or
let htmlText = '<p>The blue fox ate the moon grass.</p>';
let textRegex = /\>([A-Z]\w+) ((?:\w|\s)*)([.!?])?\</;
let lineMatch = htmlText.match(textRegex);
let firstWord = lineMatch[1];
let remainingSentence = lineMatch[2];
let punctuation = lineMatch[3];
printLarge(firstWord);
print(remainingSentence);;
printOrDefault(punctuation, '.');
After
let htmlText = '<p>The blue fox ate the moon grass.</p>';
let textRegex = /\>([A-Z]\w+) ((?:\w|\s)*)([.!?])?\</;
let [, firstWord, remainingSentence, punctuation] = htmlText.match(
textRegex);
printLarge(firstWord);
print(remainingSentence);
printOrDefault(punctuation, '.');
*With Rest operator*
Worth noting, destructuring and rest operator can be used together.
let [first, second, third, ...honorableMentions] = [0, 1, 2, 3, 4, 5, 6, 7
]; // see [1]
let {president, vicePresident, ...otherImportantPeople} = {president: 0,
vicePresident: 1, depStateSec: 2, depDefenseSec: 3, depTreasurySec: 4} //
see [2]
The combo of rest and destructuring is interesting. I can see why you
originally sent together.
Post by m***@chromium.org
*Compatibility:*
iOS 10.0-10.2 <https://kangax.github.io/compat-table/es6/>
Works with eslint, clang, and Closure [see below]
How did you test each tool?
Post by m***@chromium.org
[1] Rest operator in array destructuring is not supported by iOS.
[2] Rest operators in object destructuring is supported by iOS 11.1+ and
closure compiler with `ECMASCRIPT8` flag.
I assume you mean we'd have to change language_in to ECMASCRIPT8 in
closure_args.gni?
https://cs.chromium.org/chromium/src/third_party/closure_compiler/closure_args.gni?type=cs&sq=package:chromium&g=0&l=40
Has anybody tried this / do we know how much work it'd be?

-- Dan
Post by m***@chromium.org
*Closure support*
*1. Rest operator in object destructuring.*
As noted above, this requires the `ECMASCRIPT8` flag.
*2. Typed arrays*
The below snippet fails to throw a compiler error.
[one] = [false];
Similarly, for default values during array deconstruction, the below does
not throw an error.
[zero = 0, xnull = 0, xfalse = 0] = [undefined, null, false];
But this is a broader issue with typed arrays in general and not unique to
destructuring expressions, e.g. the below also fails to throw an error.
array = [false];
Therefore, I don't think this is an argument against allowing
destructuring, but still something worth mentioning.
--
--
Chromium Developers mailing list: chromium-***@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev+***@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CANNW_Qo%3DrQrAESONEx0koV2Rekwua8p10bB30423qF_%2BKUvp7g%40mail.gmail.com.
manukh via Chromium-dev
2018-12-04 16:16:41 UTC
Permalink
Post by Dan Beam
How did you test each tool?
iOS: checked the table here https://kangax.github.io/compat-table/es6/

For eslint, clang, and closure, I
1) added destructuring examples covering the use cases above to the
existing file resources/md_bookmarks/list.js,
2) added closure annotations,
3) added a few intentional eslint, closure annotation, and formatting
errors,
4) ran the respective commands (see below),
5) ensured the intentional errors were caught (or reformatted) and no other
errors were thrown.

eslint: `git cl presubmit`
clang: `git cl format --js`
closure:
ninja -C out/Default -j 1000 chrome chrome/browser/resources/md_bookmarks:
closure_compile

Let me know if one this wasn't sufficient verification.

I assume you mean we'd have to change language_in to ECMASCRIPT8 in
Post by Dan Beam
closure_args.gni?
https://cs.chromium.org/chromium/src/third_party/closure_compiler/closure_args.gni?type=cs&sq=package:chromium&g=0&l=40
Yes. I meant `ECMASCRIPT_2018` (it's already set to es8 / es2017).

Has anybody tried this / do we know how much work it'd be?
closure gave a couple of the below errors. I didn't resolve these to see
if it would then give more errors.

../../chrome/browser/resources/md_bookmarks/app.js:10: ERROR - Behaviors
must be global names or qualified names that are declared as object
literals or array literals of other valid Behaviors.
bookmarks.StoreClient,
^^^^^^^^^^^^^^^^^^^^^

../../chrome/browser/resources/md_bookmarks/command_manager.js:11: ERROR -
A Polymer() declaration cannot use 'const'.
const CommandManager = Polymer({
^^^^^^^^^


Regarding the rest operator in object destructuring, it fails eslint as
well.
--
--
Chromium Developers mailing list: chromium-***@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev+***@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/941a1388-203a-491c-b49e-785b060902f5%40chromium.org.
Tommy Li
2018-12-04 17:14:40 UTC
Permalink
+1 to dbeam's suggestion that there should be a disclaimer about using
these features judiciously where they add readability.

For instance, if I saw:
let {wrap: {wrap2: {fifty}}} = {wrap: {wrap2: {fifty: 50}}};
let {cats = 0, dogs = 0, firstName = '', lastName = ''} = {dogs: 3,
firstName: 'x'};

in a code review, I'd probably say: whoa calm down there.

That being said, I'm overall very supportive, these two lines, for
instance, I think should be a no-brainer Yes:
```
let [one, two] = [1, 2];
let {ten, eleven} = {ten: 10, eleven: 11};
```
Post by Dan Beam
Post by m***@chromium.org
Originally posted as a comment in the spread & rest proposal
<https://groups.google.com/a/chromium.org/forum/#!searchin/chromium-dev/proposing$20spread$24rest/chromium-dev/LqP4AniIs8c/GcFyY8N0AwAJ>,
but re-posting as it's own thread per suggestion.
Relevant to note, dpapad's CL for updating the style guide
<https://chromium-review.googlesource.com/c/chromium/src/+/1271915>
Proposing to move destructuring
<https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Destructuring_assignment> to
the "allowed features" section of the es style guide
<https://chromium.googlesource.com/chromium/src/+/master/styleguide/web/es.md#Destructuring-Assignment>
.
Preface: thank you for wanting to participate in this process and make web
development in Chromium better :).
I guess my only 2 cents to add here is: many of the simple examples on MDN
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Destructuring_assignment#Browser_compatibility
but some of the more complex ones (i.e. defaults and destructuring and
rest, possibly with objects) seem maybe *harder *to read than just ...
"structured" versions. So maybe add a cautionary note about "don't go nuts
/ cause code be to be less readable because of this" when/if we allow this.
Post by m***@chromium.org
*Syntax*
let [one, two] = [1, 2];
let {ten, eleven} = {ten: 10, eleven: 11};
// skipping
let [one, two, , four] = [1, 2, 3, 4];
13};
// renaming
let {ten, x: eleven} = {ten: 10, x: 11, twelve: 12};
// unwrapping nested objects
let {wrap: {wrap2: {fifty}}} = {wrap: {wrap2: {fifty: 50}}};
// default values
let [zero = 0, xnull = 0, xfalse = 0] = [undefined, null, false];
let {cats = 0, dogs = 0, firstName = '', lastName = ''} = {dogs: 3,
firstName: 'x'};
let {surname: lastName = ''} = {surname: 'false'};
*Benefits*
*1. Conciseness*
Before
people.forEach(person => {
processFirstName(person.name.first);
processLastName(person.name.last);
processAge(person.age);
processFull(`${person.name.first} ${person.name.last}`, person.age);
});
Or
people.forEach(person => {
let first = person.name.first;
let last = person.name.last;
let age = person.age;
processFirstName(first);
processLastName(last);
processAge(age);
processFull(`${first} ${last}`, age);
});
After
people.forEach(({name: {first, last}, age}) => {
processFirstName(first);
processLastName(last);
processAge(age);
processFull(`${first} ${last}`, age);
});
*2. Explicit function interfaces*
Before
// unclear what the parameter `settings` should look like without looking
at sample invocations.
function updatePage(settings) {};
After
function updatePage({bgColor, fontColor, fontSize}) {};
This issue is mitigated when type annotations are available.
*3. Explicit naming of array items*
Before
let htmlText = '<p>The blue fox ate the moon grass.</p>';
let textRegex = /\>([A-Z]\w+) ((?:\w|\s)*)([.!?])?\</;
let lineMatch = htmlText.match(textRegex);
printLarge(lineMatch[1]);
print(lineMatch[2]);
printOrDefault(lineMatch[3], '.');
Or
let htmlText = '<p>The blue fox ate the moon grass.</p>';
let textRegex = /\>([A-Z]\w+) ((?:\w|\s)*)([.!?])?\</;
let lineMatch = htmlText.match(textRegex);
let firstWord = lineMatch[1];
let remainingSentence = lineMatch[2];
let punctuation = lineMatch[3];
printLarge(firstWord);
print(remainingSentence);;
printOrDefault(punctuation, '.');
After
let htmlText = '<p>The blue fox ate the moon grass.</p>';
let textRegex = /\>([A-Z]\w+) ((?:\w|\s)*)([.!?])?\</;
let [, firstWord, remainingSentence, punctuation] = htmlText.match(
textRegex);
printLarge(firstWord);
print(remainingSentence);
printOrDefault(punctuation, '.');
*With Rest operator*
Worth noting, destructuring and rest operator can be used together.
let [first, second, third, ...honorableMentions] = [0, 1, 2, 3, 4, 5, 6,
7]; // see [1]
let {president, vicePresident, ...otherImportantPeople} = {president: 0,
vicePresident: 1, depStateSec: 2, depDefenseSec: 3, depTreasurySec: 4} //
see [2]
The combo of rest and destructuring is interesting. I can see why you
originally sent together.
Post by m***@chromium.org
*Compatibility:*
iOS 10.0-10.2 <https://kangax.github.io/compat-table/es6/>
Works with eslint, clang, and Closure [see below]
How did you test each tool?
Post by m***@chromium.org
[1] Rest operator in array destructuring is not supported by iOS.
[2] Rest operators in object destructuring is supported by iOS 11.1+ and
closure compiler with `ECMASCRIPT8` flag.
I assume you mean we'd have to change language_in to ECMASCRIPT8 in
closure_args.gni?
https://cs.chromium.org/chromium/src/third_party/closure_compiler/closure_args.gni?type=cs&sq=package:chromium&g=0&l=40
Has anybody tried this / do we know how much work it'd be?
-- Dan
Post by m***@chromium.org
*Closure support*
*1. Rest operator in object destructuring.*
As noted above, this requires the `ECMASCRIPT8` flag.
*2. Typed arrays*
The below snippet fails to throw a compiler error.
[one] = [false];
Similarly, for default values during array deconstruction, the below does
not throw an error.
[zero = 0, xnull = 0, xfalse = 0] = [undefined, null, false];
But this is a broader issue with typed arrays in general and not unique
to destructuring expressions, e.g. the below also fails to throw an error.
array = [false];
Therefore, I don't think this is an argument against allowing
destructuring, but still something worth mentioning.
--
--
Chromium Developers mailing list: chromium-***@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev+***@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAMWG5%3D5oSStd5c265%2BUPZz9L_PpNMO6QN1pG3OXGqac%3DMYcOkw%40mail.gmail.com.
Loading...