Inconsistent formating for the same code

[kennydee]

Hi everyone,

I've been playing with prettier (which is just amazing) on some of our project, and notice a behaviour that i found really strange, where the same code, can lead to four different formating :

Original code :

/* The 4 components are the same */

const Component1 = ({ props }) => (
  <Text>Test</Text>
);

const Component2 = ({
  props
}) => (
  <Text>Test</Text>
);

const Component3 = ({ props }) => (
  <Text>
  Test
  </Text>
);

const Component4 = ({
  props 
}) => (
  <Text>
  Test
  </Text>
);

Turns after prettier to :

const Component1 = ({ props }) => <Text>Test</Text>;

const Component2 = (
  {
    props,
  },
) => <Text>Test</Text>;

const Component3 = ({ props }) => (
  <Text>
    Test
  </Text>
);

const Component4 = (
  {
    props,
  },
) => (
  <Text>
    Test
  </Text>
);

/* Why Component2, Component3, Component4 output from prettier are not the Component1 one ? */

See on Prettier website

So my final question, is why those 4 components that are absolutely the same, are turning into something different depending on how you originaly use Carriage Return ? Is this something you want ? or a bug ? because when i read in the Readme

It removes all original styling and ensures that all outputted JavaScript conforms to a consistent style.

It seems that it's not consistent in this case :)

Hope this is not something already discussed or an already existing bug (i search without founding any), if so, please apologize :)

Thanks for helping

Kenny

[vjeux]

Thanks Kenny, that's an excellent issue!

For 1 and 2, we had to adapt the rule a bit for objects as we couldn't find a good heuristic when objects should be expanded or inlined. So if there's a \n in an object we'll keep it expanded. But, this should only apply for objects and not for object destructuring. We need to fix it.

For 3 and 4, @rattrayalex do you know why if there's a newline the text is not inlined in this case?

[vjeux]

Fix for the first part is https://github.com/prettier/prettier/pull/981

[vjeux]

Fix for the second part is https://github.com/prettier/prettier/pull/982

[kennydee]

Thanks @vjeux : Awesome for the quick fix :) Will look at the PR.

Is this other example correspond to the same problem ?

const e1 = AppNavigator.router.getStateForAction({ type: NAVIGATE, routeName: SCENE_GAME }, state);
const e2 = AppNavigator.router.getStateForAction({
  type: NAVIGATE,
  routeName: SCENE_GAME
}, state);

const e1 = AppNavigator.router.getStateForAction({ type: NAVIGATE, routeName: SCENE_GAME }, state);
const e2 = AppNavigator.router.getStateForAction(
  {
    type: NAVIGATE,
    routeName: SCENE_GAME
  },
  state
);

See on prettier website

Thanks

[vjeux]

For your latest example, this is intended. Because there's a \n in the original one, we expand the object (and properly reformat it).

This is far from ideal but before doing that, there was so many issues opened when people wanted either one or the other and we couldn't really find a good way to figure it out.

[kennydee]

Ok 👌 Thanks again for the fix, you do an incredible job on prettier !

[rattrayalex]

(related to comments on https://github.com/prettier/prettier/pull/982)

In general, JSX is like objects in that we do give developers more control over how their code looks.

JSX – both as a syntax, and as what it represents – is much more "subjective" than normal javascript code. The developer gets a lot more control over the look of it in prettier than normal JS.

jlongster wrote something affirming that direction for JS in a thread a while ago, I think. Might be worth digging up and clarifying on the Readme what cases (like objects and JSX) developers have some control over.

@kennydee what are your thoughts?

[kennydee]

Thanks for the explanation @rattrayalex My thought are a bit different. For me, the best thing with prettier is to have a consistent codebase on a project, no matter how many devs you got, and how they deal with indentation and code style. (i've got projects with lots of devs on it, already using eslint a lot, but was hoping that prettier will totally fix that. it does for a good part, but not totally)

So i was thinking that the normal behaviour from prettier was to always have the same output for a same code even if the code is not indented the same way at first.

I understand your concern, i also understand that people may don't like everything on the same line when the max_line config allow it, but i found it very strange to have multiple output on a same project depending how the dev has originally written the code :-)

[vjeux]

At the beginning prettier started as pure formatter without looking at the original source. The first thing that broke down is empty lines. We haven't found a good heuristic to put them in sensible places. We could get rid of them but they do add meaning to your code.

What is interesting is that if you do not give that control, people will go out of their way to get it back. Reason formatter doesn't respect empty lines but add one before a comment. So people start putting useless comments just to get those empty lines.

We've tried to wait as much as possible before adding control. A lot of the issues that were coming next were about objects that were not properly expanded or shortened.

The problem with a project like prettier is that it reformats your entire codebase so if there is one thing that isn't printed in an acceptable way, you are going to not use it at all. Object printing was such a thing for many people.

I think that in order for such a project to have a glimmer of a hope of succeeding we need to make trade-offs. Having objects and jsx respect the way it was inputted seems like a good one. It's not fully consistent but at the same time, it doesn't really hurt readability or understanding nor has the chance of leading to bugs.

[rattrayalex]

Probably worth writing up some of this, ideally with examples, in the readme somewhere... @kennydee 's question is a good one that a lot of users are likely to ask, and it's worth explaining the nature of the tradeoffs at hand. Plus, it may help more people come up with better heuristics on how to break objects...

Though, of course, this issue has a pretty clear title and ask (thanks Kenny!) so hopefully curious folks can find it on google.

[vjeux]

The first one has been fixed by #981. For the second one it's unclear if it's a good idea to fix it, so i'm changing the tag to "Needs discussion". Thanks for the report!

[vjeux]

I'm going to close this issue as we're not going to change this behavior. This is unfortunate that the same code can output different things, but the best trade-off we found :(

[vjeux]

As of prettier 1.5, all of them now output the same way:

Input:

const Component1 = ({ props }) => (
  <Text>Test</Text>
);

const Component2 = ({
  props
}) => (
  <Text>Test</Text>
);

const Component3 = ({ props }) => (
  <Text>
  Test
  </Text>
);

const Component4 = ({
  props 
}) => (
  <Text>
  Test
  </Text>
);

Output:

const Component1 = ({ props }) => <Text>Test</Text>;

const Component2 = ({ props }) => <Text>Test</Text>;

const Component3 = ({ props }) => <Text>Test</Text>;

const Component4 = ({ props }) => <Text>Test</Text>;

Thanks for being patient with me :)

[kennydee]

Awesome ! This is a huge win for consistency.

Thanks for keeping us updated and for the incredible work you made ! Everything from the communication (feedback, releases notes, issues, ...) to the technical aspect are pure gold on Prettier :)

[vjeux]

Awww!