mirror of
https://github.com/zebrajr/react.git
synced 2025-12-06 12:20:20 +01:00
[eslint-plugin-react-hooks][RulesOfHooks] handle React.useEffect in addition to useEffect (#34076)
## Summary This is a fix for https://github.com/facebook/react/issues/34074 ## How did you test this change? I added tests in the eslint package, and ran `yarn jest`. After adding the new tests, I have this: On main | On this branch -|- <img width="356" height="88" alt="image" src="https://github.com/user-attachments/assets/4ae099a1-0156-4032-b2ca-635ebadcaa3f" /> | <img width="435" height="120" alt="image" src="https://github.com/user-attachments/assets/b06c04b8-6cec-43de-befa-a8b4dd20500e" /> ## Changes - Add tests to check that we are checking both `CallExpression` (`useEffect(`), and `MemberExpression` (`React.useEffect(`). To do that, I copied the `getNodeWithoutReactNamespace(` fn from `ExhaustiveDeps.ts` to `RulesOfHooks.ts`
This commit is contained in:
parent
01ed0e9642
commit
87a45ae37f
|
|
@ -7735,6 +7735,9 @@ if (__EXPERIMENTAL__) {
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
onStuff();
|
onStuff();
|
||||||
}, []);
|
}, []);
|
||||||
|
React.useEffect(() => {
|
||||||
|
onStuff();
|
||||||
|
}, []);
|
||||||
}
|
}
|
||||||
`,
|
`,
|
||||||
},
|
},
|
||||||
|
|
@ -7751,6 +7754,9 @@ if (__EXPERIMENTAL__) {
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
onStuff();
|
onStuff();
|
||||||
}, [onStuff]);
|
}, [onStuff]);
|
||||||
|
React.useEffect(() => {
|
||||||
|
onStuff();
|
||||||
|
}, [onStuff]);
|
||||||
}
|
}
|
||||||
`,
|
`,
|
||||||
errors: [
|
errors: [
|
||||||
|
|
@ -7769,6 +7775,32 @@ if (__EXPERIMENTAL__) {
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
onStuff();
|
onStuff();
|
||||||
}, []);
|
}, []);
|
||||||
|
React.useEffect(() => {
|
||||||
|
onStuff();
|
||||||
|
}, [onStuff]);
|
||||||
|
}
|
||||||
|
`,
|
||||||
|
},
|
||||||
|
],
|
||||||
|
},
|
||||||
|
{
|
||||||
|
message:
|
||||||
|
'Functions returned from `useEffectEvent` must not be included in the dependency array. ' +
|
||||||
|
'Remove `onStuff` from the list.',
|
||||||
|
suggestions: [
|
||||||
|
{
|
||||||
|
desc: 'Remove the dependency `onStuff`',
|
||||||
|
output: normalizeIndent`
|
||||||
|
function MyComponent({ theme }) {
|
||||||
|
const onStuff = useEffectEvent(() => {
|
||||||
|
showNotification(theme);
|
||||||
|
});
|
||||||
|
useEffect(() => {
|
||||||
|
onStuff();
|
||||||
|
}, [onStuff]);
|
||||||
|
React.useEffect(() => {
|
||||||
|
onStuff();
|
||||||
|
}, []);
|
||||||
}
|
}
|
||||||
`,
|
`,
|
||||||
},
|
},
|
||||||
|
|
|
||||||
|
|
@ -1368,6 +1368,9 @@ if (__EXPERIMENTAL__) {
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
onClick();
|
onClick();
|
||||||
});
|
});
|
||||||
|
React.useEffect(() => {
|
||||||
|
onClick();
|
||||||
|
});
|
||||||
}
|
}
|
||||||
`,
|
`,
|
||||||
},
|
},
|
||||||
|
|
@ -1389,6 +1392,10 @@ if (__EXPERIMENTAL__) {
|
||||||
let id = setInterval(() => onClick(), 100);
|
let id = setInterval(() => onClick(), 100);
|
||||||
return () => clearInterval(onClick);
|
return () => clearInterval(onClick);
|
||||||
}, []);
|
}, []);
|
||||||
|
React.useEffect(() => {
|
||||||
|
let id = setInterval(() => onClick(), 100);
|
||||||
|
return () => clearInterval(onClick);
|
||||||
|
}, []);
|
||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
`,
|
`,
|
||||||
|
|
@ -1408,6 +1415,7 @@ if (__EXPERIMENTAL__) {
|
||||||
{
|
{
|
||||||
code: normalizeIndent`
|
code: normalizeIndent`
|
||||||
function MyComponent({ theme }) {
|
function MyComponent({ theme }) {
|
||||||
|
// Can receive arguments
|
||||||
const onEvent = useEffectEvent((text) => {
|
const onEvent = useEffectEvent((text) => {
|
||||||
console.log(text);
|
console.log(text);
|
||||||
});
|
});
|
||||||
|
|
@ -1415,6 +1423,9 @@ if (__EXPERIMENTAL__) {
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
onEvent('Hello world');
|
onEvent('Hello world');
|
||||||
});
|
});
|
||||||
|
React.useEffect(() => {
|
||||||
|
onEvent('Hello world');
|
||||||
|
});
|
||||||
}
|
}
|
||||||
`,
|
`,
|
||||||
},
|
},
|
||||||
|
|
|
||||||
|
|
@ -11,7 +11,10 @@ import type {
|
||||||
CallExpression,
|
CallExpression,
|
||||||
CatchClause,
|
CatchClause,
|
||||||
DoWhileStatement,
|
DoWhileStatement,
|
||||||
|
Expression,
|
||||||
|
Identifier,
|
||||||
Node,
|
Node,
|
||||||
|
Super,
|
||||||
TryStatement,
|
TryStatement,
|
||||||
} from 'estree';
|
} from 'estree';
|
||||||
|
|
||||||
|
|
@ -129,6 +132,24 @@ function isInsideTryCatch(
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function getNodeWithoutReactNamespace(
|
||||||
|
node: Expression | Super,
|
||||||
|
): Expression | Identifier | Super {
|
||||||
|
if (
|
||||||
|
node.type === 'MemberExpression' &&
|
||||||
|
node.object.type === 'Identifier' &&
|
||||||
|
node.object.name === 'React' &&
|
||||||
|
node.property.type === 'Identifier' &&
|
||||||
|
!node.computed
|
||||||
|
) {
|
||||||
|
return node.property;
|
||||||
|
}
|
||||||
|
return node;
|
||||||
|
}
|
||||||
|
|
||||||
|
function isUseEffectIdentifier(node: Node): boolean {
|
||||||
|
return node.type === 'Identifier' && node.name === 'useEffect';
|
||||||
|
}
|
||||||
function isUseEffectEventIdentifier(node: Node): boolean {
|
function isUseEffectEventIdentifier(node: Node): boolean {
|
||||||
if (__EXPERIMENTAL__) {
|
if (__EXPERIMENTAL__) {
|
||||||
return node.type === 'Identifier' && node.name === 'useEffectEvent';
|
return node.type === 'Identifier' && node.name === 'useEffectEvent';
|
||||||
|
|
@ -702,10 +723,11 @@ const rule = {
|
||||||
|
|
||||||
// useEffectEvent: useEffectEvent functions can be passed by reference within useEffect as well as in
|
// useEffectEvent: useEffectEvent functions can be passed by reference within useEffect as well as in
|
||||||
// another useEffectEvent
|
// another useEffectEvent
|
||||||
|
// Check all `useEffect` and `React.useEffect`, `useEffectEvent`, and `React.useEffectEvent`
|
||||||
|
const nodeWithoutNamespace = getNodeWithoutReactNamespace(node.callee);
|
||||||
if (
|
if (
|
||||||
node.callee.type === 'Identifier' &&
|
(isUseEffectIdentifier(nodeWithoutNamespace) ||
|
||||||
(node.callee.name === 'useEffect' ||
|
isUseEffectEventIdentifier(nodeWithoutNamespace)) &&
|
||||||
isUseEffectEventIdentifier(node.callee)) &&
|
|
||||||
node.arguments.length > 0
|
node.arguments.length > 0
|
||||||
) {
|
) {
|
||||||
// Denote that we have traversed into a useEffect call, and stash the CallExpr for
|
// Denote that we have traversed into a useEffect call, and stash the CallExpr for
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue
Block a user