mastodon icon indicating copy to clipboard operation
mastodon copied to clipboard

Remove `eslint-plugin-prettier`

Open renchap opened this issue 1 year ago • 4 comments

It's usage is no longer recommended as ESlint should not be used anymore for any formatting rules.

More details in this article: https://www.joshuakgoldberg.com/blog/you-probably-dont-need-eslint-config-prettier-or-eslint-plugin-prettier/

This change should speed up ESLint, and formatting issues will no longer show as ESLint warnings when editing, which often happens when developping. It is recommended to configure your editor to format using Prettier on auto-save, otherwise files will be fixed automatically before commit, or when yarn fix:js is ran.

The main change here is in the lint/fix scripts, as Prettier needs to be called separately.

renchap avatar Jan 22 '24 21:01 renchap

@renchap if I'm understanding this correctly, this just moves to using prettier directly and doesn't involve any reformatting of code or anything like that? If so, this wouldn't negatively impact streaming.

The only hold on streaming is anything that modifies the code of the .js files, since that'd create a bunch of merge conflicts on PRs just pending review & merge.

ThisIsMissEm avatar Jan 23 '24 00:01 ThisIsMissEm

I am wondering if we should drop Prettier from all the lint/fix commands, and use a format command that runs Prettier on the whole repo. Files ignored from Prettier would not be changed, and every other supported file will be formatted at once.

Main advantage is that we no longer would have && in scripts, which is not working nice when you want to add options to a script (let's say yarn fix --verbose).

Main drawback is that you would need to run 2 commands to fix issues (yarn fix && yarn format).

As Prettier is ran automatically on every changed file in git pre-commit, maybe it is not a real issue?

renchap avatar Jan 23 '24 10:01 renchap

@renchap lint-staged already calls prettier on every file from what I can tell? But I'd be in favour of a format:check and format:fix

ThisIsMissEm avatar Jan 23 '24 12:01 ThisIsMissEm

This pull request has merge conflicts that must be resolved before it can be merged.

github-actions[bot] avatar Feb 21 '24 22:02 github-actions[bot]

Updated, introduced yarn format and yarn format:check in a second commit. This allows to remove all the scripts and workflow that ran Prettier on specific extensions and were wasteful.

renchap avatar Feb 24 '24 09:02 renchap

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 85.10%. Comparing base (86500e3) to head (c89fee1). Report is 173 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #28851      +/-   ##
==========================================
+ Coverage   85.01%   85.10%   +0.08%     
==========================================
  Files        1059     1062       +3     
  Lines       28277    28352      +75     
  Branches     4538     4548      +10     
==========================================
+ Hits        24040    24129      +89     
+ Misses       3074     3061      -13     
+ Partials     1163     1162       -1     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Feb 24 '24 09:02 codecov[bot]

This pull request has resolved merge conflicts and is ready for review.

github-actions[bot] avatar Feb 24 '24 10:02 github-actions[bot]

This pull request has merge conflicts that must be resolved before it can be merged.

github-actions[bot] avatar Feb 27 '24 15:02 github-actions[bot]