Add more comments

facebook · Sep 20, 2022 · 2ce3c01 · 2ce3c01
1 parent ae39af9
commit 2ce3c01
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 7 deletions.
diff --git 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 <Foo onClick={() => onClick()} />;

diff --git 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<Identifier>} 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<Identifier>} ident
+     */
     function resolveUseEventViolation(scope, ident) {
       if (scope.references == null) return;
       for (const ref of scope.references) {
@@ -611,19 +632,13 @@ 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);
         }
       },
 
       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);