@samitouri / QOS-React-2 / commits / c89a83695c

Update RulesOfHooks with useEvent rules (#25285)

This update to the RulesOfHooks rule checks that functions created with `useEvent` can only be invoked in a `useEffect` callback, in another event function, or a closure. They can't be passed down directly as a reference to child components. This PR also updates the ExhaustiveDeps lint rule to treat useEvent's return value as stable, so it can be omitted from dependency lists. Currently this all gated behind an experimental flag. Co-authored-by: Dan Abramov <dan.abramov@gmail.com>

Lauren Tan committed Sep 23, 2022 at 14:55 UTC c89a83695c18814e98a8e815a2389a11540e0160

4 files changed +290 -2

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

+19 -1

@@ -1630,7 +1630,7 @@ const tests = {

1630 }, 1000);

1631 return () => clearInterval(id);

1632 }, [setCount]);

1633 -

1633 +

1634 return <h1>{count}</h1>;

1635 }

1636 `,

@@ -7630,6 +7630,24 @@ const tests = {

7630 ],

7631 };

7632

7633 +if (__EXPERIMENTAL__) {

7634 + tests.valid = [

7635 + ...tests.valid,

7636 + {

7637 + code: normalizeIndent`

7638 + function MyComponent({ theme }) {

7639 + const onStuff = useEvent(() => {

7640 + showNotification(theme);

7641 + });

7642 + useEffect(() => {

7643 + onStuff();

7644 + }, []);

7645 + }

7646 + `,

7647 + },

7648 + ];

7649 +}

7650 +

7651 // Tests that are only valid/invalid across parsers supporting Flow

7652 const testsFlow = {

7653 valid: [

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

+157 -1

@@ -449,7 +449,7 @@ const tests = {

449 },

450 {

451 code: `

452 - // This is a false positive (it's valid) that unfortunately

452 + // This is a false positive (it's valid) that unfortunately

453 // we cannot avoid. Prefer to rename it to not start with "use"

454 class Foo extends Component {

455 render() {

@@ -974,6 +974,154 @@ const tests = {

974 ],

975 };

976

977 +if (__EXPERIMENTAL__) {

978 + tests.valid = [

979 + ...tests.valid,

980 + `

981 + // Valid because functions created with useEvent can be called in a useEffect.

982 + function MyComponent({ theme }) {

983 + const onClick = useEvent(() => {

984 + showNotification(theme);

985 + });

986 + useEffect(() => {

987 + onClick();

988 + });

989 + }

990 + `,

991 + `

992 + // Valid because functions created with useEvent can be called in closures.

993 + function MyComponent({ theme }) {

994 + const onClick = useEvent(() => {

995 + showNotification(theme);

996 + });

997 + return <Child onClick={() => onClick()}></Child>;

998 + }

999 + `,

1000 + `

1001 + // Valid because functions created with useEvent can be called in closures.

1002 + function MyComponent({ theme }) {

1003 + const onClick = useEvent(() => {

1004 + showNotification(theme);

1005 + });

1006 + const onClick2 = () => { onClick() };

1007 + const onClick3 = useCallback(() => onClick(), []);

1008 + return <>

1009 + <Child onClick={onClick2}></Child>

1010 + <Child onClick={onClick3}></Child>

1011 + </>;

1012 + }

1013 + `,

1014 + `

1015 + // Valid because functions created with useEvent can be passed by reference in useEffect

1016 + // and useEvent.

1017 + function MyComponent({ theme }) {

1018 + const onClick = useEvent(() => {

1019 + showNotification(theme);

1020 + });

1021 + const onClick2 = useEvent(() => {

1022 + debounce(onClick);

1023 + });

1024 + useEffect(() => {

1025 + let id = setInterval(onClick, 100);

1026 + return () => clearInterval(onClick);

1027 + }, []);

1028 + return <Child onClick={() => onClick2()} />

1029 + }

1030 + `,

1031 + `

1032 + const MyComponent = ({theme}) => {

1033 + const onClick = useEvent(() => {

1034 + showNotification(theme);

1035 + });

1036 + return <Child onClick={() => onClick()}></Child>;

1037 + };

1038 + `,

1039 + `

1040 + function MyComponent({ theme }) {

1041 + const notificationService = useNotifications();

1042 + const showNotification = useEvent((text) => {

1043 + notificationService.notify(theme, text);

1044 + });

1045 + const onClick = useEvent((text) => {

1046 + showNotification(text);

1047 + });

1048 + return <Child onClick={(text) => onClick(text)} />

1049 + }

1050 + `,

1051 + `

1052 + function MyComponent({ theme }) {

1053 + useEffect(() => {

1054 + onClick();

1055 + });

1056 + const onClick = useEvent(() => {

1057 + showNotification(theme);

1058 + });

1059 + }

1060 + `,

1061 + ];

1062 + tests.invalid = [

1063 + ...tests.invalid,

1064 + {

1065 + code: `

1066 + function MyComponent({ theme }) {

1067 + const onClick = useEvent(() => {

1068 + showNotification(theme);

1069 + });

1070 + return <Child onClick={onClick}></Child>;

1071 + }

1072 + `,

1073 + errors: [useEventError('onClick')],

1074 + },

1075 + {

1076 + code: `

1077 + // This should error even though it shares an identifier name with the below

1078 + function MyComponent({theme}) {

1079 + const onClick = useEvent(() => {

1080 + showNotification(theme)

1081 + });

1082 + return <Child onClick={onClick} />

1083 + }

1084 +

1085 + // The useEvent function shares an identifier name with the above

1086 + function MyOtherComponent({theme}) {

1087 + const onClick = useEvent(() => {

1088 + showNotification(theme)

1089 + });

1090 + return <Child onClick={() => onClick()} />

1091 + }

1092 + `,

1093 + errors: [{...useEventError('onClick'), line: 4}],

1094 + },

1095 + {

1096 + code: `

1097 + const MyComponent = ({ theme }) => {

1098 + const onClick = useEvent(() => {

1099 + showNotification(theme);

1100 + });

1101 + return <Child onClick={onClick}></Child>;

1102 + }

1103 + `,

1104 + errors: [useEventError('onClick')],

1105 + },

1106 + {

1107 + code: `

1108 + // Invalid because onClick is being aliased to foo but not invoked

1109 + function MyComponent({ theme }) {

1110 + const onClick = useEvent(() => {

1111 + showNotification(theme);

1112 + });

1113 + let foo;

1114 + useEffect(() => {

1115 + foo = onClick;

1116 + });

1117 + return <Bar onClick={foo} />

1118 + }

1119 + `,

1120 + errors: [useEventError('onClick')],

1121 + },

1122 + ];

1123 +}

1124 +

1125 function conditionalError(hook, hasPreviousFinalizer = false) {

1126 return {

1127 message:

@@ -1031,6 +1179,14 @@ function classError(hook) {

1179 };

1180 }

1181

1182 +function useEventError(fn) {

1183 + return {

1184 + message:

1185 + `\`${fn}\` is a function created with React Hook "useEvent", and can only be called from ` +

1186 + 'the same component. They cannot be assigned to variables or passed down.',

1187 + };

1188 +}

1189 +

1190 // For easier local testing

1191 if (!process.env.CI) {

1192 let only = [];

packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js

+12

@@ -157,6 +157,8 @@ export default {

157 // ^^^ true for this reference

158 // const ref = useRef()

159 // ^^^ true for this reference

160 + // const onStuff = useEvent(() => {})

161 + // ^^^ true for this reference

162 // False for everything else.

163 function isStableKnownHookValue(resolved) {

164 if (!isArray(resolved.defs)) {

@@ -223,6 +225,9 @@ export default {

225 if (name === 'useRef' && id.type === 'Identifier') {

226 // useRef() return value is stable.

227 return true;

228 + } else if (isUseEventIdentifier(callee) && id.type === 'Identifier') {

229 + // useEvent() return value is stable.

230 + return true;

231 } else if (name === 'useState' || name === 'useReducer') {

232 // Only consider second value in initializing tuple stable.

233 if (

@@ -1819,3 +1824,10 @@ function isSameIdentifier(a, b) {

1824 function isAncestorNodeOf(a, b) {

1825 return a.range[0] <= b.range[0] && a.range[1] >= b.range[1];

1826 }

1827 +

1828 +function isUseEventIdentifier(node) {

1829 + if (__EXPERIMENTAL__) {

1830 + return node.type === 'Identifier' && node.name === 'useEvent';

1831 + }

1832 + return false;

1833 +}

packages/eslint-plugin-react-hooks/src/RulesOfHooks.js

+102

@@ -100,6 +100,13 @@ function isInsideComponentOrHook(node) {

100 return false;

101 }

102

103 +function isUseEventIdentifier(node) {

104 + if (__EXPERIMENTAL__) {

105 + return node.type === 'Identifier' && node.name === 'useEvent';

106 + }

107 + return false;

108 +}

109 +

110 export default {

111 meta: {

112 type: 'problem',

@@ -110,8 +117,45 @@ export default {

117 },

118 },

119 create(context) {

120 + let lastEffect = null;

121 const codePathReactHooksMapStack = [];

122 const codePathSegmentStack = [];

123 + const useEventViolations = new Set();

124 +

125 + // For a given AST node, iterate through the top level statements and add all useEvent

126 + // definitions. We can do this in non-Program nodes because we can rely on the assumption that

127 + // useEvent functions can only be declared within a component or hook at its top level.

128 + function addAllUseEventViolations(node) {

129 + if (node.body.type !== 'BlockStatement') return;

130 + for (const statement of node.body.body) {

131 + if (statement.type !== 'VariableDeclaration') continue;

132 + for (const declaration of statement.declarations) {

133 + if (

134 + declaration.type === 'VariableDeclarator' &&

135 + declaration.init &&

136 + declaration.init.type === 'CallExpression' &&

137 + declaration.init.callee &&

138 + isUseEventIdentifier(declaration.init.callee)

139 + ) {

140 + useEventViolations.add(declaration.id);

141 + }

142 + }

143 + }

144 + }

145 +

146 + // Resolve a useEvent violation, ie the useEvent created function was called.

147 + function resolveUseEventViolation(scope, ident) {

148 + if (scope.references == null || useEventViolations.size === 0) return;

149 + for (const ref of scope.references) {

150 + if (ref.resolved == null) continue;

151 + const [useEventFunctionIdentifier] = ref.resolved.identifiers;

152 + if (ident.name === useEventFunctionIdentifier.name) {

153 + useEventViolations.delete(useEventFunctionIdentifier);

154 + break;

155 + }

156 + }

157 + }

158 +

159 return {

160 // Maintain code segment path stack as we traverse.

161 onCodePathSegmentStart: segment => codePathSegmentStack.push(segment),

@@ -522,6 +566,64 @@ export default {

566 }

567 reactHooks.push(node.callee);

568 }

569 +

570 + const scope = context.getScope();

571 + // useEvent: Resolve a function created with useEvent that is invoked locally at least once.

572 + // OK - onClick();

573 + resolveUseEventViolation(scope, node.callee);

574 +

575 + // useEvent: useEvent functions can be passed by reference within useEffect as well as in

576 + // another useEvent

577 + if (

578 + node.callee.type === 'Identifier' &&

579 + (node.callee.name === 'useEffect' ||

580 + isUseEventIdentifier(node.callee)) &&

581 + node.arguments.length > 0

582 + ) {

583 + // Denote that we have traversed into a useEffect call, and stash the CallExpr for

584 + // comparison later when we exit

585 + lastEffect = node;

586 + }

587 + },

588 +

589 + Identifier(node) {

590 + // OK - useEffect(() => { setInterval(onClick, ...) }, []);

591 + if (lastEffect != null && node.parent.type === 'CallExpression') {

592 + resolveUseEventViolation(context.getScope(), node);

593 + }

594 + },

595 +

596 + 'CallExpression:exit'(node) {

597 + if (node === lastEffect) {

598 + lastEffect = null;

599 + }

600 + },

601 +

602 + FunctionDeclaration(node) {

603 + // function MyComponent() { const onClick = useEvent(...) }

604 + if (isInsideComponentOrHook(node)) {

605 + addAllUseEventViolations(node);

606 + }

607 + },

608 +

609 + ArrowFunctionExpression(node) {

610 + // const MyComponent = () => { const onClick = useEvent(...) }

611 + if (isInsideComponentOrHook(node)) {

612 + addAllUseEventViolations(node);

613 + }

614 + },

615 +

616 + 'Program:exit'(_node) {

617 + for (const node of useEventViolations.values()) {

618 + context.report({

619 + node,

620 + message:

621 + `\`${context.getSource(

622 + node,

623 + )}\` is a function created with React Hook "useEvent", and can only be called from ` +

624 + 'the same component. They cannot be assigned to variables or passed down.',

625 + });

626 + }

627 },

628 };

629 },