-
Notifications
You must be signed in to change notification settings - Fork 49.6k
[lint] Enable custom hooks configuration for useEffectEvent calling rules #34497
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
Conversation
Comparing: e6f2a8a...ac4ac1f 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) |
…#34492) --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34492). * #34497 * __->__ #34492
…#34492) --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34492). * #34497 * __->__ #34492 DiffTrain build for [e02c173](e02c173)
…#34492) --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34492). * #34497 * __->__ #34492 DiffTrain build for [e02c173](e02c173)
…facebook#34492) --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34492). * facebook#34497 * __->__ facebook#34492
608bc39
to
d08f2e4
Compare
…ules We need to be able to specify additional effect hooks for the RulesOfHooks lint rule in order to allow useEffectEvent to be called by custom effects. ExhaustiveDeps does this with a regex suppplied to the rule, but that regex is not accessible from other rules. This diff introduces a `react-eslint` entry you can put in the eslint settings that allows you to specify custom effect hooks and share them across all rules. This works like: ``` { settings: { 'react-eslint': { additionalEffectHooks: string, }, }, } ``` The next diff allows useEffect to read from the same configuration. ----
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.
Option looks good. Just noticed the EXPERIMENTAL gating which can be handled separately
return false; | ||
} | ||
function isUseEffectEventIdentifier(node: Node): boolean { | ||
if (__EXPERIMENTAL__) { |
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.
Do we need to remove this now? Not sure if there's any other gating like this
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.
Like in the diff below, we can read from the shared configuration to check exhaustive deps. I allow the classic additionalHooks configuration to override it so that this change is backwards compatible. -- --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34637). * __->__ #34637 * #34497
…ules (#34497) We need to be able to specify additional effect hooks for the RulesOfHooks lint rule in order to allow useEffectEvent to be called by custom effects. ExhaustiveDeps does this with a regex suppplied to the rule, but that regex is not accessible from other rules. This diff introduces a `react-hooks` entry you can put in the eslint settings that allows you to specify custom effect hooks and share them across all rules. This works like: ``` { settings: { 'react-hooks': { additionalEffectHooks: string, }, }, } ``` The next diff allows useEffect to read from the same configuration. ---- --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34497). * #34637 * __->__ #34497 DiffTrain build for [92cfdc3](92cfdc3)
…ules (#34497) We need to be able to specify additional effect hooks for the RulesOfHooks lint rule in order to allow useEffectEvent to be called by custom effects. ExhaustiveDeps does this with a regex suppplied to the rule, but that regex is not accessible from other rules. This diff introduces a `react-hooks` entry you can put in the eslint settings that allows you to specify custom effect hooks and share them across all rules. This works like: ``` { settings: { 'react-hooks': { additionalEffectHooks: string, }, }, } ``` The next diff allows useEffect to read from the same configuration. ---- --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/34497). * #34637 * __->__ #34497 DiffTrain build for [92cfdc3](92cfdc3)
We need to be able to specify additional effect hooks for the RulesOfHooks lint rule
in order to allow useEffectEvent to be called by custom effects. ExhaustiveDeps
does this with a regex suppplied to the rule, but that regex is not accessible from
other rules.
This diff introduces a
react-hooks
entry you can put in the eslint settings thatallows you to specify custom effect hooks and share them across all rules.
This works like:
The next diff allows useEffect to read from the same configuration.
Stack created with Sapling. Best reviewed with ReviewStack.