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

Make https://biomejs.dev/linter/rules/use-exhaustive-dependencies/ not recommended #4293

Open
Tracked by #3727
ematipico opened this issue Oct 14, 2024 · 6 comments
Open
Tracked by #3727
Assignees

Comments

@ematipico
Copy link
Member

No description provided.

@ematipico ematipico self-assigned this Oct 14, 2024
@arendjr
Copy link
Contributor

arendjr commented Oct 14, 2024

I don't think this is a good idea, since it would be a regression for React users that rely on the rule for enforcing the rules of hooks. However, we're in the process of adding a reportUnnecessaryDependencies option that I think does make sense to disable by default for 2.0. See the dicussion here: #630 (comment)

@ematipico
Copy link
Member Author

ematipico commented Oct 14, 2024

It is a regression, and that's why it's part of v2.0. The reason why we should not recommend it:

  • this is a rule for react and react native projects, and people that don't use react are going to use a rule that's not needed in their projects
  • the other rule isn't recommended https://biomejs.dev/linter/rules/use-hook-at-top-level/, this is a bit inconsistent
  • we are starting to have rules that work on other frameworks (solidjs), and I think library/framework rules should be opt-in

@arendjr
Copy link
Contributor

arendjr commented Oct 14, 2024

Are we sure we will have a good story on enabling framework-specific rules then? A large part of the appeal of Biome is that it works out-of-the-box without too much hassle configuration-wise, so if people need to look themselves which rules apply to their libraries/frameworks and enable them manually, that sounds like a red flag.

I remember there was some discussion at some point about revamping the linter config, where we could add better categories to rules. Maybe we should focus on that first?

Btw, I would argue useHookAtTopLevel should be enabled by default too, at least for React users. So maybe we need a way to detect (maybe during biome init?) which frameworks to enable rules for...

It is a regression, and that's why it's part of v2.0.

Major releases are for breaking changes, not for regressions ;)

@Conaclos
Copy link
Member

I remember there was some discussion at some point about revamping the linter config, where we could add better categories to rules. Maybe we should focus on that first?

I plan to revisit it for Biome 2.0

@ematipico
Copy link
Member Author

A large part of the appeal of Biome is that it works out-of-the-box without too much hassle configuration-wise, so if people need to look themselves which rules apply to their libraries/frameworks and enable them manually, that sounds like a red flag.

You and I both agree. Since the scope Biome is slowly increasing, by spanning over other realities such as Next.js and Solid, we need to revisit our approach of "working out of the box"

We can opt-in certain rules by checking the project manifests, and look for specific dependencies e.g. react, next, solid and more

I remember there was some discussion at some point about revamping the linter config, where we could add better categories to rules. Maybe we should focus on that first?

And that's why we have this discussion where we proposed projects/linter.domains:

#689 (comment)

@arendjr
Copy link
Contributor

arendjr commented Oct 14, 2024

I plan to revisit it for Biome 2.0

Glad to hear it!

We can opt-in certain rules by checking the project manifests, and look for specific dependencies e.g. react, next, solid and more

Cool, then I'm totally on board yeah :) Just wanted to make sure we don't leave that out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants