Configure Prettier to add trailing commas everywhere

[teoli2003]

Little by little, we are manually running Prettier on more parts of MDN (Games/, Related/, and soon WebAssembly/) in preparation for its activation in workflows and/or pre-commit hooks (When this will happen is something we don't know yet 😄 and – don't worry – there will be a discussion ahead of it.)

Prettier, by design, has very few configuration options – Prettier is opinionated. Nevertheless, we have already decided on changing one from the default (the position of the final > of a multiline tag).

It has been proposed there to add a second one:

Trailing commas are not mandatory in JavaScript and have not been accepted everywhere in the past. By default, Prettier adds them where they are legal in ES5. Prettier provides an option to allow them where they are legal in ES2017. More info about the config option

Considering that:

• it adds more consistency to our code,
• we allow and recommend writing examples with ES6+ features on MDN,
• I've been told (not found the first-hand source) that this would likely be the new default in Prettier v3,

I think this is a good idea.

What do you think?

cc/ @OnkarRuikar @nschonni (I can't add you to the Reviewers list as we hit the max)

[This is a check for consensus: approve and/or add a thumb-up if you agree; add a thumb-down and explain why if you disagree]

[Josh-Cena]

Here's the Prettier v3 release plan: prettier/prettier#13142

Change trailingComma option default value to all (prettier/prettier#11479)

[Elchi3]

I'm 👍 on using Prettier defaults.
I'm 👎 on not using Prettier defaults.

[bsmth]

tnx 👍🏻

[Josh-Cena]

I'm 👍 on using Prettier defaults.
I'm 👎 on not using Prettier defaults.

Prettier defaults change, so... sometimes we have to form our opinions ¯_(ツ)_/¯ (What if they change indentation from space to tabs?) The current config is exactly my opinion.

[teoli2003]

I would have been against the change if this wasn't becoming the new default. That way, migration to v3 (when it is coming out) won't lead to changes all over the place (at least not this one).

[bsmth]

I would have been against the change if this wasn't becoming the new default.

➕1 from me on that. Also, having .prettierrc.json only contain non-default values might be good when we start using v3, so maybe removing it from the config (when v3 lands) might be tidier.

[queengooborg]

Setting trailing commas to "all" sounds good to me since it's becoming the new default, and setting it now will help the transition to Prettier v3 when the time comes!

[Josh-Cena]

This is the only option I'm aware of that's changed because of improved syntax compatibility. Now if they change anything else, it's a pure stylistic decision. Should we also shift ours in that case?

[teoli2003]

This is the only option I'm aware of that's changed because of improved syntax compatibility. Now if they change anything else, it's a pure stylistic decision. Should we also shift ours in that case?

Likely. But I think we'll see this in time. 😄

[wbamberg]

I'm 👍 on using Prettier defaults. I'm 👎 on not using Prettier defaults.

I agree with this, so I'm 👎 here. But I don't have strong feeling about it.

(Yes, the default will probably change. So what? Change when it does.)

[nschonni]

(I can't add you to the Reviewers list as we hit the max)

I'm not a contributor, so I can't be assigned reviews.

I think this makes sense as using the trailing commas reduces the the diff when adding lines, since the previous line doesn't also get updated to add the comma. I think this is likely why Prettier is changing this, since most of their defaults aim to minimize this (including the annoying bracketSameline that was opted out of)

[Josh-Cena]

Prettier is only keeping this default because of backward compatibility. Otherwise they would have changed it long ago. There's literally no reason to arbitrarily not add trailing commas for function calls but add them for object literals (the default is "es5", not "none"), if not because it's only allowed after ES2017. Sometimes we have to have more discretion than "use the defaults".

[OnkarRuikar]

During this set up period we need to get all the wanted features done, and not wait.

To avoid surprises, let's pin the dependency version and not have Dependabot update it.

[nschonni]

The only other possible override from the limited options in Prettier would be if there are any changes to the default parsers https://prettier.io/docs/en/options.html#parser, but that's a separate discussion

[Josh-Cena]

No, we just need to get this option in, and stop worrying about Prettier version changes in the near future.

[caugner]

To avoid surprises, let's pin the dependency version and not have Dependabot update it.

Major updates don't get auto-merged, so it shouldn't really be an issue, see:

content/.github/workflows/auto-merge.yml

Line 16 in 6e821c2

target: minor

[teoli2003]

So I'm making the call here:

• We don't have a consensus.
• The drawbacks are minors
• There is no loss of information (I.e. both decisions are revertable)

Note that if Prettier changes its default – it is likely in v3 –, the issue behind this PR will be automatically fixed (with consensus). Prettier v3 may land even before we automate Prettier on MDN.

So let's not add an extra config option here. (Even if I was in favour of it)

[queengooborg]

It has been over six months since this PR was closed, despite us actually having a consensus (not a unanimous one), and Prettier v3 still has not come out. I think that we should revisit this, see #24143.