A copy of this email can be found here: https://docs.google.com/document/d/1UDERMflocqdszMGhhtxiVhaCTVOHo6jxLsGbt4BR9uw <https://docs.google.com/document/d/1UDERMflocqdszMGhhtxiVhaCTVOHo6jxLsGbt4BR9uw>
TL;DR To improve developer productivity, we plan to automate JS formatting using Prettier <https://prettier.io/>, a broadly adopted tool specifically designed for this task and maintained by well known stakeholders. This should reduce the amount of time spent writing and reviewing JS code. This choice was informed by explicit goals such as compatibility with third-party libraries (e.g. React), consistency, and predictability. The resulting integration includes automation (try, phabricator) as well as CLI (mach). Changes have been tested and verified with a small part of the codebase before being rolled out to the entire tree. Background Mozilla is more invested than ever in using automated tools for checking and rewriting code. Various tools are already in use on CI, for example clang-format <https://clang.llvm.org/docs/ClangFormat.html>, checkstyle <https://github.com/checkstyle/checkstyle>, flake8 <http://flake8.pycqa.org/en/latest/>, as well as C/C++ static analysis. For JS, eslint <https://eslint.org/> is primarily used for ensuring code quality, however consistent and predictable styling was a non-goal until now. As with C++, Mozilla has had a coding style for JS, but it’s not entirely enforced through tooling. In addition, the current style is only used in our projects; by contrast, Prettier is complete, well supported <https://www.npmjs.com/browse/depended/prettier> in tooling, and widely used <https://prettier.io/en/users/> by many other stakeholders. We still have styling inconsistencies in our codebase. On top of that, we have been spending a lot of time talking about and adjusting whitespace. This wastes human time when writing, reviewing, and discussing code. Details We will build on the existing tooling foundation by applying coding style checks and automated rewrites throughout the authoring process (pre-commit, commit, review, landing). We’re announcing the following changes to our coding style and its enforcement through our commit policy: 1. We will migrate to the Prettier coding style to encourage more consistent code. We will be preserving all other aspects pertaining to code quality (linting) through eslint. 2. We will check conformance with coding style rules upon review and landing, so that issues can be easily addressed or fixed through automation. The preference will be to enforce style issues as early as possible (before landing) to avoid late surprises. 3. We will automatically enforce restrictions on formatting of whitespace (such as indentation and braces). For other stylistic issues (such as naming), no particular style is enforced, and consistency with surrounding code should take precedence. These changes have been approved both by Firefox senior engineering leadership and the Firefox Technical Leadership Module <https://wiki.mozilla.org/Modules/Firefox_Technical_Leadership>. We’ve already smoke-tested these changes, as well as fine tuned various aspects of the tooling with the DevTools and Activity Stream teams. When? A month ago, we pushed a patch <https://bugzilla.mozilla.org/show_bug.cgi?id=1551218> to reformat and enable CLI and automation support for Prettier in a section of the tree (devtools/debugger). This first step was intended to be a smoke test in a controlled environment with a team of developers who have volunteered to help us test this change. We haven’t found any major surprises. Last Friday we landed <https://bugzilla.mozilla.org/show_bug.cgi?id=1556013> top-level support for Prettier, as a preliminary stop-gap for allowing folks to check out what `./mach lint --fix` does for them, and manually whitelist sources in the meantime if needed. Assuming that everything continues to go well, on Friday, July 5, we will push a patch to mozilla-central in order to rewrite the entire tree using Prettier. We want to do this work right before a merge (when nightly version moves to beta) to limit the impact on the beta uplift process. To mitigate the impact on the developers who backport fixes to ESR, two weeks after the merge we will also reformat the ESR68 code base. A more detailed roadmap is available here <https://docs.google.com/document/d/1qV3aULyhulHsNHvnlbgAxeqlMGnpklUnxmpnY1OovXk/edit#>. Next week during Whistler, we also invite you to the “Using Prettier across Mozilla” lightning talk to discuss in person about this project. How? To reformat code locally after the landing on July 5: $ ./mach lint <glob> --fix Eslint integrations for most popular code editors already allow developers to easily format the code they’re working on at the time they’re making changes to it. Prettier leverages this infrastructure and is triggered automatically when running `eslint --fix`. If your code editor has an eslint plugin, enable auto-fix to trigger Prettier. We’ll share some tutorials about this soon. Providing Feedback This post is intended as an announcement, but we do welcome your feedback on this, both at a high level and at the technical level. For high level feedback please reach out to vpo...@mozilla.com <mailto:vpo...@mozilla.com>. For technical feedback (e.g. bugs about the conversion process) please file bugs under bug prettier-format <https://bugzilla.mozilla.org/show_bug.cgi?id=1558517>. We understand that coding styles can be subjective. Many people would agree that having a consistent style across the code base is valuable, and that’s not where we are now. This change doesn’t mean that the coding style will remain forever this way; it gives us the opportunity to easily change our coding style and apply the change on the code base. It’s the first step to actually having consistency. Thanks to the many who have helped out technically and with planning, including Dave Townsend, Mark Banner, Dan Mosedale, Kate Hudson, Ed Lee, Jason Laster, Sylvestre Ledru, Andi-Bogdan Postelnicu, Sebastian Hengst and Ritu Kothari. Victor FAQ: 1. Can I see what it looks like? To reformat code locally today, apply the patches in bug 1558517 <https://bugzilla.mozilla.org/show_bug.cgi?id=1558517> and whitelist the respective sources in the top level .prettierignor <https://dxr.mozilla.org/mozilla-central/source/.prettierignore>e <https://dxr.mozilla.org/mozilla-central/source/.prettierignore> file, then: $ ./mach lint <glob> --fix A formatted repository is available on Github: https://github.com/victorporof/gecko-dev/tree/prettier <https://github.com/victorporof/gecko-dev/tree/prettier> to visualise the changes. 2. Can I ignore just a file? If you have a good reason, yes. Just add it into .prettierignore <https://dxr.mozilla.org/mozilla-central/source/.prettierignore> with a comment explaining why it should be ignored. 3. Can I ignore a whole directory? If it’s third party code, yes! Just add it into thirdpartypaths.txt <https://dxr.mozilla.org/mozilla-central/source/tools/rewriting/ThirdPartyPaths.txt?q=thirdpartypaths.txt&redirect_type=direct> and `mach` will ignore it. Otherwise, you can also add a glob to .prettierignore <https://dxr.mozilla.org/mozilla-central/source/.prettierignore>. 4. Why not continue using eslint? To a great degree, we’ve moved closer towards a consistent codebase using eslint. However, eslint specializes in code quality checks, and not formatting. For example, auto-fix is not available for most stylistic issues <https://eslint.org/docs/rules/#stylistic-issues>. Another problem pertains to output being inconsistent depending on the initial styling. Furthermore, depending on which rules are chosen, conflicts may also arise and successive runs can result in differently styled output. All of those issues are incompatible with our goals. To enforce a coding style, consistency and predictability are necessary, both of which are not possible with eslint. We’ll continue having eslint coexisting with Prettier. Rules strictly pertaining to code quality will be preserved, and this task will continue to be in eslint's domain. Most styling-related rules will be removed in favour of separating this concern into Prettier’s domain. Both tools will be running in tandem. 5. Why not something else? Our choice of Prettier was informed by meticulously analyzing existing JS formatters, including eslint, jsbeautify and clang-format. We’ve tested reformatting mozilla-central using each one of them, measuring consistency (“is the output predictable regardless of source?”), integration effort (“can this fit into our CLI and build infrastructure?”), build/server time cost (“is this fast enough?”), capabilities (“can this parse our funny-looking JS or JSX?”) and also configurability (“can this tool suit our existing code quality checks?”). In addition, every single rule for all of those tools was also individually tested against all of mozilla-central, measuring code churn. We’ve gathered a lot of info, and decided that a minimal configuration <https://dxr.mozilla.org/mozilla-central/source/.prettierrc> is the least impactful objectively: all other configuration rules have relatively equal churn across the codebase regardless of the settings used, so we defined those as subjective and settled for the defaults. 6. Will this break blame on VCS? As with the clang-format rollout for reformatting C++, we will tag the formatting changeset commit message with “skip-blame” so that Mercurial would automatically ignore the reformat changeset for blame operations. 7. Will I have to rebase? In order to mitigate the impact for pending changes, we will re-enable format-source <https://pypi.python.org/pypi/hg-formatsource> and make it work with Prettier, to locally reformat the changes before the big patch is pulled. This will fix most of the conflict issues. We will share more details about this later. _______________________________________________ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
