Skip to content

Commit 2381ecc

Browse files
authored
[ESLint] Disallow passing effect event down when inlined as a prop (#34820)
## Summary Fixes #34793. We are allowing passing down effect events when they are inlined as a prop. ``` <Child onClick={useEffectEvent(...)} /> ``` This seems like a case that someone not familiar with `useEffectEvent`'s purpose could fall for so this PR introduces logic to disallow its usage. An alternative implementation would be to modify the name and function of `recordAllUseEffectEventFunctions` to record all `useEffectEvent` instances either assigned to a variable or not, but this seems clearer. Or we could also specifically disallow its usage inside JSX. Feel free to suggest any improvements. ## How did you test this change? - Added a new test in `packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js`. All tests pass.
1 parent 5418d8b commit 2381ecc

File tree

2 files changed

+44
-1
lines changed

2 files changed

+44
-1
lines changed

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1555,6 +1555,17 @@ const allTests = {
15551555
`,
15561556
errors: [useEffectEventError('onClick', false)],
15571557
},
1558+
{
1559+
code: normalizeIndent`
1560+
// Invalid because useEffectEvent is being passed down
1561+
function MyComponent({ theme }) {
1562+
return <Child onClick={useEffectEvent(() => {
1563+
showNotification(theme);
1564+
})} />;
1565+
}
1566+
`,
1567+
errors: [{...useEffectEventError(null, false), line: 4}],
1568+
},
15581569
{
15591570
code: normalizeIndent`
15601571
// This should error even though it shares an identifier name with the below
@@ -1726,6 +1737,14 @@ function classError(hook) {
17261737
}
17271738

17281739
function useEffectEventError(fn, called) {
1740+
if (fn === null) {
1741+
return {
1742+
message:
1743+
`React Hook "useEffectEvent" can only be called at the top level of your component.` +
1744+
` It cannot be passed down.`,
1745+
};
1746+
}
1747+
17291748
return {
17301749
message:
17311750
`\`${fn}\` is a function created with React Hook "useEffectEvent", and can only be called from ` +

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

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,15 @@ function isUseEffectEventIdentifier(node: Node): boolean {
171171
return node.type === 'Identifier' && node.name === 'useEffectEvent';
172172
}
173173

174-
function useEffectEventError(fn: string, called: boolean): string {
174+
function useEffectEventError(fn: string | null, called: boolean): string {
175+
// no function identifier, i.e. it is not assigned to a variable
176+
if (fn === null) {
177+
return (
178+
`React Hook "useEffectEvent" can only be called at the top level of your component.` +
179+
` It cannot be passed down.`
180+
);
181+
}
182+
175183
return (
176184
`\`${fn}\` is a function created with React Hook "useEffectEvent", and can only be called from ` +
177185
'Effects and Effect Events in the same component.' +
@@ -772,6 +780,22 @@ const rule = {
772780
// comparison later when we exit
773781
lastEffect = node;
774782
}
783+
784+
// Specifically disallow <Child onClick={useEffectEvent(...)} /> because this
785+
// case can't be caught by `recordAllUseEffectEventFunctions` as it isn't assigned to a variable
786+
if (
787+
isUseEffectEventIdentifier(nodeWithoutNamespace) &&
788+
node.parent?.type !== 'VariableDeclarator' &&
789+
// like in other hooks, calling useEffectEvent at component's top level without assignment is valid
790+
node.parent?.type !== 'ExpressionStatement'
791+
) {
792+
const message = useEffectEventError(null, false);
793+
794+
context.report({
795+
node,
796+
message,
797+
});
798+
}
775799
},
776800

777801
Identifier(node) {

0 commit comments

Comments
 (0)