prettier: Allow inline method chaining if line length does not exceed `printWidth`

There are many popular patterns chaining method calls using enzyme. Please find below an example when Prettier creates new lines for each chained method of wrapper. Would be really nice to have an option to inline at least the first call independent on the heuristics mentioned above.

Prettier 1.7.4 Playground link

Input:

it('gets triggered by mouseenter', () => {
  const wrapper = shallow(<CalendarDay />);
  wrapper.dive().find(Button).prop();
});

Output:

it("gets triggered by mouseenter", () => {
  const wrapper = shallow(<CalendarDay />);
  wrapper
    .dive()
    .find(Button)
    .prop();
});

fixed by #6685, not yet released

@j-f1 #4765 seems great - it’s saying “instead of always expanding to multiple lines, prefer one-line when it was originally one-line, where possible”.

However, what we want much more than that is “instead of always collapsing to one line when it fits, prefer multi-line when it was originally multi-line”.

Both seem possible (ie, they don’t conflict with each other). Is there any chance of also getting the second behavior?

@sharmilajesupaul @vjeux The destructuring thing is already tracked in #2550

Do you have examples where Promise.resolve or Promise.all do not behave the way you want?

Would it be possible to make the list of exceptions like _ and $ configurable? That way we can add fejax and other cases to that list.

I’d love to know how many times you hit this in practice. Adding configuration for small things like this has a really big cost. I believe that printing a slightly less optimal output for those is a good trade-off.

We want to restrict the format of arguments to either all be in one line or each argument on its own line by itself, but not mixed

This is indeed how prettier is supposed to work. Looks like you found a bug where there’s a trailing comment in the first line: https://prettier.io/playground/#N4Igxg9gdgLgprEAuEAzOArAhgDwHQDmcMAUgMoDyAcgBQDkA9KhBAwJYDOA+nADZsE2AI15w6ASjwwAFghoAdKAAIlYWWADWAQSgATAKJQsIuAGEsvXkKyaANEoYNFKmsCUcYWGAFcOAFTgcGHsAJzgOAAdoDjgAoKUAX3ElAF4APiVgZxUHBiUAEU5jUSUYCCUIsIA3BBglMIi4LzhdVQteDjxslV0ikyoAVQANMm8QmoBPGnEAbmyExVnFEFsQCAiYNmjkUCwPZFQLGNWhEJsNYjIImzYoAmQYEO84Vd0IMAOjl5AMDhwAITOmkuWAAtnAADK3OCfDrfCDeGARREAJlhxxA1xCMRCyBA1iEE140BWmJCtxgAHU2LoZMgABwABlWMVBbAeT2+HFuBFEAEVvBB4Ojvp4hNTadJkCjVo8sGx+HdTBBQaCsHj2qTfLFjBwRQkEkA

We’ve also discussed maybe having a way to turn off Prettier’s behavior of collapsing multiline statements. This isn’t as big of an issue, but in general multiline arguments, object properties, jsx props, and named imports are intentional choices made by a developer and they aren’t necessarily prettier one way or the other lol.

We do it for objects, but because objects can be used to mean so many different things that it’s hard to figure out why. For example React.createClass({ ... }) should never be inlined, even if the content fits in one line: here object is being used as a class.

For the others, I actually believe that this is a bad idea. The goal of prettier, and a reason why I believe it’s so successful, is that you can as a developer --not-- spend time formatting your code. You just write your code in a single ugly line, save and it’s going to look good. It affects the way you --write-- code.

With eslint, you write your code the same way as before and the compiler yells at you if you made a mistake. It is a safety guarantee but you still have to spend all the time formatting your code.

In order to achieve this result, letting people have the choice between different ways of formatting something is actually harmful. Now you won’t be sure if the input will be intentional or not. We’re doing it for objects and it’s a constant source of bug reports and people asking to get rid of it.

What would be great is if you could provide concrete examples where prettier didn’t do the thing you expected and we can figure out if there’s a way to make prettier print it best by default. What I found is that in practice, there’s very often one way that people are generally happy with: maybe not the way they would actually write it in practice, but one that doesn’t trigger a “omg, never in my codebase” reaction.

Note that I’m not against adding an option for it, but I’d love to know if we can find a way to fix it without having to use that giant hammer.

I sent #3112 to fix the case with window.FooClient.setVars

I think that, when the first invocation can fit on one line, this would work well.

However I think the real conceptual difference is that in window.FooClient.setVars( or [].map(, the invocation is conceptually a separate thing from the receiver - but in Promise.resolve(), and $.ajax(, the invocation and the receiver are part of the same conceptual thing and thus shouldn’t be split apart across lines.

I’m not sure there’s a single good heuristic around this that can be programmatically applied while meeting our requirements.

@vjeux would it be possible to have an option to avoid altering method chains, while still styling their arguments and bodies?

You’re right, those diffs do look worse and the single heuristic: chain everything, until you hit the end of the line, is definitely not going to work. The leading and inline comments get completely displaced too.

What if Prettier only changed the first property invocation to be inline?

i.e. the first diff would look like:

return action$.ofType(ActionTypes.SEARCHED_USERS)
  .map(action => action.payload.query)
  .filter(q => !!q)
  .switchMap(q =>
    Observable.timer(800) // debounce
      .takeUntil(action$.ofType(ActionTypes.CLEARED_SEARCH_RESULTS))
      .mergeMap(() =>
        Observable.merge(
          Observable.of(replace(`?q=${q}`)),
          ajax
            .getJSON(`https://api.github.com/search/users?q=${q}`)
            .map(res => res.items)
            .map(receiveUsers)
        )
      )
  );

and it would mirror the behavior of this Prettier output:

  window.FooClient
    .setVars({
      locale: getFooLocale({ page }),
      authorizationToken: data.token,
    })
    .initVerify('foo_container');

@ljharb do you agree?

How does #4765 look?

For the latest playground bug, turns out it was fixed a week ago, before you reported it 😃 https://github.com/prettier/prettier/pull/3079

@vjeux To answer your first question:

By the way, are those the only 3 issues you are facing at Airbnb or are there a bunch more?

There are a few more issues we’ve come across internally and we’ve been trying to address some of them using eslint rules.

• We want to restrict the format of arguments to either all be in one line or each argument on its own line by itself, but not mixed. Here’s a playground link illustrating this. Looks like methods that start with a capital letter or with $ and _ are treated differently.

Would it be possible to make the list of exceptions like _ and $ configurable? That way we can add fejax and other cases to that list.

• We’ve also discussed maybe having a way to turn off Prettier’s behavior of collapsing multiline statements. This isn’t as big of an issue, but in general multiline arguments, object properties, jsx props, and named imports are intentional choices made by a developer and they aren’t necessarily prettier one way or the other lol.

as for this question:

Also, how often do those 3 happen in practice on your codebase?

This is going to take some digging on my part so I’ll get back to you with a more accurate answer soon. But afaik, this particular issue with method chaining is only affecting fejax, Promise.resolve or Promise.all and window.foo (which you’ve already addressed).

For reference, here’s the code that contains the heuristic: https://github.com/prettier/prettier/blob/6ccf5c0246970557426220008d2f950e67101181/src/printer.js#L3781-L3800

The entire function printMemberChain has a lot of comments that explain how it works if you are interested 😃