From 2089b9aa16b950c505525ff87cf04190db10f197 Mon Sep 17 00:00:00 2001 From: Saravana Achu Mac Date: Mon, 4 May 2026 17:50:38 -0700 Subject: [PATCH] fix(D6): clean up strategy editor timers --- .../strategy/CodeStrategyEditor.dom.test.tsx | 15 ++++++ .../strategy/CodeStrategyEditor.tsx | 49 ++++++++++++++++--- .../strategy/VisualRuleBuilder.dom.test.tsx | 26 ++++++++++ .../components/strategy/VisualRuleBuilder.tsx | 36 ++++++++++++-- 4 files changed, 115 insertions(+), 11 deletions(-) create mode 100644 web/src/components/strategy/VisualRuleBuilder.dom.test.tsx diff --git a/web/src/components/strategy/CodeStrategyEditor.dom.test.tsx b/web/src/components/strategy/CodeStrategyEditor.dom.test.tsx index 8d692fa..f698a41 100644 --- a/web/src/components/strategy/CodeStrategyEditor.dom.test.tsx +++ b/web/src/components/strategy/CodeStrategyEditor.dom.test.tsx @@ -32,6 +32,7 @@ describe('CodeStrategyEditor save behavior', () => { }); afterEach(() => { + vi.useRealTimers(); setItemSpy.mockRestore(); }); @@ -71,4 +72,18 @@ describe('CodeStrategyEditor save behavior', () => { expect(await screen.findByText('Profile save failed')).toBeInTheDocument(); }); + + it('clears the saved status timeout when unmounted', async () => { + const clearTimeoutSpy = vi.spyOn(globalThis, 'clearTimeout'); + const user = userEvent.setup(); + + const { unmount } = render(); + await user.click(screen.getByRole('button', { name: /^save$/i })); + + await waitFor(() => expect(screen.getByRole('button', { name: /saved!/i })).toBeInTheDocument()); + unmount(); + + expect(clearTimeoutSpy).toHaveBeenCalled(); + clearTimeoutSpy.mockRestore(); + }); }); diff --git a/web/src/components/strategy/CodeStrategyEditor.tsx b/web/src/components/strategy/CodeStrategyEditor.tsx index ab61f14..d95c589 100644 --- a/web/src/components/strategy/CodeStrategyEditor.tsx +++ b/web/src/components/strategy/CodeStrategyEditor.tsx @@ -2,7 +2,7 @@ * Monaco-based code strategy editor. * Users write a JS strategy function; "Run Backtest" posts it to /api/backtest. */ -import { useState } from 'react'; +import { useCallback, useEffect, useRef, useState } from 'react'; import Editor from '@monaco-editor/react'; import { Play, Save, Copy, RotateCcw } from 'lucide-react'; import { getPlatformAccessToken } from '../../lib/authSession'; @@ -66,6 +66,23 @@ export function CodeStrategyEditor({ const [result, setResult] = useState(null); const [error, setError] = useState(null); const [saved, setSaved] = useState(false); + const mountedRef = useRef(true); + const savedResetTimeoutRef = useRef | null>(null); + + const clearSavedResetTimeout = useCallback(() => { + if (savedResetTimeoutRef.current) { + clearTimeout(savedResetTimeoutRef.current); + savedResetTimeoutRef.current = null; + } + }, []); + + useEffect(() => { + mountedRef.current = true; + return () => { + mountedRef.current = false; + clearSavedResetTimeout(); + }; + }, [clearSavedResetTimeout]); const handleRunBacktest = async () => { setRunning(true); @@ -97,11 +114,17 @@ export function CodeStrategyEditor({ throw new Error(data?.error ?? `Backtest failed (${res.status})`); } // Backend may wrap results in { success, results } or return them flat. - setResult(data?.results ?? data); + if (mountedRef.current) { + setResult(data?.results ?? data); + } } catch (err: any) { - setError(err?.message ?? 'Backtest failed'); + if (mountedRef.current) { + setError(err?.message ?? 'Backtest failed'); + } } finally { - setRunning(false); + if (mountedRef.current) { + setRunning(false); + } } }; @@ -122,12 +145,24 @@ export function CodeStrategyEditor({ code, }, }); + if (!mountedRef.current) return; setSaved(true); + clearSavedResetTimeout(); + savedResetTimeoutRef.current = setTimeout(() => { + if (mountedRef.current) { + setSaved(false); + } + savedResetTimeoutRef.current = null; + }, 3000); } catch (err: any) { - setSaved(false); - setError(err?.message ?? 'Failed to save strategy'); + if (mountedRef.current) { + setSaved(false); + setError(err?.message ?? 'Failed to save strategy'); + } } finally { - setSaving(false); + if (mountedRef.current) { + setSaving(false); + } } }; diff --git a/web/src/components/strategy/VisualRuleBuilder.dom.test.tsx b/web/src/components/strategy/VisualRuleBuilder.dom.test.tsx new file mode 100644 index 0000000..6fe820f --- /dev/null +++ b/web/src/components/strategy/VisualRuleBuilder.dom.test.tsx @@ -0,0 +1,26 @@ +// @vitest-environment jsdom +import { render, screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { VisualRuleBuilder } from './VisualRuleBuilder'; + +describe('VisualRuleBuilder save behavior', () => { + afterEach(() => { + vi.useRealTimers(); + }); + + it('clears the saved message timeout when unmounted', async () => { + const clearTimeoutSpy = vi.spyOn(globalThis, 'clearTimeout'); + const user = userEvent.setup(); + const onSave = vi.fn().mockResolvedValue(undefined); + + const { unmount } = render(); + await user.click(screen.getByRole('button', { name: /save strategy/i })); + + await waitFor(() => expect(screen.getByText('Strategy saved!')).toBeInTheDocument()); + unmount(); + + expect(clearTimeoutSpy).toHaveBeenCalled(); + clearTimeoutSpy.mockRestore(); + }); +}); diff --git a/web/src/components/strategy/VisualRuleBuilder.tsx b/web/src/components/strategy/VisualRuleBuilder.tsx index e0dde00..dbfbe2e 100644 --- a/web/src/components/strategy/VisualRuleBuilder.tsx +++ b/web/src/components/strategy/VisualRuleBuilder.tsx @@ -2,7 +2,7 @@ * Visual drag-and-drop strategy rule builder using @dnd-kit. * Lets users compose IF/THEN trading rules without writing code. */ -import { useState, useCallback } from 'react'; +import { useState, useCallback, useEffect, useRef } from 'react'; import { DndContext, closestCenter, @@ -209,6 +209,23 @@ export function VisualRuleBuilder({ symbol, onSave, onBacktest }: Props) { const [name, setName] = useState('My Strategy'); const [saving, setSaving] = useState(false); const [savedMsg, setSavedMsg] = useState(''); + const mountedRef = useRef(true); + const savedMessageTimeoutRef = useRef | null>(null); + + const clearSavedMessageTimeout = useCallback(() => { + if (savedMessageTimeoutRef.current) { + clearTimeout(savedMessageTimeoutRef.current); + savedMessageTimeoutRef.current = null; + } + }, []); + + useEffect(() => { + mountedRef.current = true; + return () => { + mountedRef.current = false; + clearSavedMessageTimeout(); + }; + }, [clearSavedMessageTimeout]); const sensors = useSensors(useSensor(PointerSensor, { activationConstraint: { distance: 5 }, @@ -238,12 +255,23 @@ export function VisualRuleBuilder({ symbol, onSave, onBacktest }: Props) { setSaving(true); try { await onSave(name.trim(), rules); + if (!mountedRef.current) return; setSavedMsg('Strategy saved!'); - setTimeout(() => setSavedMsg(''), 3000); + clearSavedMessageTimeout(); + savedMessageTimeoutRef.current = setTimeout(() => { + if (mountedRef.current) { + setSavedMsg(''); + } + savedMessageTimeoutRef.current = null; + }, 3000); } catch { - setSavedMsg('Save failed — try again'); + if (mountedRef.current) { + setSavedMsg('Save failed — try again'); + } } finally { - setSaving(false); + if (mountedRef.current) { + setSaving(false); + } } };