Don’t break chains if they fit on one line & they were written that way

[j-f1]

Ref #1565 & #3107.

[j-f1]

cc @stephenh and @vjeux

[vjeux]

This is interesting. I'm not sure yet what I think about it. Would be nice to make all the examples that were written in the issues go through this PR and see how they look like now.

[bakkot]

Would be nice to make all the examples that were written in the issues go through this PR and see how they look like now.

And add them as snapshots! :D

[j-f1]

@bakkot & @azz done 🔼

[suchipi]

I expect this will fix a lot of the complaints we've had about breaking chains in enzyme tests, too

[stephenh]

Thanks for taking the time to submit a concrete change/proposal!

[duailibe]

@stephenh The snapshots also contain the inputs (original code). The formatted parts are below “~~~~~~~~”

[stephenh]

@duailibe ah shoot, sorry, newbie mistake. Thanks!

You're right, I'd assumed the snapshot was only formatted/expected output.

Might be nice to have smaller issue-1565-comment-X.js files, so that before/after are right next to each other by the ~~~~~~.

[j-f1]

Any ideas why Travis and Appveyor can’t see the relevant snapshots?

[abemedia]

Is this PR still active? Would be really good to see this feature merged in! (I hate how prettier currently deals with chained methods...)

[pixelchutes]

Excited to find this thread and see the great progress towards this feature request.

@j-f1: has @duailibe been able to answer your question re: the failing Travis/Appveyor checks?

[duailibe]

Sorry I've been missing I'll review it this weekend

[azz]

I liked the original.

[j-f1]

If you like it better on multiple lines, you can manually break it and it’ll stay that way.

[azz]

That's the thing though, If I was writing it I'd (hopefully) understand it and probably want it to be on one line. But the next person to read the code would have found it more readable when split onto multiple lines 😉. Part of the value of Prettier is making other people's code easier to read, less so your own.

[j-f1]

Personally, I’d write it like was output before, since it’s doing multiple things instead of retrieving an object:

hmac
  .update("some data to hash") // add some data
  .update(buf) // add some more data
  .digest() // get the hash

With this PR, that format would be preserved, with or without the comments.

[RomainLanz]

Looking forward for this feature to be merged 👍

[ikatyang]

I believe Travis/AppVeyor is based on the master branch while Circle is based on the original branch.

Travis/AppVeyor:

git clone https://github.com/prettier/prettier
git fetch origin +refs/pull/4765/merge:
git checkout -f FETCH_HEAD

Circle:

git clone https://github.com/prettier/prettier
git fetch --force origin "pull/4765/head:remotes/origin/pull/4765"
git reset --hard "$CIRCLE_SHA1"
git checkout -B "$CIRCLE_BRANCH"
git reset --hard "$CIRCLE_SHA1"

---

Snapshot Summary
 › 10 snapshots failed from 1 test suite. Inspect your code changes or run `yarn test -u` to update them.
 › 10 snapshots obsolete from 1 test suite. To remove them all, run `yarn test -u`.

[MarvinJWendt]

First of all I really like prettier. This implementation would fix my only concern about using prettier. For example:

I have this very easy to read if statement

if ($(el).attr("href").includes("/wiki/")) {
  ...
}

And prettier turns it into

if (
$(el)
 .attr("href")
 .includes("/wiki/")
) {
  ...
}

I like the first one waaay more.

Let's just say we are nesting some if statements:

if (
  $(el)
  .attr("href")
  .includes("/wiki/")
) {
  if (
    $(el)
    .attr("xyz")
    .includes("/whatever/")
  ) {
    if (
      $(el)
      .attr("hello")
      .includes("/world/")
    ) {
      ...
    }
  }
}

.. could be turned into:

if ($(el).attr("href").includes("/wiki/")) {
  if ($(el).attr("xyz").includes("/whatever/")) {
    if ($(el).attr("hello").includes("/world/")) {
      ...
    }
  }
}

I think that the majority would prefer the compact one-line version.

---

But

The point that we use prettier on other peoples code is solid. Yes we do not want them to decide the style. We want to use prettier and reformat all of their code to a readable version of it.

I guess one way to implement this is with a config option.
Something like: "prefer-oneline: [true/false]" for example.

I’ve probably missed the discussion elsewhere by why not <= 3 don’t break, > 3 always break? That’s similar to what Airbnb does and it appears to work well.

I really like this approach. Prettier wouldn't decide on the users code then.
(This could also have a config key to increase/decrease the number of allowed chains to make absolutely everyone happy.)

As much as I'd like to have prettier format the code in my way, I agree with @azz that this is not how prettier should work. There are other formatters for that. Prettier should format every piece of code to the same result (if the same config is given).

TLDR:

<= 3 don’t break, > 3 always break

Seems like a good solution for me.

[epidemian]

Hi 👋

Total Prettier newbie here. I really dig the "ctrl+s and let the machine take care of formatting" approach that Prettier endorses, and i want to incorporate it in my everyday usage 🙂. But when integrating Prettier on some small projects, i noticed these sort of formatting choices related to either splitting single-line expressions (below the print-width value) into multiple lines or collapsing muti-line expressions and they caught me off guard.

I think the approach proposed in this PR would be better for Prettier than the current behavior.

As a simple example, i had this function on a little project:

// Parses a string like '321' into [1, 2, 3]
const parseNumbers = s => s.split('').map(Number).sort()

Which Prettier formatted as:

const parseNumbers = s =>
  s
    .split('')
    .map(Number)
    .sort()

This function is inside the scope of a bigger function, and i intended it to be just a simple detail. Therefore the implementation as a succinct one-liner. To me, Prettier's re-formatting of this function is problematic for various reasons:

• First, it was very unintuitive. The original code being below print-width, i had no idea why it was being formatted like that, and i thought i had some syntax error or something. I noticed that when i removed the .sort() or even the parentheses on that .sort call, the code was re-formatted to a one-liner. It took me quite some time to understand that this was an intended behavior.

• It feels arbitrary and inconsistent with the rest of formatting rules. For example, the same function could be implemented in terms of top-level functions as s => sort(map(Number, split('', s))) or with getters like: s => s.chars.map(Number).sorted, and none of those would be split into multiple lines, even though, semantically, the operations are the same. Why would chained method calls be special in this regard compared to property accesses or function calls?

• Lastly, and this is very subjective. It doesn't respect the writer's intent of splitting a statement into multiple lines or not. In the example i mentioned, my intent was for the little function to be just a passing detail, nothing of importance, but Prettier's formatting makes it take a considerable chunk of the scope it's declared in, so it seems much more important.

Expanding a bit on the last point, which i think is the most important, deciding whether a single piece of code ("statement", "expression") should be split into multiple simpler ones or not is a core part of programming. It's not the same as a menial formatting adding spaces between operators, which i think everyone using Prettier agrees should be automated 🙂

For example, a statement like:

me.eat(new Cake(getCakeIngredients()))

Could safely be written as:

const ingredients = getCakeIngredients()
const theWholeCake = new Cake(ingredients)
me.eat(theWholeCake)

For the computer, they behave exactly the same. But for human readers, i could imagine cases where either of these styles would be preferable. Each has its pros and cons. For example, if i wanted to debug line-by-line, or to emphasize that i'm eating the whole cake, i would prefer the second example. But if that me.eat() call is part of many other steps to do, maybe the second style would feel too verbose and the first one would be preferable.

What i'm really sure is that nobody would expect an automatic tool deciding to transform the first snippet into the second one or vice-versa. For now at least, it's part of a human programmer's job to make that decision, and maybe for tools to assist them in that if necessary.

On a similar vein, i wouldn't want a an automatic tool transforming this:

me
  .grabCakeIngredients()
  .prepareCake()
  .eat()

Into this:

me.grabCakeIngredients().prepareCake().eat()

Or vice-versa.

If the programmer wrote it in separate lines, it's because they wanted to communicate that each method is to be thought as a "separate step". Whereas if they decided to write it on a single line they want to communicate that this chain is to be considered a single operation. Leaving aside the eating cake example, it's easy to imagine examples for either case.

As a bad analogy, i think there's not so much controversy with other formatting rules of Prettier because they are like fixing punctuation errors or typos on prose. It's very useful to have a tool that fixes those automatically. But the rule of splitting method chains into multiple lines is like splitting longish sentences into many short ones. Maybe useful for a tool to notice, but not its decision to make 🙂

---

Or, putting this in another way, and quite more bluntly: do you think there would be as many requests to change the formatting of chained method calls if the original rule was the one proposed on this PR?

[toschlog]

Please make this change. I'm considering using Prettier because we're bringing on someone who uses it, but I can't stomach splitting chains into multiple lines.

[VictorCamargo]

Can't wait for this...

[hoshinokanade]

Splitting chaining calls are against to what Prettier suggests.

Prettier, on the other hand, strives to fit the most code into every line.
source

When it comes to fluent api, Prettier is fitting almost no code into every line.
I do think Prettier should stay with the mentioned style at the same time to response to our voices.
Please get it merged.

[rikkertkoppes]

I may have overlooked it, but has no one proposed to make the number of chain components configurable? That is:

• break into separate lines if we have more than n components in the chain
• break into separate lines if the line length exceeds the max line length
• join into one line otherwise

This would also allow to configure only s specific part of the repo differently with file overrides. For example, test files tend differ in coding style from other files as mentioned before. If I can allow long chains in tests, that would already be a huge win.

[vytautas-pranskunas-]

Any progress on this? And do not get thi swhy 3? Why cannot be just config propertybecause my team want to have all shained methods on separate line.

[jonathas]

Please guys, this is much needed!

[thorn0]

The problem here is really similar to the one from #6033, isn't it? So I think changing the current heuristic in the spirit of #6033 (like also mentioned in this comment) should do the trick. A bit more sophisticated option: use the >= 3 rule if there are function literals in arguments, switch to the > 3 rule otherwise. What do you all think about this?

[lselbeck]

Pretty please, prettier please!

[MarvinJWendt]

Is there any status update on this? It's the most upvoted issue on the repo here. I am wondering if this pull is unofficially declined now?

[farshed]

Any chances this'll ever get merged? Its been 16 months already 🙄

[lydell]

@farshed We’re currently exploring #6685

[thorn0]

Closing this since #6685 has been merged.

[syabro]

@thorn0 it will be released with 2.0?

[rvion]

Previous discussions seemd to imply that pre-existing line breaks should have been preserved.
That appears not to be the case.

IMO: it's now as annoying as before for a different set of use-cases, notably fluent APIs.

As a library author with dozen builder types libs with fluent apis, hundreds of files are now beeing reformated in a non consistent / weird and not readable way.

I now need to go and edit all those files to add comments in the middle of each chain to ensure lines are broken correctly.

Is there a way to keep using the previous heuristic ?

here is an example I'm editing right now :

domain.concept('Page').val('title', 'string').vals('widgets', 'Widget')
domain
    .concept('Widget')
    .val('title', 'string')
    .val('color', 'Color')
    .val('foo', 'Foo')
    .val('bar', 'Bar')
domain.concept('Widget').val('title', 'string').val('color', 'Color')

here is what it used to be

domain
    .concept('Page')
    .val('title', 'string')
    .vals('widgets', 'Widget')
domain
    .concept('Widget')
    .val('title', 'string')
    .val('color', 'Color')
    .val('foo', 'Foo')
    .val('bar', 'Bar')
domain // <-- I now manually add a comment here
    .concept('Widget')
    .val('title', 'string')
    .val('color', 'Color')

some people warned about this before with upvotes.

• Don’t break chains if they fit on one line & they were written that way #4765 (comment)
• Don’t break chains if they fit on one line & they were written that way #4765 (comment)

example mentioned above:

me
  .grabCakeIngredients()
  .prepareCake()
  .eat()

vs

me.grabCakeIngredients().prepareCake().eat()

[rvion]

@hwalinga @RikJansen81 and others => discussion is happening here: #7884 (comment)

your feedback there would be very much appreciated :)