From 8765ceca688830193f3b5c86d6a59a99120cfcba Mon Sep 17 00:00:00 2001 From: Na'aman Hirschfeld Date: Wed, 17 Jan 2024 08:25:10 +0100 Subject: [PATCH 1/5] chore: fix frontend hook dependencies --- .eslintrc.js | 17 ++- .../config-create-wizard/page.tsx | 44 ++------ .../configs/[promptConfigId]/page.tsx | 2 +- .../applications/[applicationId]/page.tsx | 2 +- .../[locale]/projects/[projectId]/page.tsx | 2 +- .../src/app/[locale]/projects/create/page.tsx | 4 +- frontend/src/app/[locale]/projects/page.tsx | 2 +- frontend/src/app/[locale]/settings/page.tsx | 2 +- frontend/src/app/[locale]/sign-in/page.tsx | 4 +- frontend/src/app/[locale]/support/page.tsx | 2 +- .../static-site/track-static-page.tsx | 3 +- frontend/src/hooks/use-analytics.ts | 8 +- frontend/src/hooks/use-authenticated-user.ts | 2 +- frontend/src/hooks/use-prompt-testing.ts | 101 ++++++++++-------- frontend/tests/test-utils.tsx | 2 +- 15 files changed, 93 insertions(+), 104 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index 5c3870b8..afa0ec22 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -3,10 +3,12 @@ const rules = {}; const tsWebRules = { ...rules, '@next/next/no-html-link-for-pages': 0, + 'react-hooks/exhaustive-deps': 2, }; const tsWebTestRules = { ...tsWebRules, + 'testing-library/no-wait-for-side-effects': 0, 'no-restricted-imports': [ 'error', { @@ -51,10 +53,10 @@ module.exports = { rules, }, { - files: ['{frontend,docs}/**/*.{ts,tsx}'], + files: ['frontend/**/*.{ts,tsx}'], excludedFiles: [ - '{frontend,docs}/**/tests/**/*.{ts,tsx}', - '{frontend,docs}/**/*.spec.{ts,tsx}', + 'frontend/**/tests/**/*.{ts,tsx}', + 'frontend/**/*.spec.{ts,tsx}', ], extends: [ '@tool-belt/eslint-config/react', @@ -64,17 +66,14 @@ module.exports = { }, { files: [ - '{frontend,docs}/tests/**/*.{ts,tsx}', - '{frontend,docs}/**/*.spec.{ts,tsx}', + 'frontend/**/tests/**/*.{ts,tsx}', + 'frontend/**/*.spec.{ts,tsx}', ], extends: [ '@tool-belt/eslint-config/react', 'plugin:vitest/recommended', ], - rules: { - ...tsWebTestRules, - 'testing-library/no-wait-for-side-effects': 0, - }, + rules: tsWebTestRules, }, ], }; diff --git a/frontend/src/app/[locale]/projects/[projectId]/applications/[applicationId]/config-create-wizard/page.tsx b/frontend/src/app/[locale]/projects/[projectId]/applications/[applicationId]/config-create-wizard/page.tsx index 12f99477..5468dde8 100644 --- a/frontend/src/app/[locale]/projects/[projectId]/applications/[applicationId]/config-create-wizard/page.tsx +++ b/frontend/src/app/[locale]/projects/[projectId]/applications/[applicationId]/config-create-wizard/page.tsx @@ -158,38 +158,12 @@ export default function PromptConfigCreateWizard({ const store = usePromptWizardStore(wizardStoreSelector, shallow); - // callbacks - const handleConfigNameChange = useCallback(store.setConfigName, [ - store.setConfigName, - ]); - - const handleModelVendorChange = useCallback(store.setModelVendor, [ - store.setModelVendor, - ]); - - const handleModelTypeChange = useCallback(store.setModelType, [ - store.setModelType, - ]); - - const handleMessagesChange = useCallback(store.setMessages, [ - store.setMessages, - ]); - - const handleParametersChange = useCallback(store.setParameters, [ - store.setParameters, - ]); - const validateConfigName = useCallback( (value: string) => !(promptConfigs[applicationId]?.map((c) => c.name) ?? []).includes( value, ), - [promptConfigs], - ); - - const handleTemplateVariablesChange = useCallback( - store.setTemplateVariables, - [store.setTemplateVariables], + [applicationId, promptConfigs], ); const handleRefreshProject = useCallback(async () => { @@ -199,7 +173,7 @@ export default function PromptConfigCreateWizard({ } catch { handleError(t('errorRefreshingProject')); } - }, [setProjects]); + }, [setProjects, handleError, t]); useEffect(() => { if (initialized) { @@ -209,7 +183,7 @@ export default function PromptConfigCreateWizard({ stage: store.wizardStage, }); } - }, [initialized, store.wizardStage]); + }, [applicationId, projectId, page, initialized, store.wizardStage]); const handleConfigSave = async () => { setIsLoading(true); @@ -279,15 +253,15 @@ export default function PromptConfigCreateWizard({ [] = [ { diff --git a/frontend/src/app/[locale]/projects/create/page.tsx b/frontend/src/app/[locale]/projects/create/page.tsx index 42ba72bf..9bcc5fba 100644 --- a/frontend/src/app/[locale]/projects/create/page.tsx +++ b/frontend/src/app/[locale]/projects/create/page.tsx @@ -25,13 +25,13 @@ export default function CreateProjectPage() { const handleCancel = useCallback(() => { setIsLoading(true); router.replace(`${Navigation.Projects}/${projects[0].id}`); - }, [projects]); + }, [projects, router]); useEffect(() => { if (initialized) { page('createProject'); } - }, [initialized]); + }, [initialized, page]); const isInitialRef = useRef(projects.length > 0); diff --git a/frontend/src/app/[locale]/projects/page.tsx b/frontend/src/app/[locale]/projects/page.tsx index c434c14e..3a0d9c42 100644 --- a/frontend/src/app/[locale]/projects/page.tsx +++ b/frontend/src/app/[locale]/projects/page.tsx @@ -45,7 +45,7 @@ export default function Projects() { if (selectedProject) { router.replace(`${Navigation.Projects}/${selectedProject.id}`); } - }, [selectedProject]); + }, [selectedProject, router]); return (
{ page('auth'); - }, [initialized]); + }, [initialized, page]); useEffect(() => { if (user) { router.replace(Navigation.Projects); } - }, [user]); + }, [user, router]); return (
diff --git a/frontend/src/app/[locale]/support/page.tsx b/frontend/src/app/[locale]/support/page.tsx index 0b0d1637..4345fdf1 100644 --- a/frontend/src/app/[locale]/support/page.tsx +++ b/frontend/src/app/[locale]/support/page.tsx @@ -17,7 +17,7 @@ export default function Support() { if (initialized) { page('support'); } - }, [initialized]); + }, [initialized, page]); return (
{ if (initialized) { page(pageName); } - }, [initialized, pageName]); + }, [page, initialized, pageName]); return null; } diff --git a/frontend/src/hooks/use-analytics.ts b/frontend/src/hooks/use-analytics.ts index a8fee1ca..b7ee278b 100644 --- a/frontend/src/hooks/use-analytics.ts +++ b/frontend/src/hooks/use-analytics.ts @@ -65,7 +65,7 @@ export function useAnalytics(): AnalyticsHandlers { void analyticsRef.current?.track(event, properties); }, - [analyticsRef.current], + [user?.uid, pathname], ); const page = useCallback( @@ -75,7 +75,7 @@ export function useAnalytics(): AnalyticsHandlers { void analyticsRef.current?.page(name, properties); }, - [analyticsRef.current], + [user?.uid, pathname], ); const identify = useCallback( @@ -85,7 +85,7 @@ export function useAnalytics(): AnalyticsHandlers { deepmerge(properties, user ?? {}), ); }, - [analyticsRef.current], + [user], ); const group = useCallback( @@ -93,7 +93,7 @@ export function useAnalytics(): AnalyticsHandlers { properties.userId ??= user?.uid; void analyticsRef.current?.group(groupId, properties); }, - [analyticsRef.current], + [user?.uid], ); return { diff --git a/frontend/src/hooks/use-authenticated-user.ts b/frontend/src/hooks/use-authenticated-user.ts index a4fae6b3..55016c54 100644 --- a/frontend/src/hooks/use-authenticated-user.ts +++ b/frontend/src/hooks/use-authenticated-user.ts @@ -21,7 +21,7 @@ export function useAuthenticatedUser() { } })(); } - }, []); + }); return user; } diff --git a/frontend/src/hooks/use-prompt-testing.ts b/frontend/src/hooks/use-prompt-testing.ts index 09f16cee..44191648 100644 --- a/frontend/src/hooks/use-prompt-testing.ts +++ b/frontend/src/hooks/use-prompt-testing.ts @@ -56,44 +56,59 @@ export function usePromptTesting({ ); const handlerRef = useRef | null>(null); - const handleMessage = ({ - data, - }: MessageEvent) => { - if (data.finishReason) { - setIsRunningTest(false); - setTestFinishReason(data.finishReason as StreamFinishReason); - - (async () => { - const [record] = await Promise.all([ - handleRetrievePromptTestRecordById({ - applicationId, - projectId, - promptTestRecordId: data.promptTestRecordId!, - }), - onFinish(), - ]); - - setTestRecord(record); - })(); - } - setModelResponses((oldResults) => [...oldResults, data]); - }; + const handleMessage = useCallback( + ({ data }: MessageEvent) => { + if (data.finishReason) { + setIsRunningTest(false); + setTestFinishReason(data.finishReason as StreamFinishReason); - const handleClose = (isError: boolean, reason: string) => { - if (isError) { - onError(reason); - } - if (isRunningTest) { + (async () => { + const [record] = await Promise.all([ + handleRetrievePromptTestRecordById({ + applicationId, + projectId, + promptTestRecordId: data.promptTestRecordId!, + }), + onFinish(), + ]); + + setTestRecord(record); + })(); + } + setModelResponses((oldResults) => [...oldResults, data]); + }, + [applicationId, projectId, onFinish], + ); + + const handleClose = useCallback( + (isError: boolean, reason: string) => { + if (isError) { + onError(reason); + } + if (isRunningTest) { + setIsRunningTest(false); + setTestFinishReason(isError ? StreamFinishReason.ERROR : null); + } + handlerRef.current = null; + }, + [onError, isRunningTest], + ); + + const handleError = useCallback( + (error: WebsocketError) => { setIsRunningTest(false); - setTestFinishReason(isError ? StreamFinishReason.ERROR : null); - } - handlerRef.current = null; - }; - const handleError = (error: WebsocketError) => { + setTestFinishReason(StreamFinishReason.ERROR); + onError(error.message); + }, + [onError], + ); + + const resetState = useCallback(() => { setIsRunningTest(false); - setTestFinishReason(StreamFinishReason.ERROR); - onError(error.message); - }; + setModelResponses([]); + setTestFinishReason(null); + setTestRecord(null); + }, []); useEffect(() => { resetState(); @@ -112,14 +127,14 @@ export function usePromptTesting({ handlerRef.current?.closeSocket(); handlerRef.current = null; }; - }, [applicationId, projectId]); - - const resetState = useCallback(() => { - setIsRunningTest(false); - setModelResponses([]); - setTestFinishReason(null); - setTestRecord(null); - }, []); + }, [ + applicationId, + projectId, + handleClose, + handleError, + handleMessage, + resetState, + ]); const sendMessage = async (testConfig: PromptConfigTest) => { if (handlerRef.current?.isClosed()) { diff --git a/frontend/tests/test-utils.tsx b/frontend/tests/test-utils.tsx index 894146b0..084f42e0 100644 --- a/frontend/tests/test-utils.tsx +++ b/frontend/tests/test-utils.tsx @@ -1,4 +1,4 @@ -/* eslint-disable no-restricted-imports,import/export */ +/* eslint-disable import/export,no-restricted-imports */ import { render, renderHook, From 2d5fd8c656222661c694248238ee600ecba9712b Mon Sep 17 00:00:00 2001 From: Na'aman Hirschfeld Date: Wed, 17 Jan 2024 09:38:00 +0100 Subject: [PATCH 2/5] chore: addressed validation failures --- frontend/src/components/entity-name-input.tsx | 4 +- frontend/src/components/modal.tsx | 2 +- .../config-create-wizard/base-form.tsx | 2 +- .../[configId]/prompt-config-code-snippet.tsx | 37 ++++++++++++------- .../prompt-config-general-settings.tsx | 2 +- .../cohere-model-parameters-form.tsx | 10 ++++- .../openai-model-parameters-form.tsx | 9 ++++- .../openai-prompt-template-form.tsx | 2 +- .../src/components/sign-in/firebase-login.tsx | 2 +- frontend/src/components/tab-navigation.tsx | 3 +- 10 files changed, 50 insertions(+), 23 deletions(-) diff --git a/frontend/src/components/entity-name-input.tsx b/frontend/src/components/entity-name-input.tsx index c2a62599..42bce91f 100644 --- a/frontend/src/components/entity-name-input.tsx +++ b/frontend/src/components/entity-name-input.tsx @@ -34,13 +34,13 @@ export function EntityNameInput({ if (setIsValid) { setIsValid(isLengthValid && isNameValid); } - }, [isLengthValid, isNameValid]); + }, [setIsValid, isLengthValid, isNameValid]); useEffect(() => { if (setIsChanged) { setIsChanged(value !== initialValue.current); } - }, [value]); + }, [setIsChanged, value]); const handleSetValue = (v: string) => { setValue(v.trim()); diff --git a/frontend/src/components/modal.tsx b/frontend/src/components/modal.tsx index 8c2a0906..d43c524f 100644 --- a/frontend/src/components/modal.tsx +++ b/frontend/src/components/modal.tsx @@ -28,7 +28,7 @@ export function Modal({ dialogRef.current?.close(); onClose?.(); } - }, [modalOpen, dialogRef.current]); + }, [onOpen, onClose, modalOpen]); return (
}> diff --git a/frontend/src/components/projects/[projectId]/applications/[applicationId]/config-create-wizard/base-form.tsx b/frontend/src/components/projects/[projectId]/applications/[applicationId]/config-create-wizard/base-form.tsx index 89e8f8e6..ece1747d 100644 --- a/frontend/src/components/projects/[projectId]/applications/[applicationId]/config-create-wizard/base-form.tsx +++ b/frontend/src/components/projects/[projectId]/applications/[applicationId]/config-create-wizard/base-form.tsx @@ -67,7 +67,7 @@ export function PromptConfigBaseForm({ ) { setModelType(OpenAIModelType.Gpt35Turbo); } - }, [modelVendor]); + }, [setModelType, modelVendor, modelType]); return (
diff --git a/frontend/src/components/projects/[projectId]/applications/[applicationId]/configs/[configId]/prompt-config-code-snippet.tsx b/frontend/src/components/projects/[projectId]/applications/[applicationId]/configs/[configId]/prompt-config-code-snippet.tsx index 02571e13..13682796 100644 --- a/frontend/src/components/projects/[projectId]/applications/[applicationId]/configs/[configId]/prompt-config-code-snippet.tsx +++ b/frontend/src/components/projects/[projectId]/applications/[applicationId]/configs/[configId]/prompt-config-code-snippet.tsx @@ -1,6 +1,6 @@ /* eslint-disable sonarjs/no-nested-template-literals */ import { useTranslations } from 'next-intl'; -import { Fragment, memo, useMemo, useState } from 'react'; +import { Fragment, memo, useCallback, useMemo, useState } from 'react'; import { Github } from 'react-bootstrap-icons'; import { CodeSnippet } from '@/components/code-snippet'; @@ -440,17 +440,20 @@ export function PromptConfigCodeSnippet({ const [selectedFramework, setSelectedFramework] = useState('Android'); - const handleDocClick = (tab: FrameworkTab) => { - return () => { - if (initialized) { - track('clickViewDocs', { - category: 'config-code-snippet', - framework: tab.framework, - }); - } - window.open(tab.docs, '_blank')?.focus(); - }; - }; + const handleDocClick = useCallback( + (tab: FrameworkTab) => { + return () => { + if (initialized) { + track('clickViewDocs', { + category: 'config-code-snippet', + framework: tab.framework, + }); + } + window.open(tab.docs, '_blank')?.focus(); + }; + }, + [track, initialized], + ); const mappedTabs = useMemo( () => @@ -460,6 +463,7 @@ export function PromptConfigCodeSnippet({ const ImportSnippet = importSnippetMap[tab.language]; const RequestSnippet = promptRequestSnippetMap[tab.language]; const StreamSnippet = promptStreamSnippetMap[tab.language]; + const replacer = (value: string) => { value = tab.replacers.parametersReplacer( tab.replacers.invokeReplacer(value, expectedVariables), @@ -567,7 +571,14 @@ export function PromptConfigCodeSnippet({ ); }), - [expectedVariables, initialized, selectedFramework], + [ + expectedVariables, + selectedFramework, + handleDocClick, + isDefaultConfig, + promptConfigId, + t, + ], ); return ( diff --git a/frontend/src/components/projects/[projectId]/applications/[applicationId]/configs/[configId]/prompt-config-general-settings.tsx b/frontend/src/components/projects/[projectId]/applications/[applicationId]/configs/[configId]/prompt-config-general-settings.tsx index 17ab141d..e200b3a7 100644 --- a/frontend/src/components/projects/[projectId]/applications/[applicationId]/configs/[configId]/prompt-config-general-settings.tsx +++ b/frontend/src/components/projects/[projectId]/applications/[applicationId]/configs/[configId]/prompt-config-general-settings.tsx @@ -34,7 +34,7 @@ export function PromptConfigGeneralSettings({ ?.filter((c) => c.id !== promptConfig.id) .map((c) => c.name) ?? [] ).includes(value), - [promptConfigs, promptConfig], + [applicationId, promptConfigs, promptConfig], ); async function saveSettings() { diff --git a/frontend/src/components/prompt-display-components/cohere-components/cohere-model-parameters-form.tsx b/frontend/src/components/prompt-display-components/cohere-components/cohere-model-parameters-form.tsx index cad24851..53a89670 100644 --- a/frontend/src/components/prompt-display-components/cohere-components/cohere-model-parameters-form.tsx +++ b/frontend/src/components/prompt-display-components/cohere-components/cohere-model-parameters-form.tsx @@ -113,7 +113,15 @@ export function CohereModelParametersForm({ presencePenalty, temperature, }); - }, [maxTokens, frequencyPenalty, presencePenalty, temperature, p, k]); + }, [ + setParameters, + maxTokens, + frequencyPenalty, + presencePenalty, + temperature, + p, + k, + ]); return (
diff --git a/frontend/src/components/prompt-display-components/openai-components/openai-model-parameters-form.tsx b/frontend/src/components/prompt-display-components/openai-components/openai-model-parameters-form.tsx index d3a74e80..02e4c4c6 100644 --- a/frontend/src/components/prompt-display-components/openai-components/openai-model-parameters-form.tsx +++ b/frontend/src/components/prompt-display-components/openai-components/openai-model-parameters-form.tsx @@ -103,7 +103,14 @@ export function OpenAIModelParametersForm({ temperature, topP, }); - }, [maxTokens, frequencyPenalty, presencePenalty, temperature, topP]); + }, [ + setParameters, + maxTokens, + frequencyPenalty, + presencePenalty, + temperature, + topP, + ]); return (
diff --git a/frontend/src/components/prompt-display-components/openai-components/openai-prompt-template-form.tsx b/frontend/src/components/prompt-display-components/openai-components/openai-prompt-template-form.tsx index 37b1094f..12f536e9 100644 --- a/frontend/src/components/prompt-display-components/openai-components/openai-prompt-template-form.tsx +++ b/frontend/src/components/prompt-display-components/openai-components/openai-prompt-template-form.tsx @@ -34,7 +34,7 @@ export function OpenAIPromptTemplateForm({ useEffect(() => { setMessages([...formMessages]); - }, [formMessages]); + }, [setMessages, formMessages]); const handleSetMessageContent = (index: number) => (content: string) => { const copied = [...formMessages]; diff --git a/frontend/src/components/sign-in/firebase-login.tsx b/frontend/src/components/sign-in/firebase-login.tsx index 001201f3..4db830e7 100644 --- a/frontend/src/components/sign-in/firebase-login.tsx +++ b/frontend/src/components/sign-in/firebase-login.tsx @@ -89,7 +89,7 @@ export function FirebaseLogin({ setLoading(false); })(); } - }, [isInitialized]); + }, [isInitialized, track, identify, router, setLoading, setUser]); const handleLogin = async ( provider: AuthProvider, diff --git a/frontend/src/components/tab-navigation.tsx b/frontend/src/components/tab-navigation.tsx index 0df785f2..cd76b8dc 100644 --- a/frontend/src/components/tab-navigation.tsx +++ b/frontend/src/components/tab-navigation.tsx @@ -25,11 +25,12 @@ export function TabNavigation({ const tab = tabs.find( (t) => window.location.hash.slice(1) === `tab-${t.id}`, ); + if (tab) { onTabChange(tab.id); window.location.hash = ''; } - }, []); + }, [onTabChange, tabs]); return (