Framework: Should we use Prettier?

[gziolo]

Issue Overview

The prettier javascript auto-formatter has already come up in the WordPress/packages PR and #core-js discussion a few weeks back. For anyone who wants a fun introduction to what prettier is, feel free to watch the creators lightning talk at ReactConf: https://www.youtube.com/watch?v=hkfBvpEfWdA
or listen to one of the episodes of 5 minutes of React podcast about Prettier:
https://m.soundcloud.com/5minreact_audio/018-prettier

tldw:

• Prettier auto-formats your code so you don't need to (DevEx++). A linter like eslint cannot reliably fix style issues (and also doesn't ensure standard formatting around things like linebreaks).
• It reduces the learning curve for new OSS contributors.
• It acts as a teaching tool by wrapping expressions according to javascript's order of operations
• Webpack, Babel, React, Jest, Zeit, Calypso and more are all using it (prettier is well tested).
• It is not very configurable. At the moment it doesn't support spacing conventions WordPress core's JS Coding Standards enforeces.
• The initial "npm run prettier" commit would mess with our git blame.

Props to @samouri for preparing a ver similar list here in the context of Calypso.

Current Behavior

In my opinion, there is one blocker that needs to be resolved, which @aduth mentioned in this post Proposal for JS Standards Revision: Removing Array/Function Whitespace Exceptions:

Coding standards have been a recurring topic in the JavaScript Weekly Chat over the past few months. One rule in particular which has been the focus of much discussion are the exceptions for whitespace in arrays and function calls ...

There seems to be general agreement that we should remove all exceptions, which would make usage of Prettier completely valid. There is still one issue that would have to be tackled - WordPress spacing conventions - which aren't supported by Prettier. There was already an open issue in Prettier where spaces between parens and brackets were proposed, but it was rejected.

Possible Solutions

• We can try to use ESLint plugin for prettier formatting, which adds autofixable prettier rule. If you run eslint with the --fix flag, your code will be formatted according to prettier style. It might get into conflicts with other rules, so it might not work as expected with spaces. See also this related article from @neciu to find out more.
• Automattic maintains Prettier fork that is used with Calypso which respects spaces between parens and brackets. We could use this fork or even better try to merge those changes back into Prettier.

[andreiglingeanu]

One more idea would be to use the newly introduced --require-pragma flag that will require a special flow-like pragma for flagging which files would be processed by the initial npm run prettier. That could soften out the initial burden introduced by it.

After the very small initial setup, every new addition will be autofixed by that ESLint plugin.

One more cool thing to note is the fact that prettier supports config files and config in package.json. That being said, one can go about hiding special calypso-prettier features behind specialized options that would make sense in the prettier itself, which will remove the need for maintaining the fork.

[gziolo]

One more idea would be to use the newly introduced --require-pragma flag that will require a special flow-like pragma for flagging which files would be processed by the initial npm run prettier.

@andreiglingeanu yes, this is how Facebook introduced prettier in their projects. It's a very nice way which helps to roll out it gradually.

That being said, one can go about hiding special calypso-prettier features behind specialized options that would make sense in the prettier itself, which will remove the need for maintaining the fork.

We would need calypso-prettier only to handle spaces between parens and brackets, which isn't handled by the original prettier. Otherwise, we don't need it at all. Ideally, we should encourage prettier maintainers to add this configuration as another option.

[noisysocks]

I'd love to see this. I've used prettier in the past on a Gutenberg-sized project and it's very nice to not have to be conscious about formatting rules while writing code.

[gziolo]

Another try to encourage Prettier team to add the spacing rule: prettier/prettier#1303 (comment) and https://twitter.com/gziolo/status/944165778655498240.

[ahmadawais]

I'm all for supporting Prettier + ESLint rule. While trying to build an ESLint + Prettier setup for WordPress I am having the same troubles being discussed here and am discussing it here at a Facebook thread.

Maybe we can build a combination of eslint, eslint-plugin-prettier, eslint-config-prettier, prettier and eslint-plugin-wordpress, eslint-config-wordpress. Convert it into a shareable config.

Not sure, thinking out loud here.

[gziolo]

Maybe we can build a combination of eslint, eslint-plugin-prettier, eslint-config-prettier, prettier and eslint-plugin-wordpress, eslint-config-wordpress. Convert it into a shareable config.

@ahmadawais, yes it makes perfect sense. We should definitely try to provide a setup which combines all that ingredients. Actually, I'm planning on spinning a PR with that in the upcoming days/weeks. I will put it under https://github.com/WordPress/packages.

[noisysocks]

For what it's worth, I've been using https://github.com/prettier/prettier-eslint in my day-to-day work on Gutenberg for the past 20 or so days. It works pretty nicely with just our .eslintrc.json and no additional configuration.

So far, I've only noticed one issue. prettier-eslint prints this:

{isVisible && (
    <Something
        foo="bar"
    />
)}

Instead of this:

{ isVisible && (
    <Something
        foo="bar"
    />
) }

[gziolo]

We can always add our own —fix for this case. I’m sure @aduth know all the tricks required to have it coded. I didn’t known prettier-eslint was moved to the prettier org. I assumed that it is no longer maintained...

[noisysocks]

Turns out it was just a matter of setting a property on one of our eslint rules: #4362 😄

[gziolo]

Do you have prettier-eslint configured globally and your IDE calls it on every file save?

[noisysocks]

I have it set up in my IDE using https://github.com/prettier/prettier-vscode but I haven't set it up to run automatically since doing so would introduce a lot of unrelated and probably distracting changes into my PRs.

In case anyone is curious, here's what I put in my VSCode settings (after installing the plugin) to have it work nicely for Gutenberg:

    "prettier.printWidth": 120,
    "prettier.tabWidth": 4,
    "prettier.eslintIntegration": true

[gziolo]

I started experimenting with reusable scripts which would simplify sharing code between Gutenberg and shared Wordpress packages in WordPress/packages#62.

@noisysocks, we should also work on prettier command which would use the setup you described. I like it a lot. If other people agree we could battle test it on the https://github.com/WordPress/packages repository :)

[andreiglingeanu]

Also, Prettier for PHP is coming 🎉

[gziolo]

Also, Prettier for PHP is coming 🎉

We already use phpcs to format WordPress core, so I guess we will continue using what has already been configured and tested in core.

[ahmadawais]

@noisysocks The spacing issue with Prettier is keeping me away from using it in the WordPress projects. Can you share with me how you have set it up along with ESLint to follow WordPress Code Standards?

Also would you be willing to help build an open source goto standard library that once included in WordPress projects would the formatting for us? I had a discussion with @gziolo on AWP FB Group about it and am really interested in building something like that.

[noisysocks]

The spacing issue I described should be fixed now that #4362 is merged.

[ntwb]

I've gone with an alternate method to that in #4374 of implementing Prettier and ESLint in #4628

[ntwb]

Closing per #4628 (comment) (Quoted below for reference)

---

Prettier and WordPress are both pretty opinionated, though there's only a couple of instances where those opinions really opposed each other, thankfully ESLint's --fix was able to bring a satisfactory compromise.

To that end examing whats left after WordPress has applied its ESLint fixes to the changes made by Prettier is relatively minor, primarily these changes relate to line length, where Prettier refactors the code to not exceed x characters in length, it does a pretty good job of that for the most part. As noted in the comments above sometimes it's not ideal where and when it does this it makes for poorly formatted code.

For the remaining instances of stylistic formatting that Prettier made we can look to add ESLint rules to detect these style inconsistencies and update our coding standards to match. Hopefully we can contribute ESLint rules and fixes upstream as much as possible for the benefit of all ESLint users.

I don't think Prettier is the right solution for WordPress Core, we'll continue to enhance and iterate our JavaScript Coding Standards with ESLint.

Thanks to everyone who has tested and explored the world of Prettier with us here, I've had a blast testing this, closing this PR now, thanks again everyone 😄