Add prettier support

[kentcdodds]

Issue Type

Feature Request

Issue Description

prettier is a great new package by @jlongster that will automatically format your code. It's really quite impressive! And I really want to use it. However I prefer to omit semicolons and @jlongster would prefer to avoid adding the complexity at this time (understandably). I was planning on doing what @dtinth did by creating my own formatter that uses ESLint rather than Standard, but realized that it would even better if I could just add prettier to this package!

So, I plan on making a fork to do this, and will be making a pull request to add the configuration option to enable prettier (disabled by default). If you have any tips on where/how to add the code, I'd love to hear them! Thanks

[dtinth]

I had a few experience hacking Atom, so I may be able to give some pointer.

• When you run “Fix file” command in linter-eslint, it invokes the linter-eslint:fix-file command. There’s also a save handler.

• For configuration there’s configSchema in package.json.

• Both command handler and save handler passes message to the worker to fix a file.

• The fix job modifies the file in-place. As a first, easiest step (just to get it to work), you can maybe prettier-ify the file in-place before invoking ESLint. But this causes the file to be written to disk twice (also might cause Atom to reload the file twice).

• To make it right might require changing how ESLint is invoked.

[IanVS]

I think that executing prettier is out-of-scope for this plugin. linter-eslint exists solely to bring ESLint functionality into Atom.

That said, it might be possible to create a separate atom plugin to run prettier in atom's onWillSave hook, and only save out the changed/formatted buffer to disk. Can prettier accept text, or does it need to read a file from disk? As @dtinth mentioned, linter-eslint has to use the onDidSave hook, because eslint needs to read the file from disk during --fix, it can't accept straight text for that.

I think that having a separate prettier plugin will have other benefits as well. For example, it could be used by people who only want to use prettier without ESLint. And a single package would need to be updated if/when prettier's api changes. Let's see if we can find a way to get prettier and eslint to work together, but I don't think this package is the place to do it.

[IanVS]

Actually, it looks like prettier-eslint already uses onWillSave. I assume you've tried using it together with linter-eslint, and they are not compatible?

[kentcdodds]

Can prettier accept text, or does it need to read a file from disk?

It accepts text!

it might be possible to create a separate atom plugin to run prettier in atom's onWillSave hook

This actually gave me an idea! There's an existing plugin. Upon looking at the code, I found that they're actually already using onWillSave! So I installed it and tried it out! It's kinda wonky to see my text get updated a bunch (there's a flash of semicolons), but it works alright!

I wonder if there's a way that we could get rid of that flash of semicolons everytime I save though...

[kentcdodds]

I'm thinking the best thing to get rid of that flash of semicolons would be to integrate ESLint and prettier with onWillSave. Any chance we could work on that?

[IanVS]

Great, I'm glad it works!

I doubt there's a way to avoid the flash of changes. After all, what's going on is prettier is changing the text in the editor, then the file is saved to disk, then linter-eslint tells eslint to run the file through --fix, and when eslint is finished atom notices that the file has changed on disk, and updates the text buffer in the editor.

Yes, it could be more seamless if the output of prettier could somehow be piped directly to eslint before changing the text buffer. But, aside from the problems I mentioned with that being out-of-scope for linter-eslint, eslint cannot run --fix on text that is piped into eslint (see http://eslint.org/docs/user-guide/command-line-interface#fix).

[kentcdodds]

Hmm... Could we not use the CLIEngine interface in node? Specifically executeOnText?

[Arcanemagus]

A rewrite of linter-eslint to use the CLIEngine has been planned for a while. Even when we have moved to that though I still feel the type of integration you are asking for is beyond what the intent of this package is since the goal here is to provide ESLInt in Atom, not ESLint + however many random additional programs people can think of requesting.

[IanVS]

As @Arcanemagus mentioned, we have talked about refactoring linter-eslint to use CLIEngine, but nobody has made any attempts so far. There were some challenges (I don't recall what, exactly) which drove @steelbrain to choose to avoid it, but it's worth another look to see if things have changed. I think it's worth opening an issue and trying it out.

Even if we do switch to CLIEngine, there is no way to coordinate between atom plugins, to ensure that the prettier changes are made before the eslint fixes (or vice-versa). And I'm in agreement with @Arcanemagus that we wouldn't want to add prettier directly to this package. What we really need is some kind of atom meta-plugin which can coordinate the execution order of other plugins.

In light of this, I'm going to close the issue. The good news is that at least the prettier and eslint plugins can inter-operate for now. If we do switch to CLIEngine though, all bets are off. ;)

[kentcdodds]

FYI for anyone coming after, I'm currently working on prettier-eslint-atom which will make things much more smooth:

Simply disable --fix on linter-eslint and use this plugin. There are still some kinks (and simple perf optimizations) to work out, but I think it's a pretty solid concept.

[Arcanemagus]

Just to be clear, you mean "don't enable" since it's not enabled by default 😉.