Option for ignoring original formatting 100%

[ccorcos]

Edit by @lydell: Prettier currently keeps multiline object literals multiline even if they fit in a single line, and does special things to blank lines: removes some, collapses consecutive blank lines into one and never adds blank lines. This is because the majority of people seem to want this level of control since we haven’t found a super good heuristic for printing those things. Some people prefer a 100% uniform printing, though, that completely ignores original formatting, even if it sometimes results in “uglier” code. We should discuss this.

Original post:

Is it possible to have a strict option that disambiguates whitespace and always prints out the same so long as the AST is the same? I'm running into some weird issues where lines wrap and then don't unwrap when they are under the line-length limit. This would greatly help with some of my merge conflicts right now :)

P.S. I'm using typescript.

Thanks

[vjeux]

You are probably thinking about objects that keep being expanded when there's a \n inside, even if they don't fit.

There are unfortunate cases like this where we can't just blindly print the ast. We try to keep them to a minimum though.

But, I think that the root of your issue is that the workflow for solving merge conflicts is not well oiled out. We should focus on making this better. If you could explain what you are currently doing and what are the merge conflicts and how you try to solve them, that would be super helpful.

[ccorcos]

Exactly. So here's a good example of something that is giving us trouble.

This was the code in branch1.

export const trackSelectTemplate = function(args: { name: string }) {
	analyticsHelpers.track("select_template", args)
}

And this is the same code on master.

export const trackSelectTemplate = function(
	args: {
		name: string
	}
) {
	analyticsHelpers.track("select_template", args)
}

Both have been run through prettier and they're stable. I honestly have no idea how we got into this situation though... It seems like a solid solution is just to unwrap lines that can be unwrapped. What's wrong with that solution?

[lydell]

@ccorcos See https://github.com/prettier/prettier/blob/76efb33e75365aceab508b0504f0e6f5ba2dd540/docs/rationale.md#multi-line-objects and the issues it links to. I'm sorry but this has already been discussed a lot and the best trade-off we've come up with is to keep objects multliline if they have a newline inside.

[ccorcos]

Interesting. Would it be possible to have an option at least? It appears that the discussion has focused mostly on aesthetics but the reason I bring it up is to have an entirely unambiguous formatter. I don't actually care that much about how it looks. I just want one way of doing things.

[lydell]

@ccorcos How would you handle empty lines?

[ccorcos]

I think we could easily come up with some conventions, don't you?

• empty line after imports

import x from "x"

function blah() {}

• empty line around function definitions

const w = 10

function x() {}

function y() {}

• empty line before comments

const x = 10

// this does something
const y = 20
const z = 50

• empty line around class methods

Can you foresee any tricky scenarios?

[lydell]

I think this would be an interesting option so I’m opening up this for discussion for a while.

[SimenB]

I'd also love to see an option to ignore the style of the input. I'd think I'd not use it all the time, but would love to have the option. In the rare case where I find the result less readable, I imagine just reverting it and prettier won't touch it the next time.

[ccorcos]

Exactly. And I think when coding in a large organization, the less ambiguity the better :)

[lydell]

ESLint rule to take inspiration from when it comes to blank lines: http://eslint.org/docs/rules/padding-line-between-statements

[josephfrazier]

This is tangentially related, but I noticed that adding --debug-check to our test suites causes 11 of them to fail the prettier(x) == prettier(prettier(x)) condition, and I'm guessing that it's because input formatting is taken into account. See here for details: #3512 (comment)

[lydell]

prettier(x) == prettier(prettier(x)) should always be true, regardless if we look at the original input or not. Everything else is a bug.

On a related note, I often think about this issue, but have never had the time and energy to work on it yet.

[rattrayalex]

From @jlongster in #3524 (comment), referring to this issue:

@rattrayalex I actually would use the hell out it, but I'm not sure about adding it as an option. Personally I'd like to keep this as zero config as we can get and minimize the amount of differences. While it's not as big of a change as the this issue, it's still a lot bigger change than just single/double quotes, bracket spacing, etc. I don't feel as strongly about it though and if other maintainers decide it should be an option, you can do it. I'd vote against but wouldn't block that decision.

I think that if we're going to collapse objects, we'll need really good heuristics about when we do. Identifying when something is a "config object" and best split out sounds both tough and important. Same with "what's a chuck of code", as @ccorcos mentioned. (hi Chet!)

Perhaps we could have a "temporary, experimental" option in prettier 1.x while we iterate on these heuristics, so we can gather feedback from the community? The goal would be to launch prettier 2.0 with "full input-text ignorance".

Thoughts?

[rattrayalex]

Looking through the linked issues a bit (I see #74, #88, #495)

@vjeux had this idea in #88 (comment):

Another idea: automatically expand all the objects at the top level. This may be weird and feel inconsistent but would solve the problem of RN StyleSheet.create and the brunch config.

In other words, "if any node of a nested object must expand, all nodes must expand", which I think should extend to include arrays as well (eg; the top example in #74). There are likely to be subtleties of course 😄

Thoughts?

Also, @ccorcos are you interested in putting together a PR along the lines of what you described in
#2068 (comment) ? (Assuming there's interest in proceeding along this path, which I think there is)

[lydell]

What about "if three or more items in the object, expand it"?

[rattrayalex]

@lydell what if the three items are all shorthand notation and have short names? eg; runProgram({data, config, onChange}) should probably not expand. I think that might be a useful heuristic in other scenarios though.

I think there are some similarities between objects and JSX; some of our JSX handling re; expansion is fairly sophisticated and might be worth a look. IIRC, the decision to allow user input on objects occurred fairly early in prettier's life, before we played around with various techniques which may be useful here.

(I'll be afk for the next 10 days; otherwise would give it a go).

[azz]

"if three or more and there's at least one non-shorthand property"?

runProgram({data, config, onChange});
runProgram({
  data: null,
  config,
  onChange
});
runProgram({data: null, config: null});

(Probably not a good idea but I typed it up so I may as well post it)

[loynoir]

Can any one explain to me, why this marked as difficulty: hard, like I'm 5 years old? 🤔
Seem like kind of like

featPrettier = x => currentPrettier(minifier(x, every_no_aggressive_options))

[alexander-akait]

@loynoir PR welcome if you think it is easy

[loynoir]

Current my rough workaround

Gulp task

// Gulpfile.ts
import gulp from 'gulp';
import terser from 'gulp-terser';
import prettier from 'gulp-prettier';
// import { argv } from 'yargs'

if (process.env.prefix) {
  process.chdir(process.env.prefix)
}

function terserPrettier() {
  return gulp.src('dist/**/*js')
    .pipe(terser({
      compress: false,
      mangle: false,
      keep_classnames: true,
      keep_fnames: true,
      // // @ts-expect-error
      // keep_number: true,
      output: {
        keep_quoted_props: true,
      }
    }))
    .pipe(prettier())
    .pipe(gulp.dest('dist.min'));
}

terserPrettier.flags = {};

gulp.task('default', terserPrettier)

# diffing AST changes
$ meld a/dist.min b/dist.min

[loynoir]

@alexander-akait
Sorry. Apologize if I offense. 🙇
I just curious what is the bottleneck here...

[uriva]

Hi I'm interested in the following subset of this problem:

prettier allows:

const newState = { ...state };

but also

const newState = { 
  ...state,
};

I think just choosing between these too would go a long way for me. was this discussed? if so what is the status?

thanks

[nullromo]

@uriva Unfortunately the maintainers of Prettier have rejected this idea many times over. They are concerned with having as few options as possible which means they won't add a flag for it, and also with satisfying people who care about all their objects looking the same for some reason or another, which means they won't change this behavior. So while it would be nice if prettier were "deterministic" or "reversible," it's not going to happen, no matter [ how ] [ many ] [ times ] it's brought up. I agree that it's a shame that it's missing this feature, since I think Prettier is such a great and well-made tool, apart from it's lack of determinism.

[uriva]

I argue it should be the default.
Let's fork..

[pauldraper]

I'm sorry but this has already been discussed a lot and the best trade-off we've come up with is to keep objects multliline if they have a newline inside.

No, it's more obscure than that.

The object is kept multiline if there is a new line between the opening brace and first property

The following are formatted as multi-line:

const obj = {
  name1: "value1",
  name2: "value2",
};

const obj = {
  name1: "value1", name2: "value2",
};

const obj = {
  name1: "value1",
  name2: "value2", };

The following are formatted as single-line:

const obj = { name1: "value1", name2: "value2", };

const obj = { name1: "value1",
  name2: "value2",
};

it's not going to happen, no matter [ how ] [ many ] [ times ] it's brought up

The current Prettier documentation explicitly says

THIS IS NOT A FEATURE

The semi-manual formatting for object literals is in fact a workaround, not a feature.

https://prettier.io/docs/en/rationale.html

The maintainers have invited pull requests. #2068 (comment)

[pauldraper]

You can use https://github.com/resolritter/prettier-plugin-compactify though that project isn't active. (And you can only have one of these "preprocess" plugins per language....I already use prettier-plugin-organize-imports.)

[roman-petrov]

I tried the plugin prettier-plugin-compactify and it working almost great, but a bit outdated: it does not supports some latest TypeScript features like import assertions. Just hoping that such kind of preprocessor will be part of prettier core as a configuration options some days...

[pauldraper]

I've created PR #16163.

[nullromo]

@pauldraper Looks like a very well-reasoned PR. I wish you the best of luck!