neovim / node-client

Nvim Node.js client and plugin host
https://neovim.io/node-client/
MIT License
492 stars 53 forks source link

chore: remove unneeded prettier eslint plugin #437

Closed mikavilpas closed 3 weeks ago

mikavilpas commented 3 weeks ago

Issue

Prettier rules seem to be enforced both via prettier itself, and in addition by eslint via eslint-plugin-prettier.

Prettier docs state the following recommendation:

When searching for both Prettier and your linter on the Internet you’ll probably find more related projects. These are generally not recommended[...]

By running Prettier inside your linters, you didn’t have to set up any new infrastructure and you could re-use your editor integrations for the linters. But these days you can run prettier --check . and most editors have Prettier support.

The downsides of those plugins are:

  • You end up with a lot of red squiggly lines in your editor, which gets annoying. Prettier is supposed to make you forget about formatting – and not be in your face about it!

https://prettier.io/docs/en/integrating-with-linters.html

Solution

Remove eslint-plugin-prettier from the project, and continue to use prettier as a standalone tool. This removes the red squiggly lines in the editor.

justinmk commented 3 weeks ago

Minimizing dependencies is definitely appreciated.

However in this case I don't understand what problem is being solved. In this project, the prettier rules are also enforced "lint" rules. That's intentional. I don't really give a flip about red squiggles, and I don't understand why that is undesirable anyways--because again, prettier rules are lint rules.

mikavilpas commented 3 weeks ago

Oh, I did not realize that was the case. In my experience, usually prettier is run with an editor plugin, and on the command line in CI, while being run as part of eslint is an older way and nowadays not recommended.

If running as part of eslint is a good fit for this project, I think with the same reasoning https://github.com/neovim/node-client/pull/435 can also be closed. It's about some prettier errors that appear when you try to run pretttier on the command line.

justinmk commented 1 week ago

while being run as part of eslint is an older way and nowadays not recommended.

Ok, reviewing https://prettier.io/docs/en/integrating-with-linters.html , I guess I missed that.

But this PR doesn't add prettier --check, which means we are back to manually checking that prettier rules are enforced.

I'd be in favor of removing the plugin as you did here, if we have: