From 4b6e6202c6d06d62d6f523230e49498ec50c4354 Mon Sep 17 00:00:00 2001 From: Lauren Tan Date: Tue, 20 Sep 2022 16:26:53 -0400 Subject: [PATCH] Add more comments --- .../__tests__/ESLintRulesOfHooks-test.js | 5 ++-- .../src/RulesOfHooks.js | 27 ++++++++++++++----- 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js b/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js index d9fedff542196..f2a57e74ed871 100644 --- a/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js +++ b/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js @@ -461,7 +461,8 @@ const tests = { } `, ` - // Valid because onClick is hoisted and invoked + // Valid because onClick is invoked, and is created in MyComponent's environment prior to + // invocation function MyComponent({ theme }) { function Child() { return onClick()} />; @@ -1101,7 +1102,7 @@ const tests = { return onClick()} /> } `, - errors: [{...useEventError('onClick'), line: 4, column: 17}], + errors: [{...useEventError('onClick'), line: 4}], }, { code: ` diff --git a/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js b/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js index 1ebc9214f691f..70bcd7d548acb 100644 --- a/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js +++ b/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js @@ -137,6 +137,15 @@ export default { const codePathSegmentStack = []; const useEventViolations = new Map(); + /** + * For a given AST node, traverse into it and add all useEvent definitions, namespaced by the + * scope in which the definition was created. We can do this in non-Program nodes because we can + * rely on the assumption that useEvent functions can only be declared within a component or + * hook. + * + * @param {Scope} scope https://eslint.org/docs/latest/developer-guide/scope-manager-interface#scope-interface + * @param {Node} node + */ function addAllUseEventViolations(scope, node) { traverse(context, node, childNode => { if (isUseEventVariableDeclarator(childNode)) { @@ -145,6 +154,12 @@ export default { }); } + /** + * Add a single useEvent violation. + * + * @param {Scope} scope https://eslint.org/docs/latest/developer-guide/scope-manager-interface#scope-interface + * @param {Node} ident + */ function addUseEventViolation(scope, ident) { const scopedViolations = useEventViolations.get(scope) ?? new Set(); if (useEventViolations.has(scope)) { @@ -155,6 +170,12 @@ export default { } } + /** + * Resolve a useEvent violation, ie the useEvent created function was called. + * + * @param {Scope} scope https://eslint.org/docs/latest/developer-guide/scope-manager-interface#scope-interface + * @param {Node} ident + */ function resolveUseEventViolation(scope, ident) { if (scope.references == null) return; for (const ref of scope.references) { @@ -611,9 +632,6 @@ export default { }, FunctionDeclaration(node) { - // useEvent: First pass to record all definitions of useEvent functions. We do this here - // rather than in the Program visitor because we can rely on the assumption that useEvent - // functions can only be declared within a component or hook. // function MyComponent() { const onClick = useEvent(...) } if (isInsideComponentOrHook(node)) { addAllUseEventViolations(context.getScope(), node); @@ -621,9 +639,6 @@ export default { }, ArrowFunctionExpression(node) { - // useEvent: First pass to record all definitions of useEvent functions. We do this here - // rather than in the Program visitor because we can rely on the assumption that useEvent - // functions can only be declared within a component or hook. // const MyComponent = () => { const onClick = useEvent(...) } if (isInsideComponentOrHook(node)) { addAllUseEventViolations(context.getScope(), node);