Skip to content

Commit 92cfdc3

Browse files
authored
[lint] Enable custom hooks configuration for useEffectEvent calling rules (#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
1 parent a55e98f commit 92cfdc3

File tree

3 files changed

+110
-3
lines changed

3 files changed

+110
-3
lines changed

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

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -581,6 +581,27 @@ const allTests = {
581581
};
582582
`,
583583
},
584+
{
585+
code: normalizeIndent`
586+
// Valid: useEffectEvent can be called in custom effect hooks configured via ESLint settings
587+
function MyComponent({ theme }) {
588+
const onClick = useEffectEvent(() => {
589+
showNotification(theme);
590+
});
591+
useMyEffect(() => {
592+
onClick();
593+
});
594+
useServerEffect(() => {
595+
onClick();
596+
});
597+
}
598+
`,
599+
settings: {
600+
'react-hooks': {
601+
additionalEffectHooks: '(useMyEffect|useServerEffect)',
602+
},
603+
},
604+
},
584605
],
585606
invalid: [
586607
{
@@ -1353,6 +1374,39 @@ const allTests = {
13531374
`,
13541375
errors: [tryCatchUseError('use')],
13551376
},
1377+
{
1378+
code: normalizeIndent`
1379+
// Invalid: useEffectEvent should not be callable in regular custom hooks without additional configuration
1380+
function MyComponent({ theme }) {
1381+
const onClick = useEffectEvent(() => {
1382+
showNotification(theme);
1383+
});
1384+
useCustomHook(() => {
1385+
onClick();
1386+
});
1387+
}
1388+
`,
1389+
errors: [useEffectEventError('onClick', true)],
1390+
},
1391+
{
1392+
code: normalizeIndent`
1393+
// Invalid: useEffectEvent should not be callable in hooks not matching the settings regex
1394+
function MyComponent({ theme }) {
1395+
const onClick = useEffectEvent(() => {
1396+
showNotification(theme);
1397+
});
1398+
useWrongHook(() => {
1399+
onClick();
1400+
});
1401+
}
1402+
`,
1403+
settings: {
1404+
'react-hooks': {
1405+
additionalEffectHooks: 'useMyEffect',
1406+
},
1407+
},
1408+
errors: [useEffectEventError('onClick', true)],
1409+
},
13561410
],
13571411
};
13581412

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

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import type {
2020

2121
// @ts-expect-error untyped module
2222
import CodePathAnalyzer from '../code-path-analysis/code-path-analyzer';
23+
import { getAdditionalEffectHooksFromSettings } from '../shared/Utils';
2324

2425
/**
2526
* Catch all identifiers that begin with "use" followed by an uppercase Latin
@@ -147,8 +148,23 @@ function getNodeWithoutReactNamespace(
147148
return node;
148149
}
149150

150-
function isEffectIdentifier(node: Node): boolean {
151-
return node.type === 'Identifier' && (node.name === 'useEffect' || node.name === 'useLayoutEffect' || node.name === 'useInsertionEffect');
151+
function isEffectIdentifier(node: Node, additionalHooks?: RegExp): boolean {
152+
const isBuiltInEffect =
153+
node.type === 'Identifier' &&
154+
(node.name === 'useEffect' ||
155+
node.name === 'useLayoutEffect' ||
156+
node.name === 'useInsertionEffect');
157+
158+
if (isBuiltInEffect) {
159+
return true;
160+
}
161+
162+
// Check if this matches additional hooks configured by the user
163+
if (additionalHooks && node.type === 'Identifier') {
164+
return additionalHooks.test(node.name);
165+
}
166+
167+
return false;
152168
}
153169
function isUseEffectEventIdentifier(node: Node): boolean {
154170
if (__EXPERIMENTAL__) {
@@ -169,8 +185,23 @@ const rule = {
169185
recommended: true,
170186
url: 'https://react.dev/reference/rules/rules-of-hooks',
171187
},
188+
schema: [
189+
{
190+
type: 'object',
191+
additionalProperties: false,
192+
properties: {
193+
additionalHooks: {
194+
type: 'string',
195+
},
196+
},
197+
},
198+
],
172199
},
173200
create(context: Rule.RuleContext) {
201+
const settings = context.settings || {};
202+
203+
const additionalEffectHooks = getAdditionalEffectHooksFromSettings(settings);
204+
174205
let lastEffect: CallExpression | null = null;
175206
const codePathReactHooksMapStack: Array<
176207
Map<Rule.CodePathSegment, Array<Node>>
@@ -726,7 +757,7 @@ const rule = {
726757
// Check all `useEffect` and `React.useEffect`, `useEffectEvent`, and `React.useEffectEvent`
727758
const nodeWithoutNamespace = getNodeWithoutReactNamespace(node.callee);
728759
if (
729-
(isEffectIdentifier(nodeWithoutNamespace) ||
760+
(isEffectIdentifier(nodeWithoutNamespace, additionalEffectHooks) ||
730761
isUseEffectEventIdentifier(nodeWithoutNamespace)) &&
731762
node.arguments.length > 0
732763
) {
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
/**
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*/
7+
8+
import { Rule } from 'eslint';
9+
10+
const SETTINGS_KEY = 'react-hooks';
11+
const SETTINGS_ADDITIONAL_EFFECT_HOOKS_KEY = 'additionalEffectHooks';
12+
13+
export function getAdditionalEffectHooksFromSettings(
14+
settings: Rule.RuleContext['settings'],
15+
): RegExp | undefined {
16+
const additionalHooks = settings[SETTINGS_KEY]?.[SETTINGS_ADDITIONAL_EFFECT_HOOKS_KEY];
17+
if (additionalHooks != null && typeof additionalHooks === 'string') {
18+
return new RegExp(additionalHooks);
19+
}
20+
21+
return undefined;
22+
}

0 commit comments

Comments
 (0)