From 4b9cb74402e7b86337149ee278bb72a9eacfbf5d Mon Sep 17 00:00:00 2001 From: Zane <75694352+zanesq@users.noreply.github.com> Date: Thu, 10 Apr 2025 08:54:10 -0700 Subject: [PATCH] Fix re-renders from adding too many dependencies to useEffect (#2123) Co-authored-by: Alex Hancock --- .../settings_v2/extensions/ExtensionsSection.tsx | 3 ++- .../settings_v2/models/ModelsSection.tsx | 14 ++------------ .../models/bottom_bar/ModelsBottomBar.tsx | 2 +- .../models/subcomponents/AddModelModal.tsx | 2 +- .../forms/DefaultProviderSetupForm.tsx | 3 ++- ui/desktop/src/components/ui/DeepLinkModal.tsx | 3 ++- ui/desktop/tests/e2e/app.spec.ts | 6 ------ 7 files changed, 10 insertions(+), 23 deletions(-) diff --git a/ui/desktop/src/components/settings_v2/extensions/ExtensionsSection.tsx b/ui/desktop/src/components/settings_v2/extensions/ExtensionsSection.tsx index d21e1008..b0e794c5 100644 --- a/ui/desktop/src/components/settings_v2/extensions/ExtensionsSection.tsx +++ b/ui/desktop/src/components/settings_v2/extensions/ExtensionsSection.tsx @@ -32,7 +32,8 @@ export default function ExtensionsSection() { useEffect(() => { fetchExtensions(); - }, [fetchExtensions]); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []); const handleExtensionToggle = async (extension: FixedExtensionEntry) => { // If extension is enabled, we are trying to toggle if off, otherwise on diff --git a/ui/desktop/src/components/settings_v2/models/ModelsSection.tsx b/ui/desktop/src/components/settings_v2/models/ModelsSection.tsx index a96c2fb5..b9aef231 100644 --- a/ui/desktop/src/components/settings_v2/models/ModelsSection.tsx +++ b/ui/desktop/src/components/settings_v2/models/ModelsSection.tsx @@ -43,19 +43,9 @@ export default function ModelsSection({ setView }: ModelsSectionProps) { }, [read, getProviders]); useEffect(() => { - // Initial load loadModelData(); - - // Set up polling interval to check for changes - const interval = setInterval(() => { - loadModelData(); - }, 1000); // Check every second - - // Clean up interval on unmount - return () => { - clearInterval(interval); - }; - }, [loadModelData]); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []); return (
diff --git a/ui/desktop/src/components/settings_v2/models/bottom_bar/ModelsBottomBar.tsx b/ui/desktop/src/components/settings_v2/models/bottom_bar/ModelsBottomBar.tsx index ba212bc5..406545c2 100644 --- a/ui/desktop/src/components/settings_v2/models/bottom_bar/ModelsBottomBar.tsx +++ b/ui/desktop/src/components/settings_v2/models/bottom_bar/ModelsBottomBar.tsx @@ -27,7 +27,7 @@ export default function ModelsBottomBar({ dropdownRef, setView }: ModelsBottomBa setProvider(modelProvider.provider); setModel(modelProvider.model); })(); - }, [read, getProviders]); + }); // Add click outside handler useEffect(() => { diff --git a/ui/desktop/src/components/settings_v2/models/subcomponents/AddModelModal.tsx b/ui/desktop/src/components/settings_v2/models/subcomponents/AddModelModal.tsx index fac53168..9f68dc0e 100644 --- a/ui/desktop/src/components/settings_v2/models/subcomponents/AddModelModal.tsx +++ b/ui/desktop/src/components/settings_v2/models/subcomponents/AddModelModal.tsx @@ -96,7 +96,7 @@ export const AddModelModal = ({ onClose, setView }: AddModelModalProps) => { if (attemptedSubmit) { validateForm(); } - }, [provider, model, attemptedSubmit, validateForm]); + }, [attemptedSubmit, validateForm]); useEffect(() => { (async () => { diff --git a/ui/desktop/src/components/settings_v2/providers/modal/subcomponents/forms/DefaultProviderSetupForm.tsx b/ui/desktop/src/components/settings_v2/providers/modal/subcomponents/forms/DefaultProviderSetupForm.tsx index cf71f8b7..1807fab0 100644 --- a/ui/desktop/src/components/settings_v2/providers/modal/subcomponents/forms/DefaultProviderSetupForm.tsx +++ b/ui/desktop/src/components/settings_v2/providers/modal/subcomponents/forms/DefaultProviderSetupForm.tsx @@ -99,7 +99,8 @@ export default function DefaultProviderSetupForm({ useEffect(() => { loadConfigValues(); - }, [loadConfigValues]); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []); // Filter parameters to only show required ones const requiredParameters = useMemo(() => { diff --git a/ui/desktop/src/components/ui/DeepLinkModal.tsx b/ui/desktop/src/components/ui/DeepLinkModal.tsx index db7beca5..0027152f 100644 --- a/ui/desktop/src/components/ui/DeepLinkModal.tsx +++ b/ui/desktop/src/components/ui/DeepLinkModal.tsx @@ -61,7 +61,8 @@ export function DeepLinkModal({ botConfig: initialBotConfig, onClose }: DeepLink instructions, activities, }); - }, [instructions, activities, botConfig]); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [instructions, activities]); // Handle adding a new activity const handleAddActivity = () => { diff --git a/ui/desktop/tests/e2e/app.spec.ts b/ui/desktop/tests/e2e/app.spec.ts index a7d96684..e7fd4ee2 100644 --- a/ui/desktop/tests/e2e/app.spec.ts +++ b/ui/desktop/tests/e2e/app.spec.ts @@ -525,12 +525,6 @@ test.describe('Goose App', () => { // Wait a bit and dump HTML to see structure await mainWindow.waitForTimeout(1000); - const html = await mainWindow.evaluate(() => document.documentElement.outerHTML); - console.log('Full page HTML after clicking Output:', html); - - // Also dump just the response area HTML - const responseHtml = await response.evaluate(el => el.outerHTML); - console.log('Response area HTML:', responseHtml); // Take screenshot before trying to find content await mainWindow.screenshot({ path: `test-results/${provider.name.toLowerCase()}-quote-response-debug.png` });