Open Bug 1558517 (prettier-format) Opened 4 months ago Updated 19 days ago

(meta) Use Prettier for formatting JS in mozilla-central

Categories

(Firefox Build System :: Lint and Formatting, task)

task
Not set

Tracking

(Not tracked)

People

(Reporter: vporof, Unassigned)

References

(Depends on 5 open bugs)

Details

(Keywords: leave-open)

Attachments

(10 files, 7 obsolete files)

47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review

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.

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.

Assignee: nobody → vporof
Status: NEW → ASSIGNED

(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.

Attachment #9071309 - Attachment description: Bug 1558517 - Pre 3: Remove conflicting eslint rules, r=standard8 → Bug 1558517 - Pre 3: Remove conflicting eslint rules, r=standard8,MattN
Attachment #9071551 - Attachment description: Bug 1558517 - Pre 5: Fix instances where the `curly` eslint rules was changed for Prettier compatibility, r=standard8 → Bug 1558517 - Pre 5: Fix instances where the `curly` eslint rule was changed for Prettier compatibility, r=standard8
Attachment #9071310 - Attachment description: Bug 1558517 - Pre 4: Extend eslint-config-prettier to disable remaining conflicting rules from eslint:recommended, r=standard8 → Bug 1558517 - Pre 5: Extend eslint-config-prettier to disable remaining conflicting rules from eslint:recommended, r=standard8
Attachment #9071551 - Attachment description: Bug 1558517 - Pre 5: Fix instances where the `curly` eslint rule was changed for Prettier compatibility, r=standard8 → Bug 1558517 - Pre 6: Fix instances where the `curly` eslint rule was changed for Prettier compatibility, r=standard8
Alias: prettier-format

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.

Flags: needinfo?(vporof)

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

Flags: needinfo?(vporof)
Depends on: 1560186
Attachment #9071572 - Attachment description: Bug 1558517 - Pre 4: Change conflicting of the camelcase which would fail after running Prettier, r=standard8 → Bug 1558517 - Pre 4: Change conflicting camelcase rule which would fail after running Prettier, r=standard8
Attachment #9071306 - Attachment description: Bug 1558517 - Pre 0: Prevent circular dependencies with the eslint mozilla plugin, r=standard8 → Bug 1558517 - Pre 0: Prevent circular dependencies with the mozilla eslint plugin, r=standard8
Attachment #9073307 - Attachment description: Bug 1558517 - Pre 8: Remove redundant or unused globs from .prettierignore → Bug 1558517 - Pre 8: Remove redundant or unused globs from .prettierignore, r=standard8
Summary: Reformat the tree using Prettier → (meta) Use Prettier for formatting JS in mozilla-central
Depends on: 1561435
Attachment #9071306 - Attachment description: Bug 1558517 - Pre 0: Prevent circular dependencies with the mozilla eslint plugin, r=standard8 → Bug 1558517 - Pre 0: Prevent circular dependencies with eslint-plugin-mozilla, r=standard8
Attachment #9071309 - Attachment description: Bug 1558517 - Pre 3: Remove conflicting eslint rules, r=standard8,MattN → Bug 1558517 - Pre 3: Remove conflicting eslint rules, r=standard8
Depends on: 1561656
Attachment #9071306 - Attachment description: Bug 1558517 - Pre 0: Prevent circular dependencies with eslint-plugin-mozilla, r=standard8 → Bug 1558517 - Pre 0: Prevent circular dependencies between the top-level .eslintrc and eslint-plugin-mozilla, r=standard8
Attachment #9071309 - Attachment description: Bug 1558517 - Pre 3: Remove conflicting eslint rules, r=standard8 → Bug 1558517 - Pre 3.0: Remove conflicting eslint rules, and turn on "curly: all" everywhere, r=standard8
Attachment #9071572 - Attachment description: Bug 1558517 - Pre 4: Change conflicting camelcase rule which would fail after running Prettier, r=standard8 → Bug 1558517 - Pre 3.1: Change conflicting camelcase rule which would fail after running Prettier, r=standard8
Attachment #9071310 - Attachment description: Bug 1558517 - Pre 5: Extend eslint-config-prettier to disable remaining conflicting rules from eslint:recommended, r=standard8 → Bug 1558517 - Pre 3.2: Disable conflicting rules frrom eslint:recommended by extending eslint-config-prettier, r=standard8
Attachment #9071310 - Attachment description: Bug 1558517 - Pre 3.2: Disable conflicting rules frrom eslint:recommended by extending eslint-config-prettier, r=standard8 → Bug 1558517 - Pre 3.2: Disable conflicting eslint:recommended rules by extending eslint-config-prettier, r=standard8
Attachment #9071573 - Attachment description: Bug 1558517 - Pre 7: Ignore html files when running prettier, r=standard8 → Bug 1558517 - Pre 4: Ignore html files when running prettier, r=standard8
Attachment #9073307 - Attachment description: Bug 1558517 - Pre 8: Remove redundant or unused globs from .prettierignore, r=standard8 → Bug 1558517 - Pre 5: Remove redundant or unused globs from .prettierignore, r=standard8
Attachment #9074668 - Attachment description: Bug 1558517 - Pre 9: Ignore default user profiles, r=njn → Bug 1558517 - Pre 6: Ignore default user profiles, r=njn
Attachment #9071551 - Attachment is obsolete: true
Attachment #9071572 - Attachment description: Bug 1558517 - Pre 3.1: Change conflicting camelcase rule which would fail after running Prettier, r=standard8 → Bug 1558517 - Pre 3.1: Change conflicting "camelcase" rule which would fail after running Prettier, r=standard8
Depends on: 1563182
Depends on: 1563300
Depends on: 1563367
Attachment #9074914 - Attachment is obsolete: true
Attachment #9074915 - Attachment is obsolete: true
Attachment #9074916 - Attachment is obsolete: true
Attachment #9074917 - Attachment is obsolete: true
Attachment #9074918 - Attachment is obsolete: true
Attachment #9074919 - Attachment is obsolete: true
Attachment #9071573 - Attachment description: Bug 1558517 - Pre 4: Ignore html files when running prettier, r=standard8 → Bug 1558517 - Pre 4: Ignore html files when running Prettier, r=standard8
Attachment #9074668 - Attachment description: Bug 1558517 - Pre 6: Ignore default user profiles, r=njn → Bug 1558517 - Pre 6: Add default user profiles to .prettierignore, r=njn
Pushed by vporof@mozilla.com:
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
Depends on: 1563763
Keywords: leave-open
Depends on: 1563806
Blocks: 1563927
No longer blocks: 1563927
Depends on: 1563927
Depends on: 1564114
Depends on: 1564824
Depends on: 1564854
No longer depends on: 1564854
Depends on: 1564862
Depends on: 1567617
Depends on: 1569574
Depends on: 1572072
Depends on: 1572814
Depends on: 1576768
See Also: → 1564300
Depends on: 1566979
Assignee: vporof → nobody
Status: ASSIGNED → NEW
You need to log in before you can comment on or make changes to this bug.