Wp-calypso: Docs: Javascript style guide - multi-line arrow functions are okay without braces

[samouri]

As sparked in a conversation here, @spen had the idea that we should revise our javascript style guide to allow for multiline arrow functions without braces.

The current style guide explicitly says:

// bad
// it's too magic, it works only with parentheses, otherwise it returns collection of undefine values
[ 1, 2, 3 ].map( x => ( { value: x * x } ) );

But it is becoming an increasingly common pattern to utilize brace-less multiline arrow functions.
What is everyones thoughts ? Is it a good idea to update the docs and allow items like so:

list.map( item =>
  <div>
    <li> item.name </li>
  </div>
);

[brunoscopelliti]

I think we should distinguish the two cases...

In my opinion this is good:

list.map( item =>
  <ul>
    <li>{ item.name }</li>
  </ul>
);

because the single root element provides the same visual help of a couple of parentheses.
It also works without further parentheses cause the babel react-preset reduces it to a single expression. You can note how the arrow function was not transpiled in this babel repl demo.

On the other hand the first example can't work correctly without parentheses. The parentheses are needed to help the engine distinguish the object literal { value: x * x } from a block with a label statement.
In this case, I think we should be biased through the most readable form, even at the cost of a few more key strokes. That being said, I think that @spentay has a good point here:

the first example labelled as 'bad' uses parens for execution but claims that doing so is 'too magic'... Since it seems to be a fairly common pattern for returning components it might be worth revising

That is, now that people are getting more and more familiar with ES2015, to return an object literal by wrapping it into parentheses is becoming an idiomatic form.

[spen]

@spentay had the idea that we should revise our javascript style guide to allow for multiline arrow functions without braces.

I'm actually for using braces on multi-line.
As much as I do _visually_ prefer :

list.map( item =>
    <li>
        <span>Name: { item.name }</span>
    </li>
);

I still like to add the parens:

list.map( item => (
    <li>
        <span>Name: { item.name }</span>
    </li>
) );

I don't feel _too_ strongly about this as I do find this opinion hard to defend.
One thing though: I think it keeps consistency between implicit and explicit returns:

list.map( item => {
    const name = capitalize(item.name);
    return (
        <li>
            <span>Name: { name }</span>
        </li>
    );
) );

return needs the ( ... ) wrapper to avoid ASI from doing return;

Also AirBnB recommends using parens for multi-liners.

Testing the following on chrome & FF actually works fine but I still don't completely trust ASI not to error when omitting the parens (and not using babel).

a = b =>
1
console.log( a() ) // 1

Still I feel like there's room for extra clarity here - I'd vote for adding wrapping parens to any multi-line implicitly returning arrow functions (MLIRAFs for short 😂)

I've got a PR coming with a couple of edits to the JS guidelines as I didn't realise there was one in progress (I think it's merged now though).
I'll open a separate one that might better show my thoughts on this :)

[brunoscopelliti]

@spen you've a good point on consistency between implicit and explicit returns.
In fact this could also be useful if you've to refactor a single line arrow function to a multi lines arrow function.

[spen]

@aduth's PR #9037 (merged) does cover this topic under the Iteration heading.

This seems fine to me. If anything it could be slightly more explicit about what we've discussed above but I'm on the fence! Thoughts?

[aduth]

9037 also removed the text from the original comment that referred to the arrow function usage as being "bad". To me personally, specific usage of arrow functions falls on the side of developer preference, not warranting an explicit guideline one way or the other.

[samouri]

Closing because I feel this issue was resolved in #9037.
If anyone feels different feel free to reopen

[stale[bot]]

This issue has been marked as stale because it hasn't been updated in a while. It will be closed in a week.
If you would like it to remain open, can you please you comment below and see what you can do to get things moving with this issue?
Thanks! 🙏