Skip to content

Commit 2a04bae

Browse files
authored
[lint] Use settings for additional hooks in exhaustive deps (facebook#34637)
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). * __->__ facebook#34637 * facebook#34497
1 parent 92cfdc3 commit 2a04bae

File tree

2 files changed

+108
-4
lines changed

2 files changed

+108
-4
lines changed

packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1485,6 +1485,70 @@ const tests = {
14851485
}
14861486
`,
14871487
},
1488+
{
1489+
// Test settings-based additionalHooks - should work with settings
1490+
code: normalizeIndent`
1491+
function MyComponent(props) {
1492+
useCustomEffect(() => {
1493+
console.log(props.foo);
1494+
});
1495+
}
1496+
`,
1497+
settings: {
1498+
'react-hooks': {
1499+
additionalEffectHooks: 'useCustomEffect',
1500+
},
1501+
},
1502+
},
1503+
{
1504+
// Test settings-based additionalHooks - should work with dependencies
1505+
code: normalizeIndent`
1506+
function MyComponent(props) {
1507+
useCustomEffect(() => {
1508+
console.log(props.foo);
1509+
}, [props.foo]);
1510+
}
1511+
`,
1512+
settings: {
1513+
'react-hooks': {
1514+
additionalEffectHooks: 'useCustomEffect',
1515+
},
1516+
},
1517+
},
1518+
{
1519+
// Test that rule-level additionalHooks takes precedence over settings
1520+
code: normalizeIndent`
1521+
function MyComponent(props) {
1522+
useCustomEffect(() => {
1523+
console.log(props.foo);
1524+
}, []);
1525+
}
1526+
`,
1527+
options: [{additionalHooks: 'useAnotherEffect'}],
1528+
settings: {
1529+
'react-hooks': {
1530+
additionalEffectHooks: 'useCustomEffect',
1531+
},
1532+
},
1533+
},
1534+
{
1535+
// Test settings with multiple hooks pattern
1536+
code: normalizeIndent`
1537+
function MyComponent(props) {
1538+
useCustomEffect(() => {
1539+
console.log(props.foo);
1540+
}, [props.foo]);
1541+
useAnotherEffect(() => {
1542+
console.log(props.bar);
1543+
}, [props.bar]);
1544+
}
1545+
`,
1546+
settings: {
1547+
'react-hooks': {
1548+
additionalEffectHooks: '(useCustomEffect|useAnotherEffect)',
1549+
},
1550+
},
1551+
},
14881552
],
14891553
invalid: [
14901554
{
@@ -3714,6 +3778,40 @@ const tests = {
37143778
},
37153779
],
37163780
},
3781+
{
3782+
// Test settings-based additionalHooks - should detect missing dependency
3783+
code: normalizeIndent`
3784+
function MyComponent(props) {
3785+
useCustomEffect(() => {
3786+
console.log(props.foo);
3787+
}, []);
3788+
}
3789+
`,
3790+
settings: {
3791+
'react-hooks': {
3792+
additionalEffectHooks: 'useCustomEffect',
3793+
},
3794+
},
3795+
errors: [
3796+
{
3797+
message:
3798+
"React Hook useCustomEffect has a missing dependency: 'props.foo'. " +
3799+
'Either include it or remove the dependency array.',
3800+
suggestions: [
3801+
{
3802+
desc: 'Update the dependencies array to be: [props.foo]',
3803+
output: normalizeIndent`
3804+
function MyComponent(props) {
3805+
useCustomEffect(() => {
3806+
console.log(props.foo);
3807+
}, [props.foo]);
3808+
}
3809+
`,
3810+
},
3811+
],
3812+
},
3813+
],
3814+
},
37173815
{
37183816
code: normalizeIndent`
37193817
function MyComponent() {

packages/eslint-plugin-react-hooks/src/rules/ExhaustiveDeps.ts

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ import type {
2121
VariableDeclarator,
2222
} from 'estree';
2323

24+
import { getAdditionalEffectHooksFromSettings } from '../shared/Utils';
25+
2426
type DeclaredDependency = {
2527
key: string;
2628
node: Node;
@@ -69,19 +71,22 @@ const rule = {
6971
},
7072
requireExplicitEffectDeps: {
7173
type: 'boolean',
72-
}
74+
},
7375
},
7476
},
7577
],
7678
},
7779
create(context: Rule.RuleContext) {
7880
const rawOptions = context.options && context.options[0];
81+
const settings = context.settings || {};
82+
7983

8084
// Parse the `additionalHooks` regex.
85+
// Use rule-level additionalHooks if provided, otherwise fall back to settings
8186
const additionalHooks =
8287
rawOptions && rawOptions.additionalHooks
8388
? new RegExp(rawOptions.additionalHooks)
84-
: undefined;
89+
: getAdditionalEffectHooksFromSettings(settings);
8590

8691
const enableDangerousAutofixThisMayCauseInfiniteLoops: boolean =
8792
(rawOptions &&
@@ -93,7 +98,8 @@ const rule = {
9398
? rawOptions.experimental_autoDependenciesHooks
9499
: [];
95100

96-
const requireExplicitEffectDeps: boolean = rawOptions && rawOptions.requireExplicitEffectDeps || false;
101+
const requireExplicitEffectDeps: boolean =
102+
(rawOptions && rawOptions.requireExplicitEffectDeps) || false;
97103

98104
const options = {
99105
additionalHooks,
@@ -1351,7 +1357,7 @@ const rule = {
13511357
node: reactiveHook,
13521358
message:
13531359
`React Hook ${reactiveHookName} always requires dependencies. ` +
1354-
`Please add a dependency array or an explicit \`undefined\``
1360+
`Please add a dependency array or an explicit \`undefined\``,
13551361
});
13561362
}
13571363

0 commit comments

Comments
 (0)