Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

New rule: newline-per-chained-call #3253

Closed
jaufgang opened this issue Sep 27, 2017 · 11 comments
Closed

New rule: newline-per-chained-call #3253

jaufgang opened this issue Sep 27, 2017 · 11 comments
Labels
Formatting rule Relates to a rule which enforces code formatting and likely overlaps with prettier Type: Rule Suggestion

Comments

@jaufgang
Copy link

Please add a rule based on this one from tslint:

https://eslint.org/docs/rules/newline-per-chained-call

This rule requires a newline after each call in a method chain or deep member access. Computed property accesses such as instance[something] are excluded.

Chained method calls on a single line without line breaks are harder to read, so some developers place a newline character after each method call in the chain to make it more readable and easy to maintain.

Fails:

d3.select("body").selectAll("p").data([4, 8, 15, 16, 23, 42 ]).enter().append("p").text(function(d) { return "I'm number " + d + "!"; });

Passes:

d3
    .select("body")
    .selectAll("p")
    .data([
        4,
        8,
        15,
        16,
        23,
        42
    ])
    .enter()
    .append("p")
    .text(function (d) {
        return "I'm number " + d + "!";
    });
@ajafff
Copy link
Contributor

ajafff commented Sep 28, 2017

Some thoughts:

Add an option to allow the first property access to be on the same line?

foo.doStuff()
    .doMoreStuff()
    .finish();

What about this:

someArray
    .map(
        (el) => el,
    ).filter(
        (el) => !!el,
    ).forEach(
        (el) => console.log(el),
    );

Does the threshold depend on the number of calls or the number of property accesses?

some.deeply.nested.property.pop();
// vs.
some
    .deeply
    .nested
    .property
    .pop();

@aervin
Copy link
Contributor

aervin commented Sep 30, 2017

I like this rule idea. Personally, I prefer that the first prop be on its own line. I also think the rule should depend on the number of property accesses, not method calls alone.

Does styling inline callback args deserve its own rule?

@JoshuaKGoldberg
Copy link
Contributor

This was just released - issue can be closed? :)

@mmkal
Copy link
Contributor

mmkal commented Mar 7, 2018

@ajafff @adidahiya @aervin can I make a request that this rule be made configurable? It's pretty much unusable as-is in projects that use jest, because this sort of thing is a violation:

expect(foo).toEqual(bar)

i.e. if we were to turn it on, every single test assertion would have to be 'fixed'. And after fixing, they'd look worse.

Some suggestions for configuration options:

all-or-nothing

{ "rules": { "newline-per-chained-call": "all-or-nothing" } }

either all chained methods get a new line, or none of them do. e.g. these are ok:

someObj.foo().bar()
someObj
    .foo()
    .bar()

but this isn't:

someObj.foo()
    .bar()

ignore-prefix: string[]

whitelist certain expression prefixes that should be allowed to not newline chained calls, e.g.

{ "rules": { "newline-per-chained-call": [true, { "ignore-prefix": ["expect", "jest"] }] } }

would ignore these:

expect(foo).toEqual(bar)
const mockThing = jest.fn().mockReturnValue(123)

ignore-chain-with-depth: number

same as in the eslint rule.

{ "rules": { "newline-per-chained-call": [true, { "ignore-chain-with-depth": 2 }] } }

@aervin
Copy link
Contributor

aervin commented Mar 9, 2018

A while back I made some local updates that allow this config:

"newline-per-chained-call": [true, { "max-calls-per-line": 2 }]

which passes cases such as

expect(foo).toEqual(bar)
const mockThing = jest.fn().mockReturnValue(123)

PRs are piling up right now. Once the repo gets back on its feet, I'll open a PR.

@TeoTN
Copy link

TeoTN commented Mar 22, 2018

I'd love to see that max-calls-per-line!

@janez-svetin
Copy link

@aervin can you share your current local change for this you've mentioned? I would like to use it as a custom rule in our project until this gets added. Many thanks!

@aervin
Copy link
Contributor

aervin commented May 9, 2018

@janez-svetin just checked it into my fork. There is some test coverage, but this change has not been reviewed so user beware!

@jaufgang
Copy link
Author

jaufgang commented May 9, 2018

max-calls-per-line provides a useful way of allowing short readable method chains to appear on one line, but there is more to the question of what might constitute a "short, readable" method chain than merely how many methods there are in a chain.

for example, you might want to allow

obj.foo(1).foo(2).foo(3);

but not

somePromise.then((val)=>doSomething(val)).catch((err)=>handleError(err))

Even though the first sample has 3 calls in the chain but the second one only has 2. Could there be a config that determines whether to allow method single line call chains based on the types of arguments to the calls? i.e. a certain number of calls with simple arguments are allowed on a single line, but calls with functions or maybe object literals cannot be chained on a single line?

@gligoran
Copy link

gligoran commented Oct 1, 2019

Any update on making this rule more configurable?

@adidahiya adidahiya added the Formatting rule Relates to a rule which enforces code formatting and likely overlaps with prettier label Oct 1, 2019
@adidahiya
Copy link
Contributor

@gligoran no, we are not investing in code formatting rules in TSLint anymore, see #4534.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Formatting rule Relates to a rule which enforces code formatting and likely overlaps with prettier Type: Rule Suggestion
Projects
None yet
Development

No branches or pull requests

9 participants