From 1913ac42958bba5c5a56ffa674f90595e20e8005 Mon Sep 17 00:00:00 2001 From: Alexander Alemayhu Date: Tue, 24 Dec 2024 23:12:44 +0100 Subject: [PATCH] refactor: resolve hook violations 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 --- src/app/login/githublogin.tsx | 2 +- .../Contribute/Knowledge/Github/index.tsx | 53 ++++++++++------ .../Contribute/Knowledge/Native/index.tsx | 53 ++++++++++------ .../Contribute/Skill/Github/index.tsx | 27 ++++---- .../Contribute/Skill/Native/index.tsx | 27 ++++---- src/components/GithubAccessPopup/index.tsx | 2 +- src/components/PathService/PathService.tsx | 61 ++++++++++--------- 7 files changed, 136 insertions(+), 89 deletions(-) diff --git a/src/app/login/githublogin.tsx b/src/app/login/githublogin.tsx index 8281b4877..1e82502e6 100644 --- 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 index 96f9b30de..1c15a8d3a 100644 --- 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 = 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, + name, + submissionSummary, + domain, + documentOutline, + filePath, + seedExamples, + knowledgeDocumentRepositoryUrl, + knowledgeDocumentCommit, + documentName, + titleWork, + linkWork, + revision, + licenseWork, + 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 index 794ca0b0a..4f9552201 100644 --- 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 = 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, + name, + submissionSummary, + domain, + documentOutline, + filePath, + seedExamples, + knowledgeDocumentRepositoryUrl, + knowledgeDocumentCommit, + documentName, + titleWork, + linkWork, + revision, + licenseWork, + 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 index a2ab87a99..fee18422b 100644 --- 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 = ({ 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/Contribute/Skill/Native/index.tsx b/src/components/Contribute/Skill/Native/index.tsx index a017f4fc8..79a23466c 100644 --- 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 = ({ 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 index 660449803..32f24e451 100644 --- a/src/components/GithubAccessPopup/index.tsx +++ b/src/components/GithubAccessPopup/index.tsx @@ -25,7 +25,7 @@ const GithubAccessPopup: React.FunctionComponent = ({ onAccept }) => { } }; showPopupWarning(); - }, []); + }, [onAccept]); const setDecisionAndClose = () => { setIsOpen(false); diff --git a/src/components/PathService/PathService.tsx b/src/components/PathService/PathService.tsx index 03aab6abf..30cbc0115 100644 --- 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 = ({ reset, rootPath, path, handle const inputRef = useRef(null); const [validPath, setValidPath] = React.useState(); - 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 = ({ 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 = ({ reset, rootPath, path, handle setItems([]); } validatePath(); - }, [inputValue]); + }, [inputValue, fetchData, handlePathChange, validatePath]); const handleChange = (value: string) => { setInputValue(value);