react-native icon indicating copy to clipboard operation
react-native copied to clipboard

Remove eslint-plugin-prettier from eslint-config-react-native

Open LukaRusadze opened this issue 1 year ago • 5 comments

Summary:

Citing Prettier docs:

Linters usually contain not only code quality rules, but also stylistic rules. Most stylistic rules are unnecessary when using Prettier, but worse – they might conflict with Prettier! Use Prettier for code formatting concerns, and linters for code-quality concerns, as outlined in Prettier vs. Linters.

This plugin was especially useful when Prettier was new. 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!
  • They are slower than running Prettier directly.
  • They’re yet one layer of indirection where things may break.

This PR aims to remove eslint-plugin-prettier from eslint-config-react-native. This will offload code stylistic rules to prettier from eslint. This will also remove all of the eslint(prettier/prettier) warnings and errors while typing which gets fixed by formatting with prettier but gets quickly annoying until you save to format.

I'd also go as far as to remove some/all of the styling ESLint rules and move everything to Prettier config instead (if they aren't already in it) so linter doesn't do formatter's job.

image

Changelog:

  • [General] [Removed] Remove eslint-plugin-prettier in eslint-config-react-native
  • [General] [Changed] Replace 'plugin:prettier/recommended' with 'prettier' in eslint-config-react-native

Test Plan:

Before: image

After: image

This was tested by making the proposed changes in node_modules of a newly initialised project

LukaRusadze avatar Dec 10 '23 18:12 LukaRusadze

Hi @LukaRusadze!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

facebook-github-bot avatar Dec 10 '23 18:12 facebook-github-bot

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 18,881,799 -8
android hermes armeabi-v7a n/a --
android hermes x86 n/a --
android hermes x86_64 n/a --
android jsc arm64-v8a 22,250,435 -6
android jsc armeabi-v7a n/a --
android jsc x86 n/a --
android jsc x86_64 n/a --

Base commit: 0c02b26c1e00ca27d7a3ba4244a0957d4cb473aa Branch: main

analysis-bot avatar Dec 10 '23 18:12 analysis-bot

Adding on top of the Prettier docs, this stackoverflow post also provides more context of this as well as points to the recently published post from ESLint on deprecating formatting rules.

This seems a reasonable change to me, but it might miss reporting formatting issues if someone using RN doesn't have prettier setup in their IDE/dev environment, which could be a regression (although less severe to production regressions).

I think we should send a deprecating message first before removing this.

ryancat avatar Dec 11 '23 19:12 ryancat

Besides, could you help with identifying the gaps after we removing eslint-plugin-prettier from a user's perspective? For example, what might stop working with the change, and how to recover/fix them?

This would help us find a better way roll out the change without causing surprises.

ryancat avatar Dec 11 '23 21:12 ryancat

This could be a migration issue for those who check code style validity with eslint . in CI, manually, or some other place. The can be replaced with prettier --check .

Also your point of people not having prettier installed is valid but I don't think it's an issue. I'd say, in general, Prettier's already mostly adopted and every project, where it's necessary, has it enforced. The adoption issue can quickly be caught be a CI process as well.

I've seen a few developers disable ESLint altogether because of the styling errors/warnings you get while writing code, something that gets fixed the moment you save/format.

This is just the eslint-plugin-prettier removal PR atm but I'd go as far as to remove all styling related ESLint rules (prettier rules) and rely solely on Prettier when it comes to styling.

LukaRusadze avatar Dec 12 '23 21:12 LukaRusadze

Closed by https://github.com/facebook/react-native/commit/727f30bd0b27ff168e6a6556a9ffbc7e42dbb8f0.

yungsters avatar Apr 04 '24 06:04 yungsters