Change arrowParens to always by default for 2.0

[kachkaev]

Closes #6929

• I’ve added tests to confirm my change works.
• (If changing the API or CLI) I’ve documented the changes I’ve made (in the docs/ directory)
• (If the change is user-facing) I’ve added my changes to changelog_unreleased/*/pr-XXXX.md file following changelog_unreleased/TEMPLATE.md.
• I’ve read the contributing guidelines.

✨Try the playground for this PR✨

[lipis]

Is that have been decided? :(

[kachkaev]

@lipis I haven't seen objections with many 👍 in reactions, so it seems like changing this default is a go. See #6929, #6888 (comment) and #7417 (comment) for details. Similar case to #6963.

[lipis]

Whiile we at it.. let's remove the bracketSpacing too by default

[kachkaev]

Changing default for bracketSpacing would probably require a discussion like #6929, #6932 or #6931, which would be up for at least a couple of weeks. Not sure if there's enough time for this before 2.0 TBH 🤔

[ExE-Boss]

I disagree with this as it makes code like in #6929 (comment) much noisier:

arrowParens: "avoid": 👍🏻

categories
	.filter(x => x.name === category)
	.flatMap(x => x.pages)
	.filter(x => x.name.includes(search))
	.forEach(x => (x.highlight = true));

arrowParens: "always": 👎🏻

categories
	.filter((x) => x.name === category)
	.flatMap((x) => x.pages)
	.filter((x) => x.name.includes(search))
	.forEach((x) => (x.highlight = true));

[kachkaev]

@ExE-Boss these concerns have been discussed in #6929. A noise-free visual look of the code and a great developer experience are not always in line; the latter has higher priority.

Parentheses are a similar story to semicolons or single / double quotes. With semicolons, nearly every line has a noisy character, but a developer does not have to think about horrible edge cases like ;[] or ;(). With double quotes, there's 50% more 'ink' used, but there is consistency between JS and other languages (including JSON), which improves DX in certain scenarios like search.

You don't have to agree with the benefits of consistency and rule shortness when making formatting decision for your projects. If that’s the case, Prettier offers you configuration options, although generally discourages you from overusing them. The ability to set { "arrowParens": "avoid" } does not go anywhere in v2.0 😉

[karlhorky]

@kachkaev I put my 👍 on your comment as I think that the parallel with semicolons makes sense! There are footguns with no-semicolon JS, and there are almost-footguns with parentheses (while adding a new parameter afterwards).

However, I think the "double quotes" issue is different and the default should be changed here in the future to single quotes (see #4102). Why? Because it's not about other languages, it's about JavaScript. And JavaScript is much more commonly written with single quotes in the wild.

Plus, there are no footguns with single vs double. If you wanted to talk about footguns, actually the better option would be to switch everything to template strings, so that you can easily interpolate later (parallel: easily adding parameters later). But this style is even less common, and I would not suggest that Prettier change to this.

[alexander-akait]

@kachkaev Can you rebase?

[kachkaev]

Oh, I did not notice there were conflicts, thanks for pointing this out @evilebottnawi 😬 I was a bit wondering: why does everyone approve the changes, but nobody merges? 😅

Instead of rebasing, I merged next branch into my branch and resolved conflicts in one snapshot afterwards. The diff still looks clean. The PR will be squashed anyway, so the details of the commit history do not matter much. By not rebasing, I keep git history more 'honest' and thus easier to follow for those who are curios how this PR was emerging 🙂

[jurosh]

I disagree with this as it makes code like in #6929 (comment) much noisier:

arrowParens: "avoid": 👍🏻

categories
	.filter(x => x.name === category)
	.flatMap(x => x.pages)
	.filter(x => x.name.includes(search))
	.forEach(x => (x.highlight = true));

arrowParens: "always": 👎🏻

categories
	.filter((x) => x.name === category)
	.flatMap((x) => x.pages)
	.filter((x) => x.name.includes(search))
	.forEach((x) => (x.highlight = true));

Agree with @ExE-Boss
For functional programming maps, filters this looks much worse. For typescript, this would make sense only for named arrow functions - BUT never for unnamed functions in maps/promises.

So I am pretty sad by this new default as there will be lot of new fights in development teams to keep using this new or always use arrowParens: "avoid". I would def use avoid whenever possible.