8000 [eslint-plugin-react-hooks] Add `ignoredHooks` to `react-hooks/exhaustive-deps` options by exoRift · Pull Request #33203 · facebook/react · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

[eslint-plugin-react-hooks] Add ignoredHooks to react-hooks/exhaustive-deps options #33203

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed
+27 −0

Conversation

exoRift
Copy link
@exoRift exoRift commented May 14, 2025

Summary

Add an ignoredHooks option to exhaustive-deps. This allows the disabling of the rule on specific hooks such as useEffect or anything that matches the pattern. It works in the same way that additionalHooks does and overrides additionalHooks.

I created this option to disable the rule for useEffect. A lot of the times, useEffect is using states/variables that I don't want it to retrigger on; it's only supposed to listen for specific changes. These warnings can clutter the lint output and prevent real issues from being spotted.

How did you test this change?

I added a test that ignores useEffect and checks that there are no issues reported

@facebook-github-bot
Copy link

Hi @exoRift!

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 cla@meta.com. Thanks!

@kushxg kushxg mentioned this pull request May 14, 2025
Open
@facebook-github-bot
Copy link

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

@poteto
Copy link
Member
poteto commented May 15, 2025 •
edited
Loading

Thanks!

I created this option to disable the rule for useEffect. A lot of the times, useEffect is using states/variables that I don't want it to retrigger on; it's only supposed to listen for specific changes. These warnings can clutter the lint output and prevent real issues from being spotted.

Please note that this breaks the rules of React and so wouldn't make sense to be a part of eslint-plugin-react-hooks. useEffect is primarily meant for synchronization, not for triggering arbitrary side effects when some state changes. The best mental model to have is that effects run on every render, every time anything changes. The dependency array is a performance optimization, no need to do the work if we know that values not used by the effect have changed. It is not meant to be used for "do this when x changes" logic.

We have a guide written in more detail on how to address this. Thanks again for opening the PR!

@poteto poteto closed this May 15, 2025
@exoRift
Copy link
Author
exoRift commented May 15, 2025 •
edited
Loading

Thanks!

I created this option to disable the rule for useEffect. A lot of the times, useEffect is using states/variables that I don't want it to retrigger on; it's only supposed to listen for specific changes. These warnings can clutter the lint output and prevent real issues from being spotted.

Please note that this breaks the rules of React and so wouldn't make sense to be a part of eslint-plugin-react-hooks. useEffect is primarily meant for synchronization, not for triggering arbitrary side effects when some state changes. The best mental model to have is that effects run on every render, every time anything changes. The dependency array is a performance optimization, no need to do the work if we know that values not used by the effect have changed. It is not meant to be used for "do this when x changes" logic.

We have a guide written in more detail on how to address this. Thanks again for opening the PR!

I understand where you're coming from, however I think that option to be left up to the user, especially where linting is concerned. A linter is a tool that a developer uses according to their needs which is why rules in many plugins come with configurations rather than enforcing the plugin author's view of what's correct.

I don't think de-facto use of the hook should be punished

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
CLA Signed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@exoRift @facebook-github-bot @poteto
0