@samitouri / QOS-React / commits / 78968bb3d9

Validate useEffect without deps too (#15183)

Dan Abramov committed Mar 21, 2019 at 20:42 UTC 78968bb3d9049c65f74d42a1b2f4f9f95d2cba42
2 files changed +59 -1
packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js
+55
@@ -2969,6 +2969,36 @@ const tests = {
2969 `and use that variable in the cleanup function.`,
2970 ],
2971 },
2972 + {
2973 + code: `
2974 + function MyComponent() {
2975 + const myRef = useRef();
2976 + useEffect(() => {
2977 + const handleMove = () => {};
2978 + myRef.current.addEventListener('mousemove', handleMove);
2979 + return () => myRef.current.removeEventListener('mousemove', handleMove);
2980 + });
2981 + return <div ref={myRef} />;
2982 + }
2983 + `,
2984 + output: `
2985 + function MyComponent() {
2986 + const myRef = useRef();
2987 + useEffect(() => {
2988 + const handleMove = () => {};
2989 + myRef.current.addEventListener('mousemove', handleMove);
2990 + return () => myRef.current.removeEventListener('mousemove', handleMove);
2991 + });
2992 + return <div ref={myRef} />;
2993 + }
2994 + `,
2995 + errors: [
2996 + `The ref value 'myRef.current' will likely have changed by the time ` +
2997 + `this effect cleanup function runs. If this ref points to a node ` +
2998 + `rendered by React, copy 'myRef.current' to a variable inside the effect, ` +
2999 + `and use that variable in the cleanup function.`,
3000 + ],
3001 + },
3002 {
3003 code: `
3004 function useMyThing(myRef) {
@@ -4457,6 +4487,31 @@ const tests = {
4487 'Learn more about data fetching with Hooks: https://fb.me/react-hooks-data-fetching',
4488 ],
4489 },
4490 + {
4491 + code: `
4492 + function Thing() {
4493 + useEffect(async () => {});
4494 + }
4495 + `,
4496 + output: `
4497 + function Thing() {
4498 + useEffect(async () => {});
4499 + }
4500 + `,
4501 + errors: [
4502 + `Effect callbacks are synchronous to prevent race conditions. ` +
4503 + `Put the async function inside:\n\n` +
4504 + 'useEffect(() => {\n' +
4505 + ' async function fetchData() {\n' +
4506 + ' // You can await here\n' +
4507 + ' const response = await MyAPI.getData(someId);\n' +
4508 + ' // ...\n' +
4509 + ' }\n' +
4510 + ' fetchData();\n' +
4511 + `}, [someId]); // Or [] if effect doesn't need props or state\n\n` +
4512 + 'Learn more about data fetching with Hooks: https://fb.me/react-hooks-data-fetching',
4513 + ],
4514 + },
4515 {
4516 code: `
4517 function Example() {
packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js
+4 -1
@@ -88,7 +88,7 @@ export default {
88 // So no need to check for dependency inclusion.
89 const depsIndex = callbackIndex + 1;
90 const declaredDependenciesNode = node.parent.arguments[depsIndex];
91 - if (!declaredDependenciesNode) {
91 + if (!declaredDependenciesNode && !isEffect) {
92 // These are only used for optimization.
93 if (
94 reactiveHookName === 'useMemo' ||
@@ -468,6 +468,9 @@ export default {
468 },
469 );
470
471 + if (!declaredDependenciesNode) {
472 + return;
473 + }
474 const declaredDependencies = [];
475 const externalDependencies = new Set();
476 if (declaredDependenciesNode.type !== 'ArrayExpression') {