mirror of
https://github.com/zebrajr/react.git
synced 2025-12-06 12:20:20 +01:00
[ESLint] Disallow passing effect event down when inlined as a prop (#34820)
## Summary Fixes https://github.com/facebook/react/issues/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.
This commit is contained in:
parent
5418d8bdc1
commit
2381ecc290
|
|
@ -1555,6 +1555,17 @@ const allTests = {
|
|||
`,
|
||||
errors: [useEffectEventError('onClick', false)],
|
||||
},
|
||||
{
|
||||
code: normalizeIndent`
|
||||
// Invalid because useEffectEvent is being passed down
|
||||
function MyComponent({ theme }) {
|
||||
return <Child onClick={useEffectEvent(() => {
|
||||
showNotification(theme);
|
||||
})} />;
|
||||
}
|
||||
`,
|
||||
errors: [{...useEffectEventError(null, false), line: 4}],
|
||||
},
|
||||
{
|
||||
code: normalizeIndent`
|
||||
// This should error even though it shares an identifier name with the below
|
||||
|
|
@ -1726,6 +1737,14 @@ function classError(hook) {
|
|||
}
|
||||
|
||||
function useEffectEventError(fn, called) {
|
||||
if (fn === null) {
|
||||
return {
|
||||
message:
|
||||
`React Hook "useEffectEvent" can only be called at the top level of your component.` +
|
||||
` It cannot be passed down.`,
|
||||
};
|
||||
}
|
||||
|
||||
return {
|
||||
message:
|
||||
`\`${fn}\` is a function created with React Hook "useEffectEvent", and can only be called from ` +
|
||||
|
|
|
|||
|
|
@ -171,7 +171,15 @@ function isUseEffectEventIdentifier(node: Node): boolean {
|
|||
return node.type === 'Identifier' && node.name === 'useEffectEvent';
|
||||
}
|
||||
|
||||
function useEffectEventError(fn: string, called: boolean): string {
|
||||
function useEffectEventError(fn: string | null, called: boolean): string {
|
||||
// no function identifier, i.e. it is not assigned to a variable
|
||||
if (fn === null) {
|
||||
return (
|
||||
`React Hook "useEffectEvent" can only be called at the top level of your component.` +
|
||||
` It cannot be passed down.`
|
||||
);
|
||||
}
|
||||
|
||||
return (
|
||||
`\`${fn}\` is a function created with React Hook "useEffectEvent", and can only be called from ` +
|
||||
'Effects and Effect Events in the same component.' +
|
||||
|
|
@ -772,6 +780,22 @@ const rule = {
|
|||
// comparison later when we exit
|
||||
lastEffect = node;
|
||||
}
|
||||
|
||||
// Specifically disallow <Child onClick={useEffectEvent(...)} /> because this
|
||||
// case can't be caught by `recordAllUseEffectEventFunctions` as it isn't assigned to a variable
|
||||
if (
|
||||
isUseEffectEventIdentifier(nodeWithoutNamespace) &&
|
||||
node.parent?.type !== 'VariableDeclarator' &&
|
||||
// like in other hooks, calling useEffectEvent at component's top level without assignment is valid
|
||||
node.parent?.type !== 'ExpressionStatement'
|
||||
) {
|
||||
const message = useEffectEventError(null, false);
|
||||
|
||||
context.report({
|
||||
node,
|
||||
message,
|
||||
});
|
||||
}
|
||||
},
|
||||
|
||||
Identifier(node) {
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user