Prettier Options compatibility

[MichaReiser]

The goal of this discussion is to come to an agreement on what JavaScript Prettier options Rome should support and what values it should use as defaults. The discussion will also serve as a reference for our reasoning in the future.

Rome supports the following Prettier options today:

• Print width
• Tab width
• Quotes

These options aren't part of this discussion except if Rome and Prettier use different defaults.

Supporting fewer options is desirable from a technical position because it reduces the number of possible option combinations, which reduces the overall complexity. The main non-technical benefit is that it removes debates about what the "right" formatter configuration is, one of the main goals of Rome's formatter. You can read more in Prettier's excellent philosophy documentation.

This discussion is related to the Formatter options discussion but is focused on inheriting options from Prettier.

[MichaReiser]

Tabs

Both Rome and Prettier support the Tabs options but Prettier defaults to spaces whereas Rome defaults to tabs.

Rome defaults to tabs because they are more accessible. See this thread for a past discussion #425

Proposal

Keep tabs as the default. Default to spaces if migrating a project from prettier.

[ematipico]

I agree with the proposal

[vjeux]

Since you have a way to parse how many prettier configs have each, would be interesting to know how many have switched to tabs.

[MichaReiser]

There are about 200k usages of the useTabs setting in prettier.rc files. But this includes useTabs: false and useTabs: true. That's about 20% of all config files (minus all the project that don't have a configuration file).

[scarf005]

I suggest rome to default to use spaces.

rationale

it would safe to assume large portion of rome users would expect its defaults to use spaces, since search from https://cs.github.com shows that 89.7% of prettier config uses spaces, and 1/3 of all prettierconfig explicitly use spaces.

Options	explicit useTab: true	explicit useTab: false	implicit useTab: false	Total
Usages	6.8k	25k	34.2k (estimated)	66k
Percentages	10.3%	37.8%	51.9% (estimated)	100%

and in current rome.json usage, it shows that around 59% of usages explicitly uses indentStyle: space .

Options	explicit indentStyle: space	explicit indentStyle: tab	implicit indentStyle: tab	Total
Usages	49	9	25	83
Percentages	59.04%	10.84%	30.12%	100%

[MichaReiser]

Quote Props

The quote props option specifies if the formatter should remove quotes around props that aren't syntactically necessary. Rome doesn't support this option today.

The reason why quote props is configurable in Prettier is that quoted props prevent minifiers from mangling that property name.

Proposal

Don’t add support for quote props and hint users towards rome-ignore to disable Rome from stripping the quotes. This assumes, that disabling the behaviour is only needed in rare cases.

Note

Implementing the removal of quote props is tracked in #2405

[Conaclos]

@MichaReiser
The behavior you describe is a mix of "consistent" and "preserve".
Again "consistent" removes quotes in cases where there are not needed.

As an example, this code

const DICT = {
  "key": 1,
};

is formatted to the following code under "consistent":

const DICT = {
  key: 1,
};

because quotes are not "needed" for this property.

Thanks for your attention :) Have a nice evening!

[Conaclos]

Note: I am also ok with this mix because it looks more consistent. However it is not possible to reproduce that in prettier. I do not know whether it is an issue...

[MichaReiser]

Oh, I understood that "consistent" would preserve the quotes if at least one prop uses quotes. But I guess, this doesn't make sense.

Agree, Rome should then support "preserve" to cover the minifier use case and "as-needed" to remove them (default).

[vjeux]

The only reason we have this one is for Google Closure compatibility. In practice Google is the main group of people using Closure advanced mode that requires this distinction.

The amp project is using prettier and from what I’ve been told many more projects within Google are using it after this option was added.

main issue where this was discussed: prettier/prettier#4327
Pr that implenented it: prettier/prettier#5934

[ematipico]

Thank you everyone for the constructive conversion. It seems that this is an option that would like to maintain, so we will start working on it.

As mention by @MichaReiser , we would need to have a default behaviour that mimics "as-needed" from Prettier, and then have a way to opt-in the "preserve" Prettier option.

That being said, default Rome's behaviour will be "as-needed" and then we can have an option called --preserve-quotes /preserve_quotes to opt-in the "preserve" one.

[MichaReiser]

Semicolons

Prettier supports the semicolons option that specifies if semicolons should be inserted at the end of statements.

Proposal

Don't add support for it. My research shows that the option isn't widely used:

• 1K appearances of the noSemi option
• 2.3M .prettierrc files existing on GitHub (which doesn't account for everyone using the defaults).

Edit:
The configuration option is semi: false and not noSemi. GitHub search shows around 230k usages of semi: false

[mrm007]

In reality, there are over 300K instances of "semi": false, which seems significant.

I'm pretty sure that number should be taken with a grain of salt. You'll notice that some files contain multiple hits, which significantly inflates the total number of hits (here's a random example with 3 matches).

If the numbers reported by GitHub Search are to be believed, then we should add up the total of:

• results for JavaScript (between 15k-30k)
• results for JSON (between 15k-30k)
• results for YAML (between 3k-6k)

Total: 30k-70k, depending on the phase of the moon. An order of magnitude lower than the quoted 300k.

GitHub Code Search reports around 23k total, with one hit per file.

[bilalq]

The number also undercounts shared configs and private repos. For example, we have dozens of repos using prettier without semicolons from a single shared prettier config. The "semi": false pattern would only appear once. I'm sure many other orgs experience something similar.

[bilalq]

Also another 21k or so from package.json usages of the semi: false config:

https://github.com/search?p=5&q=%22semi%3A+false%22+filename%3Apackage.json&type=Code

[jeysal]

IIRC in the static site generator space, Gatsby etc., as well as with MDX users, semi: false is quite popular

[MichaReiser]

Thank you all for the number crunching! The support for semi: false is tracked in #3535. We see it as a valuable option that supports the setup of many people in the community, but it is't a high priority. Leave a comment on the issue if you're interested in implementing the feature so that we can help you.

[MichaReiser]

Trailing Commas

The trailing commas option configures where to insert trailing commas. It supports three different configurations:

• none: Removes all trailing commas
• es5: Adds trailing commas in places permitted by ES5
• all: Add trailing commas everywhere

The reason Prettier added this option is for backward compatibility because trailing commas have only been supported in a few places prior to ES2017 and TypeScript 2.7. Thus, it would have been necessary to transform the source code if targeting a runtime that doesn't support ES2017.

• Node supports ES2017 trailing commas since version 8.2 which was released in 2017 (now 17.9)
• All major browsers support ES 2017 trailing commas since mid-2017 source (at the very end)

Proposal

Don’t support the trailing comma option because all modern engines support them today and the sole reason the option was introduced by Prettier is for compatibility’s sake. Prettier even changed the default from none to es5 in their v2 release.

Alternatives

Implement the option and either:

• Support es5 and all
• Support none, es5, and all

[MichaReiser]

My understanding is that our current behaviour matches Prettier's all by only inserting a comma if a group breaks.

I don't think we need to track the language version for es5. It should be possible to not generate the trailing comma token for function arguments if the traling_coma option is set to es5.

Is your view that Rome should add support for the trailing comma option? If so, what's your reasoning?

[leops]

If the only difference between es5 and all if in function arguments I guess it's alright to support both, but otherwise I think supporting es5 would just add more complexity than necessary.
Overall I don't think we should implement support for the trailing commas option, to me the only reason we would want to implement it would be specifically to be compatible with Prettier: if we're not aiming for 100% compatibility with Prettier only implementing the all behavior seems like the better option.

[ematipico]

I agree with the proposal. Unless we aim to use the formatter on IE11, I don't see the value of the additional complexity.

[vjeux]

It’s amazing that all the mainstream parsers now support trailing commas!

[Th3S4mur41]

Prettier just release version 3 setting all as the new default for trailing comma

[MichaReiser]

JSX Quotes, Bracket Spacing, Bracket Line, and Arrow Function Parentheses

Prettier mentions on their philosophy page that they find these options harder to motivate and only keep them because it's difficult to remove them now:

But other options are harder to motivate in hindsight: --arrow-parens, --jsx-single-quote, --bracket-same-line and --no-bracket-spacingare not the type of options we’re happy to have. They cause a lot of bike-shedding in teams, and we’re sorry for that. Difficult to remove now, these options exist as a historical artefact and should not motivate adding more options (“If those options exist, why can’t this one?”).
Source]

Proposal

Take Prettier's learning and don't add these options to Rome. Hopefully, this will help Prettier too to remove these options at some point in the future.

[scarf005]

@MichaReiser could you check above? related #3910

[vjpr]

What are the stats on --no-bracket-spacing?

I don't get this rule...why do we add spaces inside curly brackets, but not round or square brackets? Seems completely arbitrary. No one is typing spaces as they go.

foo({ bar: 1 })
foo(['bar', 'baz'])
foo('bar')

[jaskp]

I would actually expect --no-bracket-spacing to be default for Rome, as the extra space characters don't offer any objective advantage (unlike semicolons to avoid ASI bugs, arrow-parens where the extra parentheses offer easier type annotations and default values, trailing commas that can reduce the git diff when changing code, etc.)

[Conaclos]

What are the stats on --no-bracket-spacing?

Looks like it is under-used.

[jaskp]

@Conaclos it's "bracketSpacing", not "noBracketSpacing", so 13.7K usages via GitHub

[MichaReiser]

Hey @vjeux would you have a moment to share some of your experience from working at Prettier? I love Prettier's philosophy document and I have the impression you really figured it out what options you want and don't want to support! I also think it would be great if our options share the same semantics to reduce friction and confusion in the community.

I've also been wondering what Prettier's plan is for the upcoming 3.0 release. Does Prettier plan to change any defaults or to deprecate or remove any options?

[vjeux]

Thanks for the ping! I am not longer actively involved with Prettier those days and can’t comment on the plans for Prettier 3.0. But I added historical context for all the options you mentioned.

[MichaReiser]

Thank you for these detailed explanations! My take away from your explanations are

• We should add support for quoteProps to properly support closure's advanced mode but we may not need to do so today because this mode isn't as widely used.
• Some options were added to drive adoption at FB or by strong ask from the community. My proposal would be to not add support these options for now because Prettier standardized the code styles in the community in recent years, so the asks for these options might be lower today. This shouldn't mean that Rome should never support these options and we may want to reconsider later.

I wasn't aware that you're no longer actively involved in Prettier's maintenance. Do you know someone who still is?

[Conaclos]

Makes sense.

[camsjams]

I would love to see the option to disable parenthesis on JSX.

Rome and Prettier rewrite code with wrapping parenthesis all over JSX components and it creates a lot of ugly noise and mess. See this largely ignored request on Prettier repo: prettier/prettier#2724 (comment)

Switching to Rome doesn't seem so useful if it produces the same output as Prettier.

Some kind of option like this on Rome would help a large list of people on that thread since Prettier won't budge on it.

[ematipico]

Yet, the introduction of another will create bikeshedding. Not sure if that's what you want.

[camsjams]

Prettier bikesheds about bikesheds... did you read the thread linked above and note the large amount of requests for this feature?

I admire the goal of removing options to somehow prevent bikeshedding, but this seems more like a way to baby development teams with strict rules and allow maintainers to have a reason to not add any features.