prettier: Break on multiple chained calls

Could we manage chains like we manage object literals, in that if they have a break within them they get broken e.g.

this examples stays formatted as it it is

d3.select("body").selectAll("p").data([1, 2]).enter().style("color", "white");

this example:

d3.select("body").selectAll("p")
  .data([1, 2])
  .enter()
  .style("color", "white");

gets formatted as this

d3
  .select("body")
  .selectAll("p")
  .data([1, 2])
  .enter()
  .style("color", "white");

Prettier will generally try to fit content in one line if possible.

Yes, and it mostly turns out better. Not always though. This is a less abstract possible solution to #1099 maybe?

consider:

I do think the second one is prettier, although I can see the argument it’s “verbose”.

The real gain in that case is on diffs, though (same reason we do trailing commas, it’s not necessarily prettier).

 point()
   .x(4)
-  .y(3)
+  .y(4)
   .z(6)
   .plot();

- point().x(4).y(3).z(6).plot();
+ point().x(4).y(4).z(6).plot();

---

 point()
   .x(4)
   .y(3)
+  .foo()
   .z(6)
   .plot();

- point().x(4).y(3).z(6).plot();
+ point().x(4).y(3).foo().z(6).plot();

---

 point()
   .x(4)
-  .y(3)
+  .y(4)
+  .foo()
   .z(6)
   .plot();

- point().x(4).y(3).z(6).plot();
+ point().x(4).y(4).foo().z(6).plot();

This not being configurable means our team can’t use prettier.

const { error } = Joi.validate(promotion, Joi.object().keys({
		companyId: Joi.number().integer().required(),
		newPlanId: Joi.number().integer().required(),
		oldPlanId: Joi.number().integer(),
		oldPlanPrice: Joi.number(),
		start: Joi.date().iso().required(),
		end: Joi.date().iso().min(Joi.ref('start')).required()
	}));

turns to

const { error } = Joi.validate(
		promotion,
		Joi.object().keys({
			companyId: Joi.number()
				.integer()
				.required(),
			newPlanId: Joi.number()
				.integer()
				.required(),
			oldPlanId: Joi.number().integer(),
			oldPlanPrice: Joi.number(),
			start: Joi.date()
				.iso()
				.required(),
			end: Joi.date()
				.iso()
				.min(Joi.ref('start'))
				.required()
		})
	);

Using prettier-ignore all over our controllers is unacceptable, workarounds are unacceptable, not using Joi is unacceptable.

EDIT: after having thought about it for a while, it seems there is no way prettier can consistently format chained methods so that the result would be acceptable in all scenarios. I believe prettier should not touch chained methods at all.

Definitely agree with @dburrows’s comment above. It’s following an already accepted pattern (object literals) and adding it to function chaining when the user opts in.

While this issue is semi-back-from-the-dead, I’d like to quickly argue again for respecting the user’s input as is done with object literals. Unfortunately, I don’t think a single “number of chained calls before wrapping” option is sufficient.

For example, I may want to see the following in a single line

moment.utc(userInput).hour(0).minute(0).second(0)

versus an unnecessarily long

moment
    .utc(userInput)
    .hour(0)
    .minute(0)
    .second(0)

but choose to split

fetchUser(id)
    .then(fetchAccountForUser)
    .catch(handleFetchError)

across multiple lines for readability compared to

fetchUser(id).then(fetchAccountForUser).catch(handleFetchError)

which I feel obscures the catch. These types of cases would not be solved with a single configurable number option.

(Per @j-f1 above, I know that the initial moment example could be cleaned up with a helper zeroTime function, but it doesn’t solve every instance this comes up in regular programming where a helper function is overkill.)

@vjeux thoughts on the above suggestion?

This particular issue is probably the main one stopping my team from adopting prettier.

Sorry for the noise but I’m hoping it might call attention to this issue once more; I am very much in favor of @dburrows and @apazzolini suggestion of checking whether there were any newlines manually added by the user, just like with object literals.

To once more illustrate the reasoning:

Ugly

Pretty

I still think dburrows’ earlier suggestion would have been the way to go.

If the user puts a line break in a chain of function calls, respect it. Otherwise, let prettier do its thing.

It’s the same pattern prettier uses for objects, which works really nicely in practice.

I would go a step further and argue that even very short chained calls should go into separate lines, so that

point().x(4).y(3).z(6).plot();

would be formatted as

point()
    .x(4)
    .y(3)
    .z(6)
    .plot();

In my opinion each call is like a separate statement. And multiple normal statements are always written in separate lines, right?

For example if the same was written with a class it should be formatted

const p = point();
p.x(4);
p.y(3);
p.z(6);
p.plot();

and not

const p = point();p.x(4);p.y(3);p.z(6);p.plot();

An ideal solution (for me, at least) would simply be the ability to turn off the “generally try to fit content in one line if possible” rule altogether.

If I’ve broken code down across multiple lines (even though it could be placed on one line), I’ve generally done so for explicit reasons of readability and clarity, and don’t want to to be changed.

@demisx As far as I know, this option is non-configurable.

The suggested behavior of this feature has been:

1. Wrap after hitting the line length limit
2. Wrap after 3 chained methods
3. Wrap after a configurable number of chained methods
4. Wrap when one of two conditions is met:
   • The line length limit is reached
   • The user opts into it by manually inserting a newline, like object literals

The initial implementation was (1). The current implementation is (2). (3) is unlikely to happen since prettier tries to avoid configuration. The consensus is against (4) due to wanting to minimize user input influence in prettier’s output.

I think the current implementation (2) makes sense in the majority of cases, but I find myself wanting (4) often enough that I no longer use prettier for JS.

Edit: grammar

Prettier will generally try to fit content in one line if possible.

I don’t think a simple solution like “break when 3 or more chains” can be reasonably applied, consider:

point().x(4).y(3).z(6).plot();

point()
  .x(4)
  .y(3)
  .z(6)
  .plot();

Which is prettier?

@vjeux I appreciate your work on Prettier, and I’m sure that you’ve thought about how to handle specific cases a lot more than I have. I understand the desire to discard all user input and consistently apply formatting, and definitely agree that’s the main value prop of Prettier.

With that in mind, would you please explain your rationale for respecting a user’s choice of keeping an object literal on one line if it was inputted that way and fits the line length limit vs not doing the exact same thing on chained method calls?

The advocates in this thread are not suggesting introducing new behavior to Prettier - we’re simply trying to mirror already implemented patterns into another situation it applies.

I haven’t seen a single person argue that the current behavior works well and that it’s the gold standard for chained calls.

The only people that are going to look for this issue are people that are not satisfied with the status quo. There may be (and likely is) a silent majority which is happy with the current behavior.

Seems like eventually someone should fork prettier to loosen up some of it’s more opinionated decisions, and then users can decide which one they like better.

I’m very surprised that people didn’t fork prettier more often. I would highly encourage you doing that 😃

The value proposition of prettier to me is the ability to write code badly formatted and when you press save, there’s a canonical version of the output. This means that the developer can stop --thinking-- about formatting the code.

I believe the user sometimes knows best what’s prettier, and it seems to be the case here. Prettier wants consistancy ; which one of the following really feels more consistant?

The fact that similar-looking expressions output pretty different formatting is not ideal and indeed not consistent. But in my opinion it’s better than having to give developers a choice.

If we preserve the way developers print their member expressions, this means that now based on where you are in the codebase they can be printed in a different way, based on the mood of the author. It also invites back all the nitpick discussions that prettier is hoping to solve and I’m sure that we’ll see people writing linters to enforce certain patterns. Especially in the case of member expressions which are --everywhere–.

For context, member expressions is the most complex piece of code of prettier. As seen in this issue, there are still issues with it and it doesn’t get everything right, but in the grand scheme of things it works really well. All the real code examples I’ve seen where it doesn’t output ideal code, the output is a more verbose than one would hope but it doesn’t look completely broken.

I’m happy with the current state of member expressions in prettier. If you are not, please try to come up with specific fixes for scenarios where it doesn’t print code the way you like and send pull requests. There are a ton of existing snapshot tests so you’ll see what other pieces of code you break in the process. Or feel free to fork the project and enable following the existing code, it should be easy to build this, but I don’t want to add it to the main project as I believe it’s the wrong trade-off.

I don’t like how it currently forces line breaks into function chains. Here’s an example:

return await db.branch(
  db.table('users').filter({ email }).count(),
  db.table('users').insert({ email }),
  db.table('users').filter({ email }),
)

// becomes...

return await db.branch(
  db
    .table('users')
    .filter({ email })
    .count(),
  db.table('users').insert({ email }),
  db.table('users').filter({ email }),
)

I think this looks pretty awful! I wish it didn’t wrap chained calls unless they’d extend past print width.

Please let make dburrows’ suggestion be the norm. That makes prettier “ignore” chained calls (as requested by a large groupe of people), but keep things consistent as no mixing solutions.

Any number of chained calls to be a condition on linebreaking cannot be a good solution as it depends enormously on what those calls are. For a letter or a whole function behavior cannot be forced to be the same.

To my eyes, the only good solutions are either to ignore formating chained calls or format following what the dev want for the specific situation, as dburrows suggested.

For what it’s worth, not having this feature was a blocking issue for our team. I love the new behavior.

Respecting what is there would also work for us, or a configurable number of chains before it breaks would also be fine, but going back I don’t think is an option.

@SimenB the problem here is more about tooling.

All of the tools I use for diffing supports word diffing as well, so maybe that point is moot. But I still think the multi-lined one is easier to read 😄

An ideal solution (for me, at least) would simply be the ability to turn off the “generally try to fit content in one line if possible” rule altogether.

I don’t agree with this. One thing I really like about prettier is that it mostly ignores input formatting in order to make code consistent. But sometimes it is overly eager.

https://prettier.github.io/prettier/#{“content”%3A"import {\n doMapping%2C\n doFilter%2C \n doSecondMapping%2C \n doReduce%2C\n} from ‘someCommonCode’%3B\n\nsomeArray\n .map(doMapping)\n .filter(doFilter)\n .map(doSecondMapping)\n .reduce(doReduce)%3B"%2C"options"%3A{“printWidth”%3A80%2C"tabWidth"%3A2%2C"singleQuote"%3Afalse%2C"trailingComma"%3A"all"%2C"bracketSpacing"%3Atrue%2C"jsxBracketSameLine"%3Afalse%2C"parser"%3A"babylon"%2C"semi"%3Atrue%2C"useTabs"%3Afalse%2C"doc"%3Afalse}}

While I think the first one is fine to one-line, the second one loses readability, IMO. And this is on 80 width, the problem is much worse if you use 100 or 120 as width. Some way of breaking even if it would have fit seems like it might solve some the issues I have.

It might still be that #1099 is better, but I don’t know if @vjeux has gotten the time to work on that.

@shellscape Prettier doesn’t (and has no intent to) read ESLint configuration. For conflicts, we recommend disabling any rules that conflict with Prettier’s style, or simply run ESLint after Prettier (https://github.com/prettier/prettier-eslint).

I believe the user sometimes knows best what’s prettier, and it seems to be the case here. Prettier wants consistancy ; which one of the following really feels more consistant?

• Current Prettier behavior: (3+ chains = break)

const a1 = b1.c1().d1();
const a2 = b2
            .c2()
            .d2()
            .e2();

return fetchApi().then(res => res.data).catch(err => err.stack);

• Suggested behavior: (leave the choice to the user, as long as he doesn’t mix)

const a1 = b1.c1().d1();
const a2 = b2.c2().d2().e2();

return fetchApi()
         .then(res => res.data)
         .catch(err => err.stack);

This issue is a big turn off for my team and me. When users starts using abusively // prettier-ignore, it’s generally a bad sign. And that’s sad because appart from that, I really love this tool 👍

A problem I see with @dburrows’ suggestion is that code previously formatted with prettier will be on one line, so if we make that change the previously formatted code will never change to the multi-line version.

Another issue is that it increases the knowledge users need to have of the inner workings of prettier. Having to do things a certain way to produce a certain output sucks.

I think I’d prefer to have a “3 chained function calls causes a multi-line expansion” rule as originally suggested in the OP.

This would also close #1282.

Any news on this? can’t wait for the feature

This is probably the biggest barrier for my team to adopt prettier as well. I normally love prettier, but the output it produces for lines that aren’t over the line length for chained calls and instead get single lined, break the expectations of myself and those I work with. I got enough pushback on it that it isn’t integrated in our pre-commit hooks anymore.

I’d prefer prettier to not respect the original formatting as much as possible. The best advantages for prettier for me is that code that different people write is as consistent as possible.

Personally I’d much prefer to not allow lines line point().x(4).y(3).z(6).plot();. If you are chaining that many things together, then I think you should put them on multiple lines. I think the ESLint rule newline-per-chained-call is terrific in that regard.

For examples like

Object.keys(props).filter(key => key in own === false).reduce((a, key) => {
    a[key] = props[key];

    return a;
}, {}),

I completely agree that this is much better written as

Object.keys(props)
    .filter(key => key in own === false)
    .reduce((a, key) => {
        a[key] = props[key];

        return a;
    }, {});

I’d love a solution that didn’t require me to manually break things if possible and prettier to do this automatically. That way prettier would never output the former and always the latter, rather than leaving this up to the developer and thus having inconsistencies in the codebase.

Is the problem just with prettier’s breaking strategy that it can’t break each chained function just because one of them broke? That said, maybe there are too many edge cases to do that well 😞

I’ve read through 5 different issues on this topic, as the behavior was throwing me off as well. What I can’t figure out is why Prettier doesn’t want to seem to listen to the newline-per-chained-call ESLint rule, which clearly defines the behavior here. Additionally, Prettier is in direct conflict with that rule if it’s set to something outside of the Prettier default.

If modifications are being done to the behavior of Prettier in this regard, I would like to suggest that obeying the settings in ESLint for this particular behavior should be a priority.

I’m planning to work on the idea I proposed this evening and perhaps open a PR.

(vjeux) code badly formatted and when you press save, there’s a canonical version of the output

For sure! What you’ve done is amazing, we’re/I’m just quibbling about minor details.

(vjeux) I’m very surprised that people didn’t fork prettier more often. I would highly encourage you doing that 😃

😃 yeah…

Thanks for chiming in! Glad to hear you’re open to PRs that tweak the behavior. That is primarily what I was curious about.

For me personally, and I hope?/assume many others on this thread, a few tweaks to method call wrapping to handle the super-simple/no-param cases like foo().bar().zaz() would hit the “good enough”. It’d be fun to hack on that…will see if I can ever get around to it. I would also love for someone else to beat me to it (of course 😃).

Given this chatter is all happening on a closed issue, maybe it’s worth opening a new issue of “Tweak chained calls” (with a “open to PRs”-type label) and people could start submitting cases, just as comments, for what they’d like to see, for whenever eventually someone wants to tackle it?

How about keeping the chain on one line if (it has < 3 members || it is currently on one line) && it’ll fit on one line? This way, people can chose to keep one-liners on one line, but aren’t able to excessively break member expressions?

Please make this option configurable. 🙏 We can’t use prettier because of this as well.

FWIW, probably too late, and everyone has an opinion, but this changed some code like:

assert.equal(this.$().text().trim(), '1000');

To be:

assert.equal(
  this.$()
    .text()
    .trim(),
  '1000'
   );

(I may not have copy/pasted the indentation 100% right from our code review tool.)

Which seems objectively much worse; a 1-liner assertion that was very readable IMO (e.g. not the complex foo.a().b().zaz().plot().whatever() examples in the above discussion) got turned into a 6 line method that, again I would assert objectively but technically IMO, is actually harder to read, because it’s needlessly strung out over multiple lines.

FWIW I love the prettier ethos/goal, just disagree on this point solution.

3 seems arbitrarily low to wrap on, regardless of line length. Something like ~6-7 seems like a more reasonable number for “yeah, this chaining is getting complex and needs broken up”.

If anything, let this be configurable? 😃 (I originally thought prettier was zero-config, but I noticed there is a config file, so had to ask. Thanks!)

Works for me. The 3 chained call rule forces more consistency, which I like. Thanks for the PR.