Update Coding Guide with notes on destructuring assignment

[jjhembd]

As flagged by @javagl in #12188 (review): The current Coding Guide does not discuss destructuring assignment.

Since #10486, many ES6 patterns have naturally found their way into the code, including destructuring assignments. For example,

const {
  option1,
  option2,
  option3 = 3.0,
} = options;

This syntax is often both more concise and easier to read, compared to the legacy pattern:

const option1 = options.option1;
const option2 = options.option2;
const option3 = defaultValue(options.option3, 3.0);

This raises 2 questions for the Coding Guide:

• Should we recommend destructuring where appropriate?
• How do we clarify where destructuring defaults can be used, and where it is necessary to use defaultValue? The destructuring default syntax will replace undefined with the given default, but will pass null through.

[jjspace]

Personally I'm a fan of destructuring everywhere it makes sense. I don't think it's main benefit is default values though. I'll also mention #11674 just to crosslink related issues.

[ggetz]

Rather than putting this in the coding guide, this should be enforced through eslint: https://eslint.org/docs/latest/rules/prefer-destructuring

[ggetz]

Are there any performance implications to using destructuring assignment everwhere?

[javagl]

Regarding the handling of null, compared to defaultValue: There are very few places where null is used at all. So that may not be critical.

Regarding the performance: There are some claims, but ... these are soooo 2018 (i.e. it's hard to measure that objectively across browsers and JITs).

Regarding the eslint:
For now, this was rather asking whether this should become part of the coding guide. Iff it should become the recommended way, then yes: Everything that can be covered with a linter should not be mentioned in the coding guide. (Except, maybe, caveats like the null stuff, TBD).

But... highly subjective: I'm not sure about enforcing that particular rule. Destructuring assignment can be convenient when you really just have an object with 10 properties and want to pull out these 10 properties as local variables. But in some cases, the syntax is really obscure. For example, consider something like

let bar;
...
bar = object.bar;

Now, that innocent, bread-and-butter, self-explanatory last line would be flagged by eslint. It should actually be written as
({ bar } = object);
(Yeah, really... 🤪 )

[jjspace]

Now, that innocent, bread-and-butter, self-explanatory last line would be flagged by eslint. It should actually be written as
({ bar } = object);
(Yeah, really... 🤪 )

I was going to call out that specific example too, that looks awful to me. I think there's merit for at least a mention of destructuring in the coding guide as a suggestion "where it makes sense to do so" but I'm not sure I like the way eslint may choose to enforce it. Most of the time I think it's a subjective thing on a case by case basis of whatever makes the code more readable.

On performance, I have never seen anything mentioned that one is better or worse. I'd assume with how long it's been around most browsers should be able to optimize around both pretty well. That said I haven't done any research into it so I could be wrong.