prettier: Excessive line breaks for a short comparison expression

I still hold my viewpoint from #1565 that Prettier should do exactly what it does for objects literals: if the user wrote it on one line and it fits, leave it. If it doesn’t fit on one line or the user added a line-break, split out the chained calls.

It’s much less arbitrary than a magic number (3), doesn’t introduce a configuration option, and I can’t see any scenario where it’ll introduce undesirable or unpredictable behavior.

My PR: #4765. Should I get that up to date and merge it @prettier/core?

Couldn’t incorporate prettier in our codebases bcoz of this method chaining issue. Respecting the printWidth would be best option

Making it configurable won’t solve the problem because at some places you may want 3, others 4, others respect printWidth. It won’t stop being arbitrary.

Sure but I think here I think it would make sense. The threshold of 3 is arbitrary and quite a few people seem to struggle with its effects

@lehni We prevent adding any options if we can: https://prettier.io/docs/en/option-philosophy.html

I would love to see this addressed. It is the main reason why I am still refraining from using prettier in my projects.

The “3 or more chained calls” rule seems arbitrary to me. I would suggest that there should be an option that allows changing this numeric threshold, and alternatively the option could also be set to a string that tells prettier to respect the printWidth, e.g.:

{
  "maxChaining": 4
}

OR

{
  "printWidth": 120,
  "maxChaining": "printWith"
}

The option could alternatively be called "chainedCallThreshold", "maxChainedCalls", etc.

Following example is a problem for our test code.

Prettier 1.11.1

Playground

Input:

expect(cells.at(1).render().text()).toBe('link text1')
expect(cells.at(2).render().text()).toBe('link text2')
expect(cells.at(3).render().text()).toBe('link text3')
expect(cells.at(4).render().text()).toBe('link text4')

Output:

expect(
  cells
    .at(1)
    .render()
    .text()
).toBe("link text1");
expect(
  cells
    .at(2)
    .render()
    .text()
).toBe("link text2");
...

j-f1 wrote:

Maybe we should only break if one of the functions is passed a function (or the chain goes past maxWidth)

Nested functions with expect are very popular in specs.

I’ll disable this rule in specs if prettier can configured. but I already understand the option philosophy in prettier. I hope that only break if the chain goes past maxWidth.

Appendix Above example can convert to other logic that expect equality with array, FYI, I thought other example like below.

expect(cells.at(1).text()).toBe('plain text1')
expect(cells.at(2).text()).toBe('plain text2')
expect(cells.at(3).render().text()).toBe('link text1')
expect(cells.at(4).render().text()).toBe('link text2')

How to use this feature? Is there any options, settings?

Maybe we should only break if one of the functions is passed a function (or the chain goes past maxWidth), meaning that Promises would continue to break.

I completely agree with @apazzolini here. This is a great proposal, and would solve the awkwardness resulting from hard-coding a completely arbitrary threshold.

@duailibe curious to hear your thoughts on this.

The discussion about this in #1282 and #1565. The PR that introduced the behavior in #2673.

Ah. Thanks for finding that.

Maybe we should go back on that “3 or more chained calls” part? A few people have raised issues about this.

That said, we should dig up more context first before making a change…

Can you open a separate issue for that? It appears unrelated.

This is definitely not very good looking 😉

It’s really hard to come up with a rule for this, though.