Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[useEvent] Lint for presence of useEvent functions in dependency lists #25512

Merged
merged 3 commits into from
Oct 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -7635,15 +7635,53 @@ if (__EXPERIMENTAL__) {
...tests.valid,
{
code: normalizeIndent`
function MyComponent({ theme }) {
const onStuff = useEvent(() => {
showNotification(theme);
});
useEffect(() => {
onStuff();
}, []);
}
`,
function MyComponent({ theme }) {
const onStuff = useEvent(() => {
showNotification(theme);
});
useEffect(() => {
onStuff();
}, []);
}
`,
},
];

tests.invalid = [
...tests.invalid,
{
code: normalizeIndent`
function MyComponent({ theme }) {
const onStuff = useEvent(() => {
showNotification(theme);
});
useEffect(() => {
onStuff();
}, [onStuff]);
}
`,
errors: [
{
message:
'Functions returned from `useEvent` 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 = useEvent(() => {
showNotification(theme);
});
useEffect(() => {
onStuff();
}, []);
}
`,
},
],
},
],
},
];
}
Expand Down
28 changes: 27 additions & 1 deletion packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ export default {
const stateVariables = new WeakSet();
const stableKnownValueCache = new WeakMap();
const functionWithoutCapturedValueCache = new WeakMap();
const useEventVariables = new WeakSet();
function memoizeWithWeakMap(fn, map) {
return function(arg) {
if (map.has(arg)) {
Expand Down Expand Up @@ -226,7 +227,12 @@ export default {
// useRef() return value is stable.
return true;
} else if (isUseEventIdentifier(callee) && id.type === 'Identifier') {
// useEvent() return value is stable.
for (const ref of resolved.references) {
if (ref !== id) {
useEventVariables.add(ref.identifier);
}
}
// useEvent() return value is always unstable.
return true;
} else if (name === 'useState' || name === 'useReducer') {
// Only consider second value in initializing tuple stable.
Expand Down Expand Up @@ -639,6 +645,26 @@ export default {
});
return;
}
if (useEventVariables.has(declaredDependencyNode)) {
reportProblem({
node: declaredDependencyNode,
message:
'Functions returned from `useEvent` must not be included in the dependency array. ' +
`Remove \`${context.getSource(
declaredDependencyNode,
)}\` from the list.`,
suggest: [
{
desc: `Remove the dependency \`${context.getSource(
declaredDependencyNode,
)}\``,
fix(fixer) {
return fixer.removeRange(declaredDependencyNode.range);
},
},
],
});
}
// Try to normalize the declared dependency. If we can't then an error
// will be thrown. We will catch that error and report an error.
let declaredDependency;
Expand Down