diff --git a/frontend/package-lock.json b/frontend/package-lock.json index c72092d1dd..6ee8de78a1 100644 --- a/frontend/package-lock.json +++ b/frontend/package-lock.json @@ -33,7 +33,7 @@ "@vitejs/plugin-react": "6.0.2", "esbuild": "0.28.0", "eslint": "10.4.1", - "eslint-plugin-react-hooks": "5.2.0", + "eslint-plugin-react-hooks": "7.1.1", "eslint-plugin-react-refresh": "0.5.2", "globals": "17.6.0", "identity-obj-proxy": "3.0.0", @@ -5943,16 +5943,23 @@ } }, "node_modules/eslint-plugin-react-hooks": { - "version": "5.2.0", - "resolved": "https://registry.npmjs.org/eslint-plugin-react-hooks/-/eslint-plugin-react-hooks-5.2.0.tgz", - "integrity": "sha512-+f15FfK64YQwZdJNELETdn5ibXEUQmW1DZL6KXhNnc2heoy/sg9VJJeT7n8TlMWouzWqSWavFkIhHyIbIAEapg==", + "version": "7.1.1", + "resolved": "https://registry.npmjs.org/eslint-plugin-react-hooks/-/eslint-plugin-react-hooks-7.1.1.tgz", + "integrity": "sha512-f2I7Gw6JbvCexzIInuSbZpfdQ44D7iqdWX01FKLvrPgqxoE7oMj8clOfto8U6vYiz4yd5oKu39rRSVOe1zRu0g==", "dev": true, "license": "MIT", + "dependencies": { + "@babel/core": "^7.24.4", + "@babel/parser": "^7.24.4", + "hermes-parser": "^0.25.1", + "zod": "^3.25.0 || ^4.0.0", + "zod-validation-error": "^3.5.0 || ^4.0.0" + }, "engines": { - "node": ">=10" + "node": ">=18" }, "peerDependencies": { - "eslint": "^3.0.0 || ^4.0.0 || ^5.0.0 || ^6.0.0 || ^7.0.0 || ^8.0.0-0 || ^9.0.0" + "eslint": "^3.0.0 || ^4.0.0 || ^5.0.0 || ^6.0.0 || ^7.0.0 || ^8.0.0-0 || ^9.0.0 || ^10.0.0" } }, "node_modules/eslint-plugin-react-refresh": { @@ -6587,6 +6594,23 @@ "node": ">= 0.4" } }, + "node_modules/hermes-estree": { + "version": "0.25.1", + "resolved": "https://registry.npmjs.org/hermes-estree/-/hermes-estree-0.25.1.tgz", + "integrity": "sha512-0wUoCcLp+5Ev5pDW2OriHC2MJCbwLwuRx+gAqMTOkGKJJiBCLjtrvy4PWUGn6MIVefecRpzoOZ/UV6iGdOr+Cw==", + "dev": true, + "license": "MIT" + }, + "node_modules/hermes-parser": { + "version": "0.25.1", + "resolved": "https://registry.npmjs.org/hermes-parser/-/hermes-parser-0.25.1.tgz", + "integrity": "sha512-6pEjquH3rqaI6cYAXYPcz9MS4rY6R4ngRgrgfDshRptUZIc3lw0MCIJIGDj9++mfySOuPTHB4nrSW99BCvOPIA==", + "dev": true, + "license": "MIT", + "dependencies": { + "hermes-estree": "0.25.1" + } + }, "node_modules/html-encoding-sniffer": { "version": "4.0.0", "resolved": "https://registry.npmjs.org/html-encoding-sniffer/-/html-encoding-sniffer-4.0.0.tgz", @@ -10330,6 +10354,29 @@ "funding": { "url": "https://github.com/sponsors/sindresorhus" } + }, + "node_modules/zod": { + "version": "4.4.3", + "resolved": "https://registry.npmjs.org/zod/-/zod-4.4.3.tgz", + "integrity": "sha512-ytENFjIJFl2UwYglde2jchW2Hwm4GJFLDiSXWdTrJQBIN9Fcyp7n4DhxJEiWNAJMV1/BqWfW/kkg71UDcHJyTQ==", + "dev": true, + "license": "MIT", + "funding": { + "url": "https://github.com/sponsors/colinhacks" + } + }, + "node_modules/zod-validation-error": { + "version": "4.0.2", + "resolved": "https://registry.npmjs.org/zod-validation-error/-/zod-validation-error-4.0.2.tgz", + "integrity": "sha512-Q6/nZLe6jxuU80qb/4uJ4t5v2VEZ44lzQjPDhYJNztRQ4wyWc6VF3D3Kb/fAuPetZQnhS3hnajCf9CsWesghLQ==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">=18.0.0" + }, + "peerDependencies": { + "zod": "^3.25.0 || ^4.0.0" + } } } } diff --git a/frontend/package.json b/frontend/package.json index 251e1c7ba0..cae1a19cd2 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -47,7 +47,7 @@ "@vitejs/plugin-react": "6.0.2", "esbuild": "0.28.0", "eslint": "10.4.1", - "eslint-plugin-react-hooks": "5.2.0", + "eslint-plugin-react-hooks": "7.1.1", "eslint-plugin-react-refresh": "0.5.2", "globals": "17.6.0", "identity-obj-proxy": "3.0.0", diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index d4aeac6a7d..73b05f82e2 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -20,15 +20,18 @@ const AUTO_DISMISS_MS = 5_000 function ConnectionBannerContainer() { const { status, reconnectCount } = useConnectionHealth() - const [showReconnected, setShowReconnected] = useState(false) + // Track how many reconnects the user has already had the banner dismissed for. + // `showReconnected` is derived: the banner is visible whenever there are + // un-dismissed reconnects. The auto-dismiss timer bumps `dismissedCount` so + // we avoid calling setState synchronously in an effect body. + const [dismissedCount, setDismissedCount] = useState(0) + const showReconnected = reconnectCount > dismissedCount useEffect(() => { - if (reconnectCount > 0) { - setShowReconnected(true) - const timer = setTimeout(() => setShowReconnected(false), AUTO_DISMISS_MS) - return () => clearTimeout(timer) - } - }, [reconnectCount]) + if (!showReconnected) return + const timer = setTimeout(() => setDismissedCount(reconnectCount), AUTO_DISMISS_MS) + return () => clearTimeout(timer) + }, [showReconnected, reconnectCount]) if (status === 'connected' && !showReconnected) { return null diff --git a/frontend/src/components/Chat/ChatWindow.tsx b/frontend/src/components/Chat/ChatWindow.tsx index 230678e220..3ee4d27005 100644 --- a/frontend/src/components/Chat/ChatWindow.tsx +++ b/frontend/src/components/Chat/ChatWindow.tsx @@ -1,4 +1,4 @@ -import { useState, useRef, useEffect, useCallback } from 'react' +import { useState, useRef, useEffect, useCallback, useMemo } from 'react' import { Button, Text, @@ -81,37 +81,43 @@ export default function ChatWindow({ setAttachmentData(data) }, []) - // Auto-clear stale conversions when their original input no longer matches. + // Auto-prune stale conversions whose original input no longer matches. // For text: the typed text differs from the captured originalValue. // For media: the uploaded base64 changed (or was removed). - useEffect(() => { - setPieceConversions((prev) => { - const entries = Object.entries(prev) - if (entries.length === 0) return prev - let changed = false - const next: Record = {} - for (const [key, conv] of entries) { - if (key === 'text') { - if (conv.originalValue !== chatInputText) { - changed = true - continue - } - } else if (attachmentData[key] !== conv.originalValue) { - changed = true - continue - } + // Deriving this rather than syncing via an effect avoids triggering + // react-hooks/set-state-in-effect and is the pattern recommended by React + // (see frontend-style-guide → "Prefer Derived Values Over Effects"). + const activePieceConversions = useMemo(() => { + const entries = Object.entries(pieceConversions) + if (entries.length === 0) return pieceConversions + const next: Record = {} + let hasStale = false + for (const [key, conv] of entries) { + const stillValid = key === 'text' + ? conv.originalValue === chatInputText + : attachmentData[key] === conv.originalValue + if (stillValid) { next[key] = conv + } else { + hasStale = true } - return changed ? next : prev - }) - }, [chatInputText, attachmentData]) - - // Auto-open conversation sidebar when loading a historical attack with multiple conversations - useEffect(() => { - if (relatedConversationCount && relatedConversationCount > 0) { - setIsPanelOpen(true) } - }, [attackResultId, relatedConversationCount]) + return hasStale ? next : pieceConversions + }, [pieceConversions, chatInputText, attachmentData]) + + // Auto-open conversation sidebar when loading a historical attack with multiple + // conversations. Uses the "adjust state during render" pattern to avoid + // react-hooks/set-state-in-effect. + const [autoOpenedForAttack, setAutoOpenedForAttack] = useState(null) + if ( + attackResultId + && attackResultId !== autoOpenedForAttack + && relatedConversationCount + && relatedConversationCount > 0 + ) { + setAutoOpenedForAttack(attackResultId) + setIsPanelOpen(true) + } // Set by panel click to bypass the in-flight guard on the next useEffect cycle. // This lets users switch to a sending conversation while still protecting // optimistic messages when handleSend internally updates activeConversationId. @@ -126,13 +132,18 @@ export default function ChatWindow({ // Used to restore the user's input when switching back to an in-flight conversation. const pendingUserMessagesRef = useRef>(new Map()) - // Clear internal messages when attack state is reset (e.g. New Attack) - useEffect(() => { + // Clear internal messages when attack state is reset (e.g. New Attack). + // Uses the "adjust state during render" pattern (see React docs: + // https://react.dev/reference/react/useState#storing-information-from-previous-renders) + // instead of a useEffect so we don't trigger react-hooks/set-state-in-effect. + const [prevAttackResultId, setPrevAttackResultId] = useState(attackResultId) + if (attackResultId !== prevAttackResultId) { + setPrevAttackResultId(attackResultId) if (!attackResultId) { setMessages([]) setLoadedConversationId(null) } - }, [attackResultId]) + } // Load messages for a given conversation const loadConversation = useCallback(async (arId: string, convId: string) => { @@ -180,9 +191,11 @@ export default function ChatWindow({ // Synchronous loading derivation: if activeConversationId differs from the // conversation whose messages we've loaded, we're in a transition gap. // This avoids the 1-frame flash between useEffect fire and render. + // Reads `sendingConversations` (state) rather than `sendingConvIdsRef` so the + // computation stays render-safe (the ref is for handlers/effects only). const awaitingConversationLoad = Boolean( activeConversationId && activeConversationId !== loadedConversationId - && !sendingConvIdsRef.current.has(activeConversationId) + && !sendingConversations.has(activeConversationId) ) // Handle conversation selection from the panel @@ -199,7 +212,7 @@ export default function ChatWindow({ if (!activeTarget) { return } // Capture all piece conversions upfront before any async work or state clears - const conversions = { ...pieceConversions } + const conversions = { ...activePieceConversions } const textConversion = conversions['text'] const isTextTextConversion = textConversion?.convertedDataType === 'text' const isTextFileConversion = Boolean(textConversion) && !isTextTextConversion @@ -618,8 +631,8 @@ export default function ChatWindow({ isConverterPanelOpen={isConverterPanelOpen} onInputChange={setChatInputText} onAttachmentsChange={handleAttachmentsChange} - convertedValue={pieceConversions['text']?.convertedDataType === 'text' ? (pieceConversions['text']?.convertedValue ?? null) : null} - originalValue={pieceConversions['text']?.originalValue ?? null} + convertedValue={activePieceConversions['text']?.convertedDataType === 'text' ? (activePieceConversions['text']?.convertedValue ?? null) : null} + originalValue={activePieceConversions['text']?.originalValue ?? null} onClearConversion={() => setPieceConversions((prev) => { const next = { ...prev }; delete next['text']; return next })} onConvertedValueChange={(val) => setPieceConversions((prev) => { const existing = prev['text'] @@ -627,7 +640,7 @@ export default function ChatWindow({ return { ...prev, text: { ...existing, convertedValue: val } } })} convertedFileChip={(() => { - const tc = pieceConversions['text'] + const tc = activePieceConversions['text'] if (!tc || tc.convertedDataType === 'text') return null if (!isPathDataType(tc.convertedDataType)) return null return { @@ -637,8 +650,8 @@ export default function ChatWindow({ } })()} onClearConvertedFileChip={() => setPieceConversions((prev) => { const next = { ...prev }; delete next['text']; return next })} - converterOutputDataTypes={Object.values(pieceConversions).map((c) => c.convertedDataType)} - mediaConversions={Object.entries(pieceConversions) + converterOutputDataTypes={Object.values(activePieceConversions).map((c) => c.convertedDataType)} + mediaConversions={Object.entries(activePieceConversions) .filter(([k]) => k !== 'text') .map(([k, v]) => ({ pieceType: k, convertedValue: v.convertedValue, convertedDataType: v.convertedDataType }))} onClearMediaConversion={(pieceType) => setPieceConversions((prev) => { diff --git a/frontend/src/components/Chat/ConversationPanel.tsx b/frontend/src/components/Chat/ConversationPanel.tsx index 267b0feaf1..569ead5763 100644 --- a/frontend/src/components/Chat/ConversationPanel.tsx +++ b/frontend/src/components/Chat/ConversationPanel.tsx @@ -49,34 +49,64 @@ export default function ConversationPanel({ const styles = useConversationPanelStyles() const [conversations, setConversations] = useState([]) const [mainConversationId, setMainConversationId] = useState(null) - const [isLoading, setIsLoading] = useState(false) + // Initialize to `true` only when we will actually fetch on mount so the spinner + // shows during the initial fetch without needing a synchronous setState inside + // the effect body (which `react-hooks/set-state-in-effect` would flag). + const [isLoading, setIsLoading] = useState(() => attackResultId != null) const [error, setError] = useState(null) - const fetchConversations = useCallback(async () => { + // Counter used by the retry button to re-trigger the fetch effect without + // calling setState synchronously in an effect body. + const [retryCount, setRetryCount] = useState(0) + + useEffect(() => { if (!attackResultId) { - setConversations([]) - setMainConversationId(null) return } - setIsLoading(true) - setError(null) - try { - const response = await attacksApi.getConversations(attackResultId) - setConversations(response.conversations) - setMainConversationId(response.main_conversation_id) - } catch (err) { + let cancelled = false + attacksApi.getConversations(attackResultId) + .then((response) => { + if (cancelled) return + setConversations(response.conversations) + setMainConversationId(response.main_conversation_id) + setError(null) + }) + .catch((err) => { + if (cancelled) return + setConversations([]) + setMainConversationId(null) + setError(toApiError(err).detail) + }) + .finally(() => { + if (cancelled) return + setIsLoading(false) + }) + return () => { + cancelled = true + } + }, [attackResultId, activeConversationId, refreshKey, retryCount]) + + // Reset local state when the attack is unloaded so stale data from a prior + // attack doesn't briefly appear when the user opens a new one. Uses the + // "adjust state during render" pattern instead of a useEffect to satisfy + // react-hooks/set-state-in-effect. + const [prevAttackResultId, setPrevAttackResultId] = useState(attackResultId ?? null) + if (attackResultId !== prevAttackResultId) { + setPrevAttackResultId(attackResultId ?? null) + if (!attackResultId) { setConversations([]) setMainConversationId(null) - setError(toApiError(err).detail) - } finally { + setError(null) setIsLoading(false) } - }, [attackResultId]) + } - useEffect(() => { - fetchConversations() - }, [fetchConversations, activeConversationId, refreshKey]) + const handleRetry = useCallback(() => { + setIsLoading(true) + setError(null) + setRetryCount((c) => c + 1) + }, []) // Expose refresh via a data attribute on the root element so parent can call it // Actually, we'll handle refresh via the attackConversationId dependency @@ -145,7 +175,7 @@ export default function ConversationPanel({ appearance="primary" size="small" icon={} - onClick={fetchConversations} + onClick={handleRetry} data-testid="conversation-retry-btn" > Retry diff --git a/frontend/src/components/Chat/ConverterPanel/ConverterPanel.tsx b/frontend/src/components/Chat/ConverterPanel/ConverterPanel.tsx index d212e97e94..5a9ccc2bd5 100644 --- a/frontend/src/components/Chat/ConverterPanel/ConverterPanel.tsx +++ b/frontend/src/components/Chat/ConverterPanel/ConverterPanel.tsx @@ -45,27 +45,29 @@ export default function ConverterPanel({ onClose, previewText = '', attachmentDa const [panelWidth, setPanelWidth] = useState(320) const isDragging = useRef(false) - const loadConverters = useCallback(async () => { - setIsLoading(true) - setError(null) - - try { - const response = await convertersApi.listConverterCatalog() - setConverters(response.items) - } catch (err) { - setConverters([]) - setSelectedConverterType('') - setQuery('') - setError(toApiError(err).detail) - } finally { - setIsLoading(false) + useEffect(() => { + let cancelled = false + convertersApi.listConverterCatalog() + .then((response) => { + if (cancelled) return + setConverters(response.items) + setError(null) + }) + .catch((err) => { + if (cancelled) return + setConverters([]) + setSelectedConverterType('') + setQuery('') + setError(toApiError(err).detail) + }) + .finally(() => { + if (!cancelled) setIsLoading(false) + }) + return () => { + cancelled = true } }, []) - useEffect(() => { - void loadConverters() - }, [loadConverters]) - // Tabs: always show Text, plus one for each attachment type const tabs = useMemo(() => { const seen = new Set(['text']) @@ -79,15 +81,14 @@ export default function ConverterPanel({ onClose, previewText = '', attachmentDa return result }, [activeInputTypes]) - // Reset to text tab when tabs change and active tab is no longer available - useEffect(() => { - if (!tabs.includes(activeTab)) { - setActiveTab('text') - } - }, [tabs, activeTab]) + // If the selected tab disappears (e.g. user removes an attachment), fall back + // to 'text' for any derivation that depends on the active tab. The actual + // `activeTab` state is only updated on user click; deriving the effective + // value avoids a setState-in-effect that react-hooks/set-state-in-effect would flag. + const effectiveActiveTab = tabs.includes(activeTab) ? activeTab : 'text' // Filter converters by the active tab's input type - const activeDataType = PIECE_TYPE_TO_DATA_TYPE[activeTab] ?? 'text' + const activeDataType = PIECE_TYPE_TO_DATA_TYPE[effectiveActiveTab] ?? 'text' const filteredConverters = useMemo(() => { let filtered = converters.filter((c) => { @@ -194,7 +195,7 @@ export default function ConverterPanel({ onClose, previewText = '', attachmentDa }, []) const handlePreview = useCallback(async () => { - const previewValue = activeTab === 'text' ? previewText : (attachmentData[activeTab] ?? '') + const previewValue = effectiveActiveTab === 'text' ? previewText : (attachmentData[effectiveActiveTab] ?? '') if (!selectedConverterType || !previewValue.trim()) { return } @@ -224,7 +225,7 @@ export default function ConverterPanel({ onClose, previewText = '', attachmentDa } finally { setIsPreviewing(false) } - }, [activeTab, previewText, attachmentData, selectedConverterType, missingRequiredParams, paramValues, activeDataType, getOrCreateConverterInstance]) + }, [effectiveActiveTab, previewText, attachmentData, selectedConverterType, missingRequiredParams, paramValues, activeDataType, getOrCreateConverterInstance]) // Auto-preview is only safe for converters that: // - aren't LLM-based (network cost) @@ -239,26 +240,34 @@ export default function ConverterPanel({ onClose, previewText = '', attachmentDa }, [selectedConverter]) const autoPreviewTimer = useRef | null>(null) - useEffect(() => { - if (autoPreviewTimer.current) { - clearTimeout(autoPreviewTimer.current) - autoPreviewTimer.current = null - } - - const currentPreviewValue = activeTab === 'text' ? previewText : (attachmentData[activeTab] ?? '') - // Clear preview when input is emptied (e.g. after sending) - if (!currentPreviewValue.trim()) { + // Clear preview when input is emptied (e.g. after sending). Uses the "adjust + // state during render" pattern with a prev-value comparison so the reset + // fires exactly once per input transition, even if the input has been empty + // before (e.g. user types, clears, types, clears again). Avoids + // react-hooks/set-state-in-effect. + const currentInput = effectiveActiveTab === 'text' ? previewText : (attachmentData[effectiveActiveTab] ?? '') + const [prevInput, setPrevInput] = useState(currentInput) + if (currentInput !== prevInput) { + setPrevInput(currentInput) + if (!currentInput.trim()) { setPreviewOutput('') setPreviewOutputType('text') setPreviewConverterInstanceId(null) setPreviewError(null) } + } + + useEffect(() => { + if (autoPreviewTimer.current) { + clearTimeout(autoPreviewTimer.current) + autoPreviewTimer.current = null + } if ( !selectedConverter || !supportsAutoPreview || - !currentPreviewValue.trim() || + !currentInput.trim() || missingRequiredParams.length ) { return @@ -273,7 +282,7 @@ export default function ConverterPanel({ onClose, previewText = '', attachmentDa clearTimeout(autoPreviewTimer.current) } } - }, [activeTab, previewText, attachmentData, missingRequiredParams, selectedConverter, supportsAutoPreview, handlePreview]) + }, [currentInput, missingRequiredParams, selectedConverter, supportsAutoPreview, handlePreview]) const handleMouseDown = useCallback(() => { isDragging.current = true document.body.style.cursor = 'col-resize' @@ -320,7 +329,7 @@ export default function ConverterPanel({ onClose, previewText = '', attachmentDa {tabs.length > 1 && ( (null) + if ( + open + && isRoundRobin + && existingTargets + && existingTargets.length > 0 + && existingTargets !== seenExistingTargets + ) { + setSeenExistingTargets(existingTargets) + setAvailableTargets(existingTargets) + } + useEffect(() => { if (!open || !isRoundRobin) return - if (existingTargets && existingTargets.length > 0) { - setAvailableTargets(existingTargets) - return - } + if (existingTargets && existingTargets.length > 0) return let cancelled = false - targetsApi.listTargets(200).then((res) => { - if (!cancelled) setAvailableTargets(res.items) - }).catch(() => { - // Ignore fetch errors — the list will just be empty - }) + targetsApi.listTargets(200) + .then((res) => { + if (!cancelled) setAvailableTargets(res.items) + }) + .catch(() => { + // Ignore fetch errors — the list will just be empty + }) return () => { cancelled = true } }, [open, isRoundRobin, existingTargets]) diff --git a/frontend/src/components/Config/TargetConfig.tsx b/frontend/src/components/Config/TargetConfig.tsx index d9b858d3b4..6344922eb5 100644 --- a/frontend/src/components/Config/TargetConfig.tsx +++ b/frontend/src/components/Config/TargetConfig.tsx @@ -25,41 +25,53 @@ export default function TargetConfig({ activeTarget, onSetActiveTarget }: Target const [loading, setLoading] = useState(true) const [error, setError] = useState(null) const [dialogOpen, setDialogOpen] = useState(false) + // Counter used to re-trigger the fetch effect from event handlers (Refresh, + // dialog close) without invoking setState synchronously in the effect body. + const [refetchCount, setRefetchCount] = useState(0) // Retry fetching targets a few times with backoff. The Vite dev proxy // returns 502 while the backend is still starting, so a single failed // request on initial page load would show a confusing error to the user. - const fetchTargets = useCallback(async () => { + useEffect(() => { const maxRetries = 3 - setLoading(true) - setError(null) - for (let attempt = 0; attempt <= maxRetries; attempt++) { + let cancelled = false + + const attempt = async (n: number): Promise => { try { const response = await targetsApi.listTargets(200) + if (cancelled) return setTargets(response.items) + setError(null) setLoading(false) - return } catch (err) { - if (attempt < maxRetries) { - // Wait before retrying (1s, 2s, 3s) - await new Promise(r => setTimeout(r, (attempt + 1) * 1000)) - } else { - setError(toApiError(err).detail) + if (cancelled) return + if (n < maxRetries) { + await new Promise(r => setTimeout(r, (n + 1) * 1000)) + if (cancelled) return + return attempt(n + 1) } + setError(toApiError(err).detail) + setLoading(false) } } - setLoading(false) + + attempt(0) + return () => { + cancelled = true + } + }, [refetchCount]) + + const fetchTargets = useCallback(() => { + setLoading(true) + setError(null) + setRefetchCount(c => c + 1) }, []) - useEffect(() => { + const handleTargetCreated = useCallback(() => { + setDialogOpen(false) fetchTargets() }, [fetchTargets]) - const handleTargetCreated = async () => { - setDialogOpen(false) - await fetchTargets() - } - return (
diff --git a/frontend/src/components/History/AttackHistory.tsx b/frontend/src/components/History/AttackHistory.tsx index 5eb3870eff..3028f3a428 100644 --- a/frontend/src/components/History/AttackHistory.tsx +++ b/frontend/src/components/History/AttackHistory.tsx @@ -22,6 +22,28 @@ interface AttackHistoryProps { onFiltersChange: (filters: HistoryFilters) => void } +const PAGE_SIZE = 25 + +type ListParams = Parameters[0] + +function buildListParams(filters: HistoryFilters, pageCursor: string | undefined): ListParams { + const labelParams: string[] = [] + for (const op of filters.operator) { labelParams.push(`operator:${op}`) } + for (const op of filters.operation) { labelParams.push(`operation:${op}`) } + labelParams.push(...filters.otherLabels) + + const params: ListParams = { limit: PAGE_SIZE } + if (pageCursor) params.cursor = pageCursor + if (filters.attackTypes.length > 0) params.attack_types = filters.attackTypes + if (filters.outcome) params.outcome = filters.outcome + if (filters.converter.length > 0) params.converter_types = filters.converter + // Match mode is only meaningful with >=2 converters selected. + if (filters.converter.length >= 2) params.converter_types_match = filters.converterMatchMode + if (filters.hasConverters !== undefined) params.has_converters = filters.hasConverters + if (labelParams.length > 0) params.label = labelParams + return params +} + export default function AttackHistory({ onOpenAttack, filters, onFiltersChange }: AttackHistoryProps) { const styles = useAttackHistoryStyles() const [attacks, setAttacks] = useState([]) @@ -40,48 +62,15 @@ export default function AttackHistory({ onOpenAttack, filters, onFiltersChange } const [isLastPage, setIsLastPage] = useState(true) const [page, setPage] = useState(0) - const PAGE_SIZE = 25 + // Bumped from event handlers (Refresh button, pagination) to re-trigger the + // fetch effect without calling setState synchronously inside it. + const [fetchToken, setFetchToken] = useState({ cursor: undefined as string | undefined, nonce: 0 }) - const fetchAttacks = useCallback(async (pageCursor?: string) => { + const fetchAttacks = useCallback((pageCursor?: string) => { setLoading(true) setError(null) - try { - const labelParams: string[] = [] - for (const op of filters.operator) { labelParams.push(`operator:${op}`) } - for (const op of filters.operation) { labelParams.push(`operation:${op}`) } - labelParams.push(...filters.otherLabels) - - // Build request params; set each field only when the filter is active. - const params: Parameters[0] = { limit: PAGE_SIZE } - if (pageCursor) params.cursor = pageCursor - if (filters.attackTypes.length > 0) params.attack_types = filters.attackTypes - if (filters.outcome) params.outcome = filters.outcome - if (filters.converter.length > 0) params.converter_types = filters.converter - // Match mode is only meaningful with >=2 converters selected. - if (filters.converter.length >= 2) params.converter_types_match = filters.converterMatchMode - if (filters.hasConverters !== undefined) params.has_converters = filters.hasConverters - if (labelParams.length > 0) params.label = labelParams - - const response = await attacksApi.listAttacks(params) - setAttacks(response.items.map(attack => ({ ...attack, labels: attack.labels ?? {} }))) - setIsLastPage(!response.pagination.has_more) - setCursor(response.pagination.next_cursor ?? undefined) - } catch (err) { - setAttacks([]) - setError(toApiError(err).detail) - } finally { - setLoading(false) - } - }, [ - filters.attackTypes, - filters.outcome, - filters.converter, - filters.converterMatchMode, - filters.hasConverters, - filters.operator, - filters.operation, - filters.otherLabels, - ]) + setFetchToken(prev => ({ cursor: pageCursor, nonce: prev.nonce + 1 })) + }, []) // Load filter options on mount useEffect(() => { @@ -114,12 +103,50 @@ export default function AttackHistory({ onOpenAttack, filters, onFiltersChange } .catch(() => { /* ignore */ }) }, []) - // Reload when filters change + // Fetch attacks whenever filters change or an event handler bumps fetchToken. + // All setState calls live in .then/.catch/.finally so we don't trigger + // react-hooks/set-state-in-effect. useEffect(() => { - setPage(0) - setCursor(undefined) - fetchAttacks() - }, [fetchAttacks]) + let cancelled = false + attacksApi.listAttacks(buildListParams(filters, fetchToken.cursor)) + .then(response => { + if (cancelled) return + setAttacks(response.items.map(attack => ({ ...attack, labels: attack.labels ?? {} }))) + setIsLastPage(!response.pagination.has_more) + setCursor(response.pagination.next_cursor ?? undefined) + setError(null) + // Reset displayed page index when the trigger is a filter change (no + // explicit cursor). Pagination handlers pass an explicit cursor and + // update `page` themselves. + if (!fetchToken.cursor) setPage(0) + }) + .catch(err => { + if (cancelled) return + setAttacks([]) + setError(toApiError(err).detail) + if (!fetchToken.cursor) setPage(0) + }) + .finally(() => { + if (!cancelled) setLoading(false) + }) + return () => { + cancelled = true + } + // The filter fields are listed individually rather than as `filters` so the + // effect only re-runs when a meaningful sub-field changes (the parent + // creates a new `filters` object on every render). + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [ + filters.attackTypes, + filters.outcome, + filters.converter, + filters.converterMatchMode, + filters.hasConverters, + filters.operator, + filters.operation, + filters.otherLabels, + fetchToken, + ]) const handleNextPage = () => { if (cursor) { diff --git a/frontend/src/components/Home/Home.tsx b/frontend/src/components/Home/Home.tsx index c79fe76327..ffbe8dd476 100644 --- a/frontend/src/components/Home/Home.tsx +++ b/frontend/src/components/Home/Home.tsx @@ -112,13 +112,12 @@ export default function Home({ useEffect(() => { let ignore = false - setLoading(true) - setError(null) attacksApi .listAttacks({ limit: RECENT_ATTACKS_LIMIT }) .then(resp => { if (ignore) return setAttacks(resp.items.map(item => ({ ...item, labels: item.labels ?? {} }))) + setError(null) }) .catch(err => { if (ignore) return