Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFC]: remove use of eslint-plugin-prettier #50

Open
antoine-cottineau opened this issue Aug 29, 2023 · 10 comments
Open

[RFC]: remove use of eslint-plugin-prettier #50

antoine-cottineau opened this issue Aug 29, 2023 · 10 comments
Labels
⭐ enhancement New feature or request 📏 eslint-plugin Issue concerning the eslint plugin

Comments

@antoine-cottineau
Copy link
Collaborator

Why?

I find that having integrated eslint-plugin-prettier into the package deteriorates the dev experience and adds useless complexity to the esLint config.

By activating this plugin, formatting errors that are found by Prettier are now handled by esLint. This means that whenever you have a missing space or a line that needs to be split up you get those red squiggly lines in the editor, which creates some unnecessary pollution in the mind of the developper.

Moreover, the official Prettier documentation advises against using linter to format the code:

When searching for both Prettier and your linter on the Internet you’ll probably find more related projects. These are generally not recommended, but can be useful in certain circumstances.

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!
They are slower than running Prettier directly.
They’re yet one layer of indirection where things may break.

In my opinion, a better solution would be to have:

  • formatOnSave enabled in the editor so that the dev doesn't have to care about formatting and it is done automatically,
  • a script in package.json that can prettify the whole codebase (prettier -w .),
  • a script in package.json that errors if some files are not formatted correctly, which can be useful in a pre-push hook or in a CI (prettier -c .).

Rule Documentation 📜

https://github.com/prettier/eslint-plugin-prettier

Config Selection 🛠

recommended

(Optional) Additional Details 📝

No response

@antoine-cottineau antoine-cottineau added ⭐ enhancement New feature or request 📏 eslint-plugin Issue concerning the eslint plugin labels Aug 29, 2023
@pierrezimmermannbam
Copy link
Contributor

The reason I find it useful to have this as an eslint plugin is because I have autosave enabled in vscode so I don't necessarily do cmd + s to save my files which means I may forget to format them. In the past when working on projects where this wasn't in eslint config I frequently had errors for non formatted files.
That being said I know there aren't many users of auto save so my case is rather marginal

@matthieugicquel
Copy link
Contributor

In theory this would be better to do this. But last time I tried I couldn't get it to work properly in VS Code without resorting to unofficial extensions like rvest.vs-code-prettier-eslint.

Maybe this is solved now and with the right VS Code config everything works fine, even when eslint has to apply autofixes?

@antoine-cottineau
Copy link
Collaborator Author

@pierrezimmermannbam Hmm I understand, what about having a pre-commit hook that formats your code so you don't forget?

@matthieugicquel you mean you couldn't properly format the code with prettier + vscode extension for prettier + format on save?
If you don't have any formatting rules in your eslint config that conflicts with prettier, I don't think you would have issues. This is the setup I have on my side projects and it works well.

@pierrezimmermannbam
Copy link
Contributor

@antoine-cottineau I'm really not a big fan of pre-commit hooks, it's too late in your dev flow and in the end everybody ends up comitting with no-verify because it's super annoying. Formatting is rather fast though so in this case it seems reasonable. Another downside is it's one more thing to setup to have a correct dev environment

@matthieugicquel
Copy link
Contributor

@antoine-cottineau I mean that when I have the config you described + an eslint config with autofixes (not necessarily rules that conflict, but rules that change the formatting too), I couldn't get VS code to reliably and deterministically run the eslint autofixes first and then prettier after, which is necessary to get correctly formatted and non-broken code.

@antoine-cottineau
Copy link
Collaborator Author

@pierrezimmermannbam I agree with you for the pre-commit hook but I would make an exception for formatting as you don't really need it early in your dev flow.

@matthieugicquel Oh okay that's a shame. In that case I understand that having esLint rules for syncing with Prettier is maybe the best solution, even if I think it's not a good one 😅

@delphinebugner
Copy link

I have an other issue by using eslint as formatter: we just installed the plugin on TF1 Info, and simply the formatter does not work anymore >< With error message : “FormattingExtension 'ESLint' is configured as formatter but it cannot format 'TypeScript'-files” or TypeScript JSX file for the .tsx.

It happened already on the past : we could not prettify our fixtures, which occured to be in the "eslintignore" file
But this time, all of our ts and tsx file are simply not prettified anymore, and they are not in eslintignore.

I haven't investigate further, I will temporarly go back to the classic prettier formatter, I'll let you know if I understand what happens - or let me know if you have an other recommandation on its usage or not with BAM eslint plugin!

@delphinebugner
Copy link

Update : all I needed to do was to restart VSCode after installing the BAM eslint plugin (source: https://stackoverflow.com/a/72681182 - it makes sense, actually)
Then the formatting reworked!

@jcalixte
Copy link
Contributor

jcalixte commented Sep 21, 2023

Hi there! I wanted to share with you what the creator of vitest thinks about formatting in linting. I couldn't have a solid opinion about it.

@pierpo
Copy link
Member

pierpo commented Dec 12, 2023

So, I am totally in favor of integrating Prettier into ESLint. I generally recommend completely ditching Prettier and relying on ESLint with auto-fix on save. Why? Because it's the best solution I've found in recent years to solve the countless problems I've seen with people's VSCode configurations. Issues like the vscode prettier extension acting up, forgotten eslint rules (I don't know).

My experience: one tool = one single source of truth = no more hassle.

Then again, maybe the VSCode Prettier extension has stopped messing around, but it drove me so crazy that I just stopped trying 😛 Also, we want to check the format with ESLint anyway (unless there's a “prettier check” mode?). So, we will need to run ESLint at some point anyway.

Edit: looks like there's a prettier -c now as indicated in the PR.
Completely disabling the formatting rules in ESLint and setting up prettier might work properly then. 🤷
If people have tried it with no issues, then I'm OK with the change and I'll give it a try 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⭐ enhancement New feature or request 📏 eslint-plugin Issue concerning the eslint plugin
Projects
None yet
Development

No branches or pull requests

6 participants