Consistency in lists/objects

[Augustin82]

Thanks for your work, I'm VERY interested in something that deals with formatting in my place =)

One thing that struck me upon trying it out tonight, though, was that it does not handle "siblings" consistently. That is, given a list of, say, { id, name } objects; if one of them has a name large enough to reach the line limit, then this object, and this object alone, will be printed over 4 lines, while its siblings will be printed on 1 line. The "exploded" one will have a trailing comma, too, while obviously the one-liners will not.

Original test.js:

const myList = [
  {
    id: 0,
    name: 'Short',
  },
  {
    id: 1,
    name: 'Short Too',
  },
  {
    id: 3,
    name: 'Too Long For Your Line Width',
  },
];

Result ($ prettier --bracket-spacing --trailing-comma --print-width 40 test.js):

const myList = [
  { id: 0, name: "Short" },
  { id: 1, name: "Short Too" },
  {
    id: 3,
    name: "Too Long For Your Line Width",
  },
];

I think that in this case, the original is the better code: it's consistent, easily readable and parsable, at the only cost of taking a lot of vertical space (scrolling is cheap, though).

EDIT: removed JSX example and reworded the issue as per @vramana's comment.

[bnjmnt4n]

That's sort of the point of the formatter. It will fit code where it can. The printer will forcibly break a lot of constructs because that's how they look bests, but objects are going to collapse like that.

There are many other times where a short objects is what you want, and this is the kind of styling that I think we need to be less opinionated about. If you add more it's not more work to break it vertically when needed because the formatter will do it for you.

from #43 (comment)

[Augustin82]

Actually, I think that we never need a short object, while we sometimes need a long one. Consistency should lead us to more often than not use the long version.

Unless lower number of lines is an actual, useful thing to aim for?

[vramana]

@Augustin82 Can you write in your description what in the input and expected output in the issue description? Is this similar to #88?

JSX one attribute per line has been requested elsewhere. May be you can remove that from the description to keep it succinct.

[Augustin82]

Thanks for your time, @vramana , I've updated the issue.

It is somewhat similar to #88, I guess, with this one maybe being more general and regarding general philosophy more than an extreme (albeit real) example.

[nperichSYKES]

I love love love this project, and so far object handling is about the only thing that's felt a bit awkward to me. But I'm not sure what the solution is since sometimes I want the object compressed onto a line, but sometimes I don't. It depends on context.

To add to the simple inconsistency noted in the OP, I personally like using "config"-y objects when appropriate. In this case it's nice to use the "long" form, stretched out over multiple lines since it's primary use over the duration of the project is as a self-documenting reference.

{
  field1: {
    validate: f1Validate,
    name: 'name'
  },
  field 2: {
    ...
  }
}

But in most cases I probably want the object compressed to a line.

submit({ value, user, id })

¯_(ツ)_/¯

[vjeux]

This is an issue for React Native StyleSheet.create where the output is not ideal ( https://github.com/facebook/react-native/blob/master/Examples/Movies/MovieCell.js#L71 ):

var styles = StyleSheet.create({
  textContainer: {
    flex: 1,
  },
  movieTitle: {
    flex: 1,
    fontSize: 16,
    fontWeight: '500',
    marginBottom: 2,
  },
  movieYear: {
    color: '#999999',
    fontSize: 12,
  },
  row: {
    alignItems: 'center',
    backgroundColor: 'white',
    flexDirection: 'row',
    padding: 5,
  },
  cellImage: {
    backgroundColor: '#dddddd',
    height: 93,
    marginRight: 10,
    width: 60,
  },
  cellBorder: {
    backgroundColor: 'rgba(0, 0, 0, 0.1)',
    height: StyleSheet.hairlineWidth,
    marginLeft: 4,
  },
});

outputs

var styles = StyleSheet.create({
  textContainer: { flex: 1 },
  movieTitle: { flex: 1, fontSize: 16, fontWeight: "500", marginBottom: 2 },
  movieYear: { color: "#999999", fontSize: 12 },
  row: {
    alignItems: "center",
    backgroundColor: "white",
    flexDirection: "row",
    padding: 5
  },
  cellImage: {
    backgroundColor: "#dddddd",
    height: 93,
    marginRight: 10,
    width: 60
  },
  cellBorder: {
    backgroundColor: "rgba(0, 0, 0, 0.1)",
    height: StyleSheet.hairlineWidth,
    marginLeft: 4
  }
});

[ravicious]

As for the "sometimes I want to use long form, sometimes I use short form": elm-format is very good at that. If you have a record on a single line, elm-format will keep it on a single line. However, once it's on multiple lines or has mixed formatting (some fields are on single line, some are on separate lines), elm-format will split all fields into separate lines.

This is great, as it let's me control when each style is used. Of course one could argue that formatting like that is not consistent, but I think being able to control these two cases (short and long form objects) is important, as @nperichSYKES argued.

Single-line record after formatting:

Mixed-line record before formatting:

Mixed-line record after formatting:

That being said, I don't know if it'd be possible with prettier, since the readme says it disregards the original formatting:

In technical terms: prettier parses your JavaScript into an AST and pretty-prints the AST, completely ignoring any of the original formatting. Say hello to completely consistent syntax!

[vjeux]

@ravicious: prettier reads the original source code to maintain empty lines, so we can technically implement what you describe.

Thanks a lot for the input!

[vjeux]

@ravicious #495 is an implementation of what you suggested

[Augustin82]

@vjeux
Thank you very much for putting this in, it's great!

Shouldn't the same reasoning apply to arrays as well?

Original:

export const appLocales = [
  'en',
  'cn',
  'es',
];

Changed to:

export const appLocales = ['en', 'cn', 'es'];

Expected it to stay the same.

I strongly believe that "pretty" output means "output that can communicate the most information in the most consistent way with as little effort as possible". For me, being able to know that there are three locales in a split second without having to parse a list and make out if the commas are in, or out, of the quotes, is invaluable, and removing that is making the code less readable.

Thanks for your time and great work (on this and other stuff ^^).

[pablen]

I just wanted to chime in (maybe a bit late) to second @ravicious comment. I think the way elm-format handles the single/multi line format depending on original format is fantastic too and I would really love if prettier could work that way.

I think many times the desired format has more to do with "local consistency" (or specific use cases readability) than with global rules that apply everywhere throughout a codebase.

Anyways, awesome project! really grateful for your work!

[gustavopch]

Why aren't arrays following the same reasoning applied to objects?

[scottmas]

Can this be re-opened per gustavopch's comment?

[devuxer]

Really interesting discussion here. I agree that local consistency is often the key to readability, but that sure seems like a difficult thing for an algorithm to enforce (what is "local"?).

This may be a terrible or infeasible idea, but I kind of find myself wishing the Prettier would allow me to "hint" how I want an object or list to be formatted/reformatted.

What I mean by this is, let's say I current have this:

const objectWithThreeProperties = { foo: "foo", bar: "bar", baz: "baz" };

Now I go in and add a line break:

const objectWithThreeProperties = {
foo: "foo", bar: "bar", baz: "baz" };

Now, if I format this, Prettier should say, "Oh, he wants to split this", and spit out:

const objectWithThreeProperties = {
    foo: "foo",
    bar: "bar",
    baz: "baz"
};

If I change my mind, I just do:

const objectWithThreeProperties = { foo: "foo",
    bar: "bar",
    baz: "baz"
};

And when I format this, Prettier should say, "Oh, he wants to join this", and spit out:

const objectWithThreeProperties = { foo: "foo", bar: "bar", baz: "baz" };

So, basically, it compares the current formatted (assuming that is known) with the edited original and realizes that the edited original has a different line-break intent than the currently formatted, then it tries to make the new formatted version match the intent.

[lydell]

@devuxer Prettier already takes hints about object literals. If there’s a newline anywhere inside it, it will stay expanded. If not, it will stay on a single line as long as it doesn’t get too long. So you can add a random newline somewhere to force Prettier to expand an object literal, and you can manually join it all up on a single line (so not exactly like your suggestion, but doable) to attempt to make it single-line.

https://prettier.io/docs/en/rationale.html#multi-line-objecs

[devuxer]

@lydell, Yes, so really my suggestions boils down to making it easier to join it back up again.

[tobia]

I was wondering, would it be possible and/or would it make sense to align the values of object literals using spaces, when the object literal is multiline but the values are not?

For example this:

const theme = {
  container:                'react-autosuggest__container',
  containerOpen:            'react-autosuggest__container--open',
  input:                    'react-autosuggest__input',
  inputOpen:                'react-autosuggest__input--open',
  inputFocused:             'react-autosuggest__input--focused',
  suggestionsContainer:     'react-autosuggest__suggestions-container',
  suggestionsContainerOpen: 'react-autosuggest__suggestions-container--open',
  suggestionsList:          'react-autosuggest__suggestions-list',
  suggestion:               'react-autosuggest__suggestion',
  suggestionFirst:          'react-autosuggest__suggestion--first',
  suggestionHighlighted:    'react-autosuggest__suggestion--highlighted',
  sectionContainer:         'react-autosuggest__section-container',
  sectionContainerFirst:    'react-autosuggest__section-container--first',
  sectionTitle:             'react-autosuggest__section-title',
};

is arguably easier to read than this:

const theme = {
  container: 'react-autosuggest__container',
  containerOpen: 'react-autosuggest__container--open',
  input: 'react-autosuggest__input',
  inputOpen: 'react-autosuggest__input--open',
  inputFocused: 'react-autosuggest__input--focused',
  suggestionsContainer: 'react-autosuggest__suggestions-container',
  suggestionsContainerOpen: 'react-autosuggest__suggestions-container--open',
  suggestionsList: 'react-autosuggest__suggestions-list',
  suggestion: 'react-autosuggest__suggestion',
  suggestionFirst: 'react-autosuggest__suggestion--first',
  suggestionHighlighted: 'react-autosuggest__suggestion--highlighted',
  sectionContainer: 'react-autosuggest__section-container',
  sectionContainerFirst: 'react-autosuggest__section-container--first',
  sectionTitle: 'react-autosuggest__section-title',
};

[duailibe]

@tobia Your subject has almost nothing related with the issue's original subject. We try our best to keep discussions focused.

Also, this was already discussed, we won't include that inside Prettier, see #1622.