-
Notifications
You must be signed in to change notification settings - Fork 50.2k
Add reporting modes for react-hooks/exhaustive-effect-dependencies and temporarily enable #35365
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
Add reporting modes for react-hooks/exhaustive-effect-dependencies and temporarily enable #35365
Conversation
| description: | ||
| 'Validates that effect dependencies are exhaustive and without extraneous values', | ||
| preset: LintRulePreset.Off, | ||
| preset: LintRulePreset.Recommended, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
technically this shouldn't be needed as we don't use presets internally, but okay to leave in if we plan to ship.
if you choose to leave it, you’ll need to manually add the rule to the eslint config for this fixture
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I only changed this because the rule noops in the eslint-v9 fixture with it off. But if we can control it separately internally I can disable the fixture test while we experiment.
|
Comparing: bcf97c7...61f0cb3 Critical size changesIncludes critical production bundles, as well as any change greater than 2%:
Significant size changesIncludes any change greater than 0.2%: (No significant changes) |
…d temporarily enable (#35365) `react-hooks/exhaustive-effect-dependencies` from `ValidateExhaustiveDeps` reports errors for both missing and extra effect deps. We already have `react-hooks/exhaustive-deps` that errors on missing dependencies. In the future we'd like to consolidate this all to the compiler based error, but for now there's a lot of overlap. Let's enable testing the extra dep warning by splitting out reporting modes. This PR - Creates `on`, `off`, `missing-only`, and `extra-only` reporting modes for the effect dep validation flag - Temporarily enables the new rule with `extra-only` in `eslint-plugin-react-hooks` - Adds additional null checking to `manualMemoLoc` to fix a bug found when running against the fixture DiffTrain build for [88ee1f5](88ee1f5)
…d temporarily enable (#35365) `react-hooks/exhaustive-effect-dependencies` from `ValidateExhaustiveDeps` reports errors for both missing and extra effect deps. We already have `react-hooks/exhaustive-deps` that errors on missing dependencies. In the future we'd like to consolidate this all to the compiler based error, but for now there's a lot of overlap. Let's enable testing the extra dep warning by splitting out reporting modes. This PR - Creates `on`, `off`, `missing-only`, and `extra-only` reporting modes for the effect dep validation flag - Temporarily enables the new rule with `extra-only` in `eslint-plugin-react-hooks` - Adds additional null checking to `manualMemoLoc` to fix a bug found when running against the fixture DiffTrain build for [88ee1f5](88ee1f5)
react-hooks/exhaustive-effect-dependenciesfromValidateExhaustiveDepsreports errors for both missing and extra effect deps. We already havereact-hooks/exhaustive-depsthat errors on missing dependencies. In the future we'd like to consolidate this all to the compiler based error, but for now there's a lot of overlap. Let's enable testing the extra dep warning by splitting out reporting modes.This PR
on,off,missing-only, andextra-onlyreporting modes for the effect dep validation flagextra-onlyineslint-plugin-react-hooksmanualMemoLocto fix a bug found when running against the fixture