Tslint: New rule: newline-per-chained-call

Created on 27 Sep 2017  路  11Comments  路  Source: palantir/tslint

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 + "!";
    });
Formatting rule Rule Suggestion

Most helpful comment

@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 }] } }

All 11 comments

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();

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?

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

@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 }] } }

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.

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

@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!

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

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?

Any update on making this rule more configurable?

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

Was this page helpful?
0 / 5 - 0 ratings