sindresorhus / eslint-plugin-unicorn

More than 100 powerful ESLint rules
MIT License
4.18k stars 362 forks source link

Add `unopinionated` preset #896

Open kripod opened 3 years ago

kripod commented 3 years ago

I propose turning off the following rules by default, so that the recommended config could act as a baseline which may be extended explicitly:

As an alternative, a recommended-loose or recommended-unopinionated config may also be added to avoid the issues outline above. My goal is to set up ESLint with a strict set of rules as simple as below:

{
  "root": true,
  "parserOptions": { "project": "./tsconfig.json" },
  "extends": [
    "airbnb-typescript",
    "airbnb/hooks",
    "plugin:@typescript-eslint/recommended",
    "plugin:unicorn/recommended",
    "prettier",
    "prettier/@typescript-eslint",
    "prettier/react",
    "prettier/unicorn"
  ]
}
sindresorhus commented 3 years ago

I'm open to adding an unopinionated config, but let's see what the other maintainers here think. I don't think it should include the word recommended though, as it's not actually something we recommend.


unicorn/no-nested-ternary – Doesn't work properly when used along with Prettier, as formatting removes parentheses used in ternary expressions.

This is not about being opinionated though. We are not going to leave out rules just because they don't work with Prettier. This is already handled by https://github.com/prettier/eslint-config-prettier/blob/07380e2235cea405822b7c0de20bbecbf2dc6da0/unicorn.js#L5.


unicorn/no-nested-ternary – Doesn't work properly when used along with Prettier, as formatting removes parentheses used in ternary expressions.

We already handle some React cases and we're happy to handle more, but yes, this one is quite opinionated.


unicorn/prefer-query-selector – The alternatives, namely getElementById and getElementsByClassName, offer better performance. Selector-based queries should be avoided in general, even in CSS, as they get complex pretty fast. Using IDs and class names for locating elements is a well-tested practice, while complex selectors are questionable.

You can select classes and IDs with query selector too. I remember reading something about engines just using the fast-path internally for simple selectors.


While the intent is great, way too many false positives may occur. For instance, React uses the term props to identify attributes passed to a JSX element. They aren't widely called properties in the documentation and tutorials. Same goes for the req and res parameters of an API handler function, as popularized by Express.

I can buy the React argument as there are many places you have to use the props wording (Although, here too we have workarounds for React). But in the case of Express, it just leads to less readable code.

kripod commented 3 years ago

I would highly appreciate a separate unopinionated or loose config if that’s a possibility. Thanks for taking your time to read and review my suggestions! 🙏

kripod commented 3 years ago

@sindresorhus Is there anything I could help with to advance the state of this proposal?

sindresorhus commented 3 years ago

@fisker Are you ok with this?

fisker commented 3 years ago

Yes, I think it's good.

fregante commented 3 years ago

Isn't every rule potentially "opinionated"? I think you're looking at a airbnb-specific unicorn configuration, like a eslint-config-airbnb-unicorn package like the many that exist for XO: https://github.com/xojs/xo#configs

Or, if worth having in this package, perhaps it should explicitly mention airbnb: plugin:unicorn/airbnb

sindresorhus commented 3 years ago

Isn't every rule potentially "opinionated"?

No. Some rules are about correctness or preventing bugs. I don't consider that opinionated. I also wouldn't consider rules that enforce using more modern APIs opinionated.

For example, no-instanceof-array is about correctness, but no-array-reduce is opinionated.

Or, if worth having in this package, perhaps it should explicitly mention airbnb: plugin:unicorn/airbnb

I don't see why it should have the name of a random ESLint config. unopinionated is perfectly clear.

osdiab commented 3 years ago

Would also be nice if there were some preset that disables the rules that conflict with prettier, as that was pretty annoying.

fisker commented 3 years ago

@osdiab https://github.com/prettier/eslint-config-prettier/blob/83c4f374f06d0cb1c0e55bc3f3b1ab75a1a34a89/index.js#L142-L144

osdiab commented 3 years ago

Ah I was on an old version, thanks for this!

Omar

On Thu, May 27, 2021 at 9:58 AM, fisker Cheung @.***> wrote:

@osdiab https://github.com/osdiab https://github.com/prettier/ eslint-config-prettier/blob/83c4f374f06d0cb1c0e55bc3f3b1ab75a1a34a89/ index.js#L142-L144

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/sindresorhus/eslint-plugin-unicorn/issues/896#issuecomment-849228606, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAONU3XEAUVQPYLM5UWPMVTTPWKLTANCNFSM4TEMYEDA .

quirimmo commented 2 years ago

Hey guys, don't trust the guy, you are doing a great job, don:t reduce all to be kind of an abbreviation.

quirimmo commented 2 years ago

dont really waste time replying to such comments, please focus working on amazing rules like no set duplicates

quirimmo commented 2 years ago

another point: how would you suggest me to loop over a list without using alll native methods? shall I perhaps access statically all the member by indexing?

andersk commented 2 years ago

You can select classes and IDs with query selector too. I remember reading something about engines just using the fast-path internally for simple selectors.

Evidence to the contrary, showing a significant performance advantage for getElementById("foo") over querySelector("#foo") in both Firefox and Chrome:

https://blog.wesleyac.com/posts/getelementbyid-vs-queryselector

Also, correctly selecting a computed ID or class name with querySelector may require a CSS.escape.

levino commented 1 year ago

If someon wants to switch off the rules from OP, here is a snippet to copy and paste:

{
  'unicorn/prevent-abbreviations': 'off',
  'unicorn/filename-case': 'off',
  'unicorn/no-nested-ternary': 'off',
  'unicorn/no-null': 'off',
  'unicorn/no-useless-undefined': 'off',
  'unicorn/prefer-query-selector': 'off',
}
RicardoValdovinos commented 1 year ago

Hello, wondering if there were any updates on an unopinionated config setting?

sindresorhus commented 7 months ago

We will add this when we have fully switched to flat config. I don't want to expose more configs until then.