1558517 - (prettier-format) [meta] Use Prettier for formatting JS in mozilla-central

Status: RESOLVED FIXED

(Developer Infrastructure :: Lint and Formatting, task)

Description

[Victor Porof [:vporof][:vp]]

We plan to reformat m-c using Prettier on July 5 before the merge. This bug serves as a place for reviewing a few more preliminary patches required before a format can be done.

Prettier was added top level in bug 1556013, smoke tests already carried out with devtools/debugger in bug 1551218.
Automatic patch reformatting during rebase is being taken care of in bug 1556328 and bug 1556393

The full roadmap is available here: https://docs.google.com/document/u/1/d/1qV3aULyhulHsNHvnlbgAxeqlMGnpklUnxmpnY1OovXk

Four different competing prettifiers were experimented with earlier this year, carrying out reformats on the entire tree and measuring their impact for every single rule individually on all of m-c. In the end, Prettier was chosen because it was the only tool providing consistency and predictability, along with parsing and integration compatibility with our existing codebase. Out of all the possible configuration options rules, we've also picked the ones that had a measurable minimal code churn, otherwise the default values were kept.

Comment 1

[Victor Porof [:vporof][:vp]]

Attachment: Bug 1558517 - Pre 0: Prevent circular dependencies between the top-level .eslintrc and eslint-plugin-mozilla, r=standard8

Comment 2

[Victor Porof [:vporof][:vp]]

Attachment: Bug 1558517 - Pre 1: Add eslint-plugin-fetch-options as a peer dependency, r=standard8

Depends on D34532

Comment 3

[Victor Porof [:vporof][:vp]]

Attachment: Bug 1558517 - Pre 2: Move the **/*.*html overrides to recommended.js, r=standard8

Depends on D34533

Comment 4

[Victor Porof [:vporof][:vp]]

Attachment: Bug 1558517 - Pre 3.0: Remove conflicting eslint rules, and turn on "curly: all" everywhere, r=standard8

Depends on D34534

Comment 5

[Victor Porof [:vporof][:vp]]

Attachment: Bug 1558517 - Pre 3.2: Disable conflicting eslint:recommended rules by extending eslint-config-prettier, r=standard8

Depends on D34535

Comment 6

[Victor Porof [:vporof][:vp]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=947062f03c54f5f5b56ffafea6e6d32d65deb176

Comment 7

[Matthew N. [:MattN]]

Please don't file bugs with empty descriptions, especially when there are no related bugs with context. There is always a minimal amount of context that can easily be provided.

Comment 8

[Victor Porof [:vporof][:vp]]

(In reply to Matthew N. [:MattN] (PM me if requests are blocking you) from comment #7)

Please don't file bugs with empty descriptions, especially when there are no related bugs with context. There is always a minimal amount of context that can easily be provided.

We plan to broadcast this plan more officially sometime this week.
Edited comment 0 for some context.

Comment 9

[Victor Porof [:vporof][:vp]]

Attachment: Bug 1558517 - Pre 6: Fix instances where the `curly` eslint rule was changed for Prettier compatibility, r=standard8

Depends on D34536

Comment 10

[Victor Porof [:vporof][:vp]]

Attachment: Bug 1558517 - Pre 3.1: Change conflicting "camelcase" rule which would fail after running Prettier, r=standard8

Depends on D34535

Comment 11

[Victor Porof [:vporof][:vp]]

Attachment: Bug 1558517 - Pre 4: Ignore html files when running Prettier, r=standard8

Depends on D34682

Comment 12

[Victor Porof [:vporof][:vp]]

New try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=4ceaa8739c9cd4534ed175b7798d703b4d588e79

Comment 13

[Mark Banner (:standard8)]

What's the landing plan for this? AIUI part 5 and probably some others are going to disable the ESLint style rules as soon as these patches land. I'm a little nervous about doing that as that's going to leave us open to bad formatting for a couple of weeks. Maybe not too bad but I'm not sure how much we want to let this regress.

Comment 14

[Victor Porof [:vporof][:vp]]

None of this touches m-c before July 5. When the trees close, these patches land alongside the mega formatting.

Comment 15

[Victor Porof [:vporof][:vp]]

Attachment: Bug 1558517 - Pre 5: Remove redundant or unused globs from .prettierignore, r=standard8

Depends on D34696

Comment 16

[Victor Porof [:vporof][:vp]]

New try https://treeherder.mozilla.org/#/jobs?repo=try&revision=0499e2859f37998df213d4a0bff3a6e65d92fb85

Comment 17

[Victor Porof [:vporof][:vp]]

Try run with everything: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3af08bc51a3ee091df076b78c4b8386fec7a683c

Comment 18

[Victor Porof [:vporof][:vp]]

Attachment: Bug 1558517 - Pre 6: Add default user profiles to .prettierignore, r=njn

Depends on D35577

Comment 19

[Victor Porof [:vporof][:vp]]

Attachment: Bug 1558517 - Pre 3.3: Auto-fix for Prettier compatibility in browser/, r=standard8

Depends on D34536

Comment 20

[Victor Porof [:vporof][:vp]]

Attachment: Bug 1558517 - Pre 3.4: Auto-fix for Prettier compatibility in dom/, r=standard8

Depends on D36399

Comment 21

[Victor Porof [:vporof][:vp]]

Attachment: Bug 1558517 - Pre 3.5: Auto-fix for Prettier compatibility in layout/, r=standard8

Depends on D36400

Comment 22

[Victor Porof [:vporof][:vp]]

Attachment: Bug 1558517 - Pre 3.6: Auto-fix for Prettier compatibility in netwerk/, r=standard8

Depends on D36401

Comment 23

[Victor Porof [:vporof][:vp]]

Attachment: Bug 1558517 - Pre 3.7: Auto-fix for Prettier compatibility in toolkit/, r=standard8

Depends on D36402

Comment 24

[Victor Porof [:vporof][:vp]]

Attachment: Bug 1558517 - Pre 3.8: Auto-fix for Prettier compatibility in the rest of m-c, r=standard8

Depends on D36403

Comment 25

[Victor Porof [:vporof][:vp]]

Attachment: Bug 1558517 - Pre 7: Bump eslint-plugin-mozilla version, r=standard8

Depends on D36275

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/5f3e686dff2d
Pre 0: Prevent circular dependencies between the top-level .eslintrc and eslint-plugin-mozilla, r=standard8
https://hg.mozilla.org/mozilla-central/rev/51f30640669e
Pre 1: Add eslint-plugin-fetch-options as a peer dependency, r=standard8
https://hg.mozilla.org/mozilla-central/rev/0ec32b1bed9c
Pre 2: Move the **/*.*html overrides to recommended.js, r=standard8
https://hg.mozilla.org/mozilla-central/rev/d8ba04c4c323
Pre 3.0: Remove conflicting eslint rules, and turn on "curly: all" everywhere, r=standard8
https://hg.mozilla.org/mozilla-central/rev/cca4e4c511dd
Pre 3.1: Change conflicting "camelcase" rule which would fail after running Prettier, r=standard8
https://hg.mozilla.org/mozilla-central/rev/be08adefd6e4
Pre 3.2: Disable conflicting eslint:recommended rules by extending eslint-config-prettier, r=standard8
https://hg.mozilla.org/mozilla-central/rev/f4cf776be931
Pre 4: Ignore html files when running Prettier, r=standard8
https://hg.mozilla.org/mozilla-central/rev/75e8178094e1
Pre 5: Remove redundant or unused globs from .prettierignore, r=standard8
https://hg.mozilla.org/mozilla-central/rev/1590c5edfdc4
Pre 6: Add default user profiles to .prettierignore, r=njn
https://hg.mozilla.org/mozilla-central/rev/cffdfed81a70
Pre 7: Bump eslint-plugin-mozilla version, r=standard8, CLOSED TREE

Comment 27

[Sylvestre Ledru [:Sylvestre]]

Victor, can we close this bug now ? :)
thanks