Update: Option for object literals in `arrow-body-style` (fixes #5936) #6216

alberto · 2016-05-18T21:51:48Z

No description provided.

eslintbot · 2016-05-18T21:51:48Z

LGTM

mention-bot · 2016-05-18T21:51:49Z

By analyzing the blame information on this pull request, we identified @vitorbal, @gyandeeps and @pedrottimark to be potential reviewers

eslintbot · 2016-05-18T22:02:09Z

LGTM

BYK · 2016-05-20T13:12:42Z

lib/rules/arrow-body-style.js

    },

    create: function(context) {
        var always = context.options[0] === "always";
        var asNeeded = !context.options[0] || context.options[0] === "as-needed";
+        var allowObjectLiteralBody = context.options[1] && context.options[1].allowObjectLiteralBody === true;


Did you mean context.options[1].allowObjectLiteralBody !== false since the default should be true?

I got the meaning backwards. I'll change it so enforceObjectLiteralBody is true by default and you allow explicit return with false.

nzakas · 2016-05-23T19:22:35Z

@alberto can you rebase when you get a second?

eslintbot · 2016-05-25T17:06:54Z

LGTM

alberto · 2016-05-25T17:20:48Z

@BYK @nzakas PR updated and rebased

BYK · 2016-05-25T20:51:52Z

Still a bit confused about what enforceObjectLiteralBody means because I first assumed it would enforce an arrow function body so when it was set to true it would require () => { return {a:1}} and complain when it saw () => ({a: 1}).

Thoughts?

alberto · 2016-05-26T06:43:49Z

Really, did I get it wrong again 😵 ? So you now expected it to be false by default and the option to make the rule stricter?

FWIW, In both cases there is an object literal, and there is a body. What this rule is really enforcing is the presence of the block in the body. So I understood it as "enforce object literal as body".

BYK · 2016-05-26T10:58:30Z

@alberto I don't think you got it wrong. It is just very hard to get the right name. That said I put the blame mostly on myself for making you change it again and again. If you're okay let's pause and try to come up with a less ambiguous name before wasting your time. What do you think?

And I am sorry for this :(

platinumazure · 2016-05-26T14:11:42Z

For what it's worth, I don't think we should necessarily design our rules to be as strict as possible and assume that options should relax the rules. I think we should design so that the default options correspond to the most common case, and then options merely represent variations on that case.

Just my two cents. Good luck on this one!

alberto · 2016-05-26T15:52:24Z

yeah, no problem @BYK

nzakas · 2016-05-26T16:13:06Z

@platinumazure that only works when creating a new rule. When modifying an existing rule, we need to maintain backwards compatibility, and that often means introducing options that turn off parts of the rule.

What about avoidObjectLiteralBodyParens?

BYK · 2016-05-26T16:33:13Z

enforceFunctionBodyForObjectLiterals?

alberto · 2016-05-26T19:09:21Z

I don't think "function body" is the most appropriate. Blockless arrow functions also have bodies.

I think the problem comes from the fact that arrow-body-style omits the part about the block:
Maybe enforceBlockBodyForObjectliterals or avoidConciseBodyForObjetLiteral?

BYK · 2016-05-26T20:24:46Z

How about noBodyExpressionForObjectLiteral?

alberto · 2016-05-27T12:13:45Z

@eslint/eslint-team we could use some help here picking an good name :)

btmills · 2016-05-27T13:12:55Z

returnObjects?

mysticatea · 2016-05-27T21:09:36Z

I thought:

"arrow-body-style": ["error", "as-needed", {"returnObjectLiteral": "needed"}]

BYK · 2016-06-01T09:58:40Z

@alberto what do you think about @mysticatea's suggestion?

alberto · 2016-06-01T11:11:15Z

I'm OKish. We tend to use boolean properties for this things. Not that the schema validation wouldn't catch typos but...

Would you guys prefer that over something like neededForObjectLiteral or needsReturnForObjectLiteral?

BYK · 2016-06-01T11:45:08Z

@alberto boolean suggestion: requireReturnForObjectLiteral

mikesherov · 2016-06-01T12:19:02Z

👍 to requireReturnForObjectLiteral

alberto · 2016-06-01T12:33:50Z

Yes :) 👍

eslintbot · 2016-06-04T14:55:41Z

LGTM

alberto · 2016-06-04T14:57:33Z

Labelling as do not merge since it was decided to go with #5934 instead.

nzakas · 2016-06-06T16:07:33Z

@alberto where was that decided?

alberto · 2016-06-06T17:00:21Z

@nzakas in the original issue, to prioritize new contributions

nzakas · 2016-06-06T19:16:16Z

@alberto that's fine, but it would be better to link to the comment so people who aren't hopping back and forth between the issue can still follow what's going on (me, for instance).

BYK · 2016-06-09T13:41:32Z

docs/rules/arrow-body-style.md


 * `"always"` enforces braces around the function body
 * `"as-needed"` enforces no braces where they can be omitted (default)

-### "always"
+The second one is an object for more fine-grained configuration when the first option is `"as-needed"`. Currently, the only available option is `requireReturnForObjectLiteral`, a boolean property. It's `false` by default. If set to `true`, it requires braces and an explicit return for object literals.


finer-grained?

it requires braces and an explicit return for object literals.

it requires braces for the function body and an explicit return when the return value is just an object literal

@BYK I am not a native english speaker, so I'm not completely sure what's better, but we use the expression "more fine-grained" in 3 other rules.

It was just a suggestion so okay to leave it that way ;)

BYK · 2016-06-09T13:46:33Z

All inline comments are nitpicks and optional. LGTM overall. Thanks for baring with me @alberto! :)

eslintbot · 2016-06-09T19:23:10Z

LGTM

BYK · 2016-06-10T14:26:18Z

@nzakas or @ilyavolodin can we merge this? :)

jquerybot added the CLA: Valid label May 18, 2016

alberto force-pushed the issue5936 branch from faf46db to 2059e0e Compare May 18, 2016 22:02

BYK reviewed May 20, 2016
View reviewed changes

alberto force-pushed the issue5936 branch from 2059e0e to 615241f Compare May 25, 2016 17:06

ilyavolodin mentioned this pull request Jun 2, 2016

Create except-object exception to arrow-body-style rule, enforcing explicit return of object literals from arrow functions. #5936

Closed

btmills mentioned this pull request Jun 3, 2016

Update: Add never option to arrow-body-style #6317

Closed

alberto force-pushed the issue5936 branch from 615241f to 36d74fe Compare June 4, 2016 14:55

alberto added the do not merge This pull request should not be merged yet label Jun 4, 2016

BYK removed the do not merge This pull request should not be merged yet label Jun 9, 2016

BYK reviewed Jun 9, 2016
View reviewed changes

Update: Option for object literals in arrow-body-style (fixes #5936)

2b0f22a

alberto force-pushed the issue5936 branch from 36d74fe to 2b0f22a Compare June 9, 2016 19:23

ilyavolodin merged commit 9bfbc64 into master Jun 10, 2016

alberto deleted the issue5936 branch June 13, 2016 19:48

eslint-deprecated bot locked and limited conversation to collaborators Feb 6, 2018

eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Feb 6, 2018

Update: Option for object literals in arrow-body-style (fixes #5936) #6216

Update: Option for object literals in arrow-body-style (fixes #5936) #6216

Conversation

alberto commented May 18, 2016

eslintbot commented May 18, 2016

mention-bot commented May 18, 2016

eslintbot commented May 18, 2016

BYK May 20, 2016 • edited

Choose a reason for hiding this comment

alberto May 20, 2016 • edited

Choose a reason for hiding this comment

nzakas commented May 23, 2016

eslintbot commented May 25, 2016

alberto commented May 25, 2016

BYK commented May 25, 2016

alberto commented May 26, 2016 • edited

BYK commented May 26, 2016 • edited

platinumazure commented May 26, 2016

alberto commented May 26, 2016

nzakas commented May 26, 2016

BYK commented May 26, 2016

alberto commented May 26, 2016 • edited

BYK commented May 26, 2016

alberto commented May 27, 2016

btmills commented May 27, 2016

mysticatea commented May 27, 2016

BYK commented Jun 1, 2016

alberto commented Jun 1, 2016

BYK commented Jun 1, 2016

mikesherov commented Jun 1, 2016

alberto commented Jun 1, 2016

eslintbot commented Jun 4, 2016

alberto commented Jun 4, 2016

nzakas commented Jun 6, 2016

alberto commented Jun 6, 2016

nzakas commented Jun 6, 2016

BYK Jun 9, 2016

Choose a reason for hiding this comment

BYK Jun 9, 2016

Choose a reason for hiding this comment

alberto Jun 9, 2016

Choose a reason for hiding this comment

BYK Jun 10, 2016

Choose a reason for hiding this comment

BYK commented Jun 9, 2016

eslintbot commented Jun 9, 2016

BYK commented Jun 10, 2016

Update: Option for object literals in `arrow-body-style` (fixes #5936) #6216

Update: Option for object literals in `arrow-body-style` (fixes #5936) #6216

BYK May 20, 2016 •

edited

alberto May 20, 2016 •

edited

alberto commented May 26, 2016 •

edited

BYK commented May 26, 2016 •

edited

alberto commented May 26, 2016 •

edited