refactor: resolve hook violations

aalemayhu · aalemayhu · commit 9d6fb7106e79 · 2024-12-24T23:23:44.000+01:00
Removes the following warnings: > $ npm run lint > > instructlab-ui@0.1.0 lint > next lint > > > ./src/app/login/githublogin.tsx > 32:6 Warning: React Hook useEffect has a missing dependency: 'searchParams'. Either include it or remove the dependency array. react-hooks/exhaustive-deps > > ./src/components/Contribute/Knowledge/Github/index.tsx > 429:9 Warning: The 'knowledgeFormData' object makes the dependencies of useEffect Hook (at line 449) change on every render. To fix this, wrap the initialization of 'knowledgeFormData' in its own useMemo() Hook. react-hooks/exhaustive-deps > > ./src/components/Contribute/Knowledge/Native/index.tsx > 428:9 Warning: The 'knowledgeFormData' object makes the dependencies of useEffect Hook (at line 448) change on every render. To fix this, wrap the initialization of 'knowledgeFormData' in its own useMemo() Hook. react-hooks/exhaustive-deps > > ./src/components/Contribute/Skill/Github/index.tsx > 327:9 Warning: The 'skillFormData' object makes the dependencies of useEffect Hook (at line 341) change on every render. To fix this, wrap the initialization of 'skillFormData' in its own useMemo() Hook. react-hooks/exhaustive-deps > > ./src/components/Contribute/Skill/Native/index.tsx > 304:9 Warning: The 'skillFormData' object makes the dependencies of useEffect Hook (at line 318) change on every render. To fix this, wrap the initialization of 'skillFormData' in its own useMemo() Hook. react-hooks/exhaustive-deps > > ./src/components/GithubAccessPopup/index.tsx > 28:6 Warning: React Hook React.useEffect has a missing dependency: 'onAccept'. Either include it or remove the dependency array. If 'onAccept' changes too often, find the parent component that defines it and wrap that definition in useCallback. react-hooks/exhaustive-deps > > ./src/components/PathService/PathService.tsx > 82:6 Warning: React Hook useEffect has a missing dependency: 'path'. Either include it or remove the dependency array. If 'setInputValue' needs the current value of 'path', you can also switch to useReducer instead of useState and read 'path' in the reducer. react-hooks/exhaustive-deps > 94:6 Warning: React Hook useEffect has missing dependencies: 'fetchData', 'handlePathChange', and 'validatePath'. Either include them or remove the dependency array. If 'handlePathChange' changes too often, find the parent component that defines it and wrap that definition in useCallback. react-hooks/exhaustive-deps > > info - Need to disable some ESLint rules? Learn more here: https://nextjs.org/docs/app/api-reference/config/eslint#disabling-rules Signed-off-by: Alexander Alemayhu <alexander@alemayhu.com>
diff --git a/src/app/login/githublogin.tsx b/src/app/login/githublogin.tsx
@@ -29,7 +29,7 @@ const GithubLogin: React.FC = () => {
       setErrorMsg(errorMessage);
       setShowError(true);
     }
-  }, []);
+  }, [searchParams]);
 
   const handleGitHubLogin = () => {
     signIn('github', { callbackUrl: '/' }); // Redirect to home page after login
diff --git a/src/components/Contribute/Knowledge/Github/index.tsx b/src/components/Contribute/Knowledge/Github/index.tsx
@@ -426,23 +426,42 @@ export const KnowledgeFormGithub: React.FunctionComponent<KnowledgeFormProps> =
     setSeedExamples(yamlSeedExampleToFormSeedExample(data.seed_examples));
   };
 
-  const knowledgeFormData: KnowledgeFormData = {
-    email: email,
-    name: name,
-    submissionSummary: submissionSummary,
-    domain: domain,
-    documentOutline: documentOutline,
-    filePath: filePath,
-    seedExamples: seedExamples,
-    knowledgeDocumentRepositoryUrl: knowledgeDocumentRepositoryUrl,
-    knowledgeDocumentCommit: knowledgeDocumentCommit,
-    documentName: documentName,
-    titleWork: titleWork,
-    linkWork: linkWork,
-    revision: revision,
-    licenseWork: licenseWork,
-    creators: creators
-  };
+  const knowledgeFormData: KnowledgeFormData = useMemo(
+    () => ({
+      email: email,
+      name: name,
+      submissionSummary: submissionSummary,
+      domain: domain,
+      documentOutline: documentOutline,
+      filePath: filePath,
+      seedExamples: seedExamples,
+      knowledgeDocumentRepositoryUrl: knowledgeDocumentRepositoryUrl,
+      knowledgeDocumentCommit: knowledgeDocumentCommit,
+      documentName: documentName,
+      titleWork: titleWork,
+      linkWork: linkWork,
+      revision: revision,
+      licenseWork: licenseWork,
+      creators: creators
+    }),
+    [
+      email,
+      name,
+      submissionSummary,
+      domain,
+      documentOutline,
+      filePath,
+      seedExamples,
+      knowledgeDocumentRepositoryUrl,
+      knowledgeDocumentCommit,
+      documentName,
+      titleWork,
+      linkWork,
+      revision,
+      licenseWork,
+      creators
+    ]
+  );
 
   useEffect(() => {
     setDisableAction(!checkKnowledgeFormCompletion(knowledgeFormData));
diff --git a/src/components/Contribute/Knowledge/Native/index.tsx b/src/components/Contribute/Knowledge/Native/index.tsx
@@ -425,23 +425,42 @@ export const KnowledgeFormNative: React.FunctionComponent<KnowledgeFormProps> =
     setSeedExamples(yamlSeedExampleToFormSeedExample(data.seed_examples));
   };
 
-  const knowledgeFormData: KnowledgeFormData = {
-    email: email,
-    name: name,
-    submissionSummary: submissionSummary,
-    domain: domain,
-    documentOutline: documentOutline,
-    filePath: filePath,
-    seedExamples: seedExamples,
-    knowledgeDocumentRepositoryUrl: knowledgeDocumentRepositoryUrl,
-    knowledgeDocumentCommit: knowledgeDocumentCommit,
-    documentName: documentName,
-    titleWork: titleWork,
-    linkWork: linkWork,
-    revision: revision,
-    licenseWork: licenseWork,
-    creators: creators
-  };
+  const knowledgeFormData: KnowledgeFormData = useMemo(
+    () => ({
+      email: email,
+      name: name,
+      submissionSummary: submissionSummary,
+      domain: domain,
+      documentOutline: documentOutline,
+      filePath: filePath,
+      seedExamples: seedExamples,
+      knowledgeDocumentRepositoryUrl: knowledgeDocumentRepositoryUrl,
+      knowledgeDocumentCommit: knowledgeDocumentCommit,
+      documentName: documentName,
+      titleWork: titleWork,
+      linkWork: linkWork,
+      revision: revision,
+      licenseWork: licenseWork,
+      creators: creators
+    }),
+    [
+      email,
+      name,
+      submissionSummary,
+      domain,
+      documentOutline,
+      filePath,
+      seedExamples,
+      knowledgeDocumentRepositoryUrl,
+      knowledgeDocumentCommit,
+      documentName,
+      titleWork,
+      linkWork,
+      revision,
+      licenseWork,
+      creators
+    ]
+  );
 
   useEffect(() => {
     setDisableAction(!checkKnowledgeFormCompletion(knowledgeFormData));
diff --git a/src/components/Contribute/Skill/Github/index.tsx b/src/components/Contribute/Skill/Github/index.tsx
@@ -1,6 +1,6 @@
 // src/components/Contribute/Skill/Github/index.tsx
 'use client';
-import React, { useEffect, useState } from 'react';
+import React, { useEffect, useState, useMemo } from 'react';
 import './skills.css';
 import { Alert, AlertActionCloseButton, AlertGroup } from '@patternfly/react-core/dist/dynamic/components/Alert';
 import { ActionGroup } from '@patternfly/react-core/dist/dynamic/components/Form';
@@ -324,17 +324,20 @@ export const SkillFormGithub: React.FunctionComponent<SkillFormProps> = ({ skill
     setSeedExamples(yamlSeedExampleToFormSeedExample(data.seed_examples));
   };
 
-  const skillFormData: SkillFormData = {
-    email: email,
-    name: name,
-    submissionSummary: submissionSummary,
-    documentOutline: documentOutline,
-    filePath: filePath,
-    seedExamples: seedExamples,
-    titleWork: titleWork,
-    licenseWork: licenseWork,
-    creators: creators
-  };
+  const skillFormData: SkillFormData = useMemo(
+    () => ({
+      email: email,
+      name: name,
+      submissionSummary: submissionSummary,
+      documentOutline: documentOutline,
+      filePath: filePath,
+      seedExamples: seedExamples,
+      titleWork: titleWork,
+      licenseWork: licenseWork,
+      creators: creators
+    }),
+    [email, name, submissionSummary, documentOutline, filePath, seedExamples, titleWork, licenseWork, creators]
+  );
 
   useEffect(() => {
     setDisableAction(!checkSkillFormCompletion(skillFormData));
diff --git a/src/components/Contribute/Skill/Native/index.tsx b/src/components/Contribute/Skill/Native/index.tsx
@@ -1,6 +1,6 @@
 // src/components/Contribute/Skill/Native/index.tsx
 'use client';
-import React, { useEffect, useState } from 'react';
+import React, { useEffect, useState, useMemo } from 'react';
 import './skills.css';
 import { Alert, AlertActionCloseButton, AlertGroup } from '@patternfly/react-core/dist/dynamic/components/Alert';
 import { ActionGroup } from '@patternfly/react-core/dist/dynamic/components/Form';
@@ -301,17 +301,20 @@ export const SkillFormNative: React.FunctionComponent<SkillFormProps> = ({ skill
     setSeedExamples(yamlSeedExampleToFormSeedExample(data.seed_examples));
   };
 
-  const skillFormData: SkillFormData = {
-    email: email,
-    name: name,
-    submissionSummary: submissionSummary,
-    documentOutline: documentOutline,
-    filePath: filePath,
-    seedExamples: seedExamples,
-    titleWork: titleWork,
-    licenseWork: licenseWork,
-    creators: creators
-  };
+  const skillFormData: SkillFormData = useMemo(
+    () => ({
+      email,
+      name,
+      submissionSummary,
+      documentOutline,
+      filePath,
+      seedExamples,
+      titleWork,
+      licenseWork,
+      creators
+    }),
+    [email, name, submissionSummary, documentOutline, filePath, seedExamples, titleWork, licenseWork, creators]
+  );
 
   useEffect(() => {
     setDisableAction(!checkSkillFormCompletion(skillFormData));
diff --git a/src/components/GithubAccessPopup/index.tsx b/src/components/GithubAccessPopup/index.tsx
@@ -25,7 +25,7 @@ const GithubAccessPopup: React.FunctionComponent<Props> = ({ onAccept }) => {
       }
     };
     showPopupWarning();
-  }, []);
+  }, [onAccept]);
 
   const setDecisionAndClose = () => {
     setIsOpen(false);
diff --git a/src/components/PathService/PathService.tsx b/src/components/PathService/PathService.tsx
@@ -1,5 +1,5 @@
 // /src/components/PathService.tsx
-import React, { useState, useEffect, useRef } from 'react';
+import React, { useState, useEffect, useRef, useCallback } from 'react';
 import { SearchInput } from '@patternfly/react-core/dist/dynamic/components/SearchInput';
 import { List } from '@patternfly/react-core/dist/dynamic/components/List';
 import { ListItem } from '@patternfly/react-core/dist/dynamic/components/List';
@@ -23,40 +23,43 @@ const PathService: React.FC<PathServiceProps> = ({ reset, rootPath, path, handle
   const inputRef = useRef<HTMLInputElement>(null);
   const [validPath, setValidPath] = React.useState<ValidatedOptions>();
 
-  const validatePath = () => {
+  const validatePath = useCallback(() => {
     if (inputValue.trim().length > 0) {
       setValidPath(ValidatedOptions.success);
       return;
     }
     setValidPath(ValidatedOptions.error);
-  };
-
-  const fetchData = async (subpath: string) => {
-    try {
-      const response = await fetch('/api/tree', {
-        method: 'POST',
-        headers: {
-          'Content-Type': 'application/json'
-        },
-        body: JSON.stringify({ root_path: rootPath, dir_name: subpath })
-      });
-
-      if (!response.ok) {
-        console.warn('Failed to get path service tree for subpath ( ' + subpath + ' ) from server.');
-      }
+  }, [inputValue]);
 
-      const result = await response.json();
-      // set items to be displayed in the dropdown
-      if (result.data === null || result.data.length === 0) {
+  const fetchData = useCallback(
+    async (subpath: string) => {
+      try {
+        const response = await fetch('/api/tree', {
+          method: 'POST',
+          headers: {
+            'Content-Type': 'application/json'
+          },
+          body: JSON.stringify({ root_path: rootPath, dir_name: subpath })
+        });
+
+        if (!response.ok) {
+          console.warn('Failed to get path service tree for subpath ( ' + subpath + ' ) from server.');
+        }
+
+        const result = await response.json();
+        // set items to be displayed in the dropdown
+        if (result.data === null || result.data.length === 0) {
+          setItems([]);
+          return;
+        }
+        setItems(result.data.map((item: string) => item.valueOf()));
+      } catch (error) {
+        console.warn('Error fetching path service data:', error);
         setItems([]);
-        return;
       }
-      setItems(result.data.map((item: string) => item.valueOf()));
-    } catch (error) {
-      console.warn('Error fetching path service data:', error);
-      setItems([]);
-    }
-  };
+    },
+    [rootPath]
+  );
 
   useEffect(() => {
     setInputValue('');
@@ -79,7 +82,7 @@ const PathService: React.FC<PathServiceProps> = ({ reset, rootPath, path, handle
     return () => {
       window.removeEventListener('keydown', handleEsc);
     };
-  }, []);
+  }, [path]);
 
   useEffect(() => {
     // check if input value is empty or ends with a slash
@@ -91,7 +94,7 @@ const PathService: React.FC<PathServiceProps> = ({ reset, rootPath, path, handle
       setItems([]);
     }
     validatePath();
-  }, [inputValue]);
+  }, [inputValue, fetchData, handlePathChange, validatePath]);
 
   const handleChange = (value: string) => {
     setInputValue(value);

Original file line number	Diff line number	Diff line change
`@@ -29,7 +29,7 @@ const GithubLogin: React.FC = () => {`
`29`	`29`	`setErrorMsg(errorMessage);`
`30`	`30`	`setShowError(true);`
`31`	`31`	`}`
`32`		`- }, []);`
	`32`	`+ }, [searchParams]);`
`33`	`33`
`34`	`34`	`const handleGitHubLogin = () => {`
`35`	`35`	`signIn('github', { callbackUrl: '/' }); // Redirect to home page after login`
Original file line number	Diff line number	Diff line change
`@@ -25,7 +25,7 @@ const GithubAccessPopup: React.FunctionComponent<Props> = ({ onAccept }) => {`
`25`	`25`	`}`
`26`	`26`	`};`
`27`	`27`	`showPopupWarning();`
`28`		`- }, []);`
	`28`	`+ }, [onAccept]);`
`29`	`29`
`30`	`30`	`const setDecisionAndClose = () => {`
`31`	`31`	`setIsOpen(false);`