From 21335d56dc32b04a02f9d60a0e2e822c9daa52f2 Mon Sep 17 00:00:00 2001 From: Gigi Date: Mon, 20 Oct 2025 15:00:39 +0200 Subject: [PATCH] fix: prevent infinite loading spinner by fixing race conditions in article/URL loaders - Add isMounted flag to track component lifecycle in useArticleLoader - Add isMounted flag to track component lifecycle in useExternalUrlLoader - Remove setter functions from useEffect dependencies to prevent re-triggers - Add cleanup functions to cancel pending state updates on unmount - Check isMounted before all state updates in async operations - Fixes issue where spinner would spin forever when loading articles --- src/hooks/useArticleLoader.ts | 42 +++++++++++++++++++++----- src/hooks/useExternalUrlLoader.ts | 49 ++++++++++++++++++++++++------- 2 files changed, 72 insertions(+), 19 deletions(-) diff --git a/src/hooks/useArticleLoader.ts b/src/hooks/useArticleLoader.ts index 41b46c7f..93062ab4 100644 --- a/src/hooks/useArticleLoader.ts +++ b/src/hooks/useArticleLoader.ts @@ -39,7 +39,12 @@ export function useArticleLoader({ useEffect(() => { if (!relayPool || !naddr) return + // Track if this effect is still mounted to prevent state updates after unmount + let isMounted = true + const loadArticle = async () => { + if (!isMounted) return + setReaderLoading(true) setReaderContent(undefined) setSelectedUrl(`nostr:${naddr}`) @@ -48,6 +53,10 @@ export function useArticleLoader({ try { const article = await fetchArticleByNaddr(relayPool, naddr, false, settings) + + // Check if still mounted before updating state + if (!isMounted) return + setReaderContent({ title: article.title, markdown: article.markdown, @@ -72,6 +81,8 @@ export function useArticleLoader({ // Fetch highlights asynchronously without blocking article display // Stream them as they arrive for instant rendering try { + if (!isMounted) return + setHighlightsLoading(true) setHighlights([]) // Clear old highlights @@ -80,6 +91,9 @@ export function useArticleLoader({ articleCoordinate, article.event.id, (highlight) => { + // Only update if still mounted + if (!isMounted) return + // Merge streaming results with existing UI state to preserve locally created highlights setHighlights((prev: Highlight[]) => { if (prev.some((h: Highlight) => h.id === highlight.id)) return prev @@ -92,19 +106,31 @@ export function useArticleLoader({ } catch (err) { console.error('Failed to fetch highlights:', err) } finally { - setHighlightsLoading(false) + if (isMounted) { + setHighlightsLoading(false) + } } } catch (err) { console.error('Failed to load article:', err) - setReaderContent({ - title: 'Error Loading Article', - html: `

Failed to load article: ${err instanceof Error ? err.message : 'Unknown error'}

`, - url: `nostr:${naddr}` - }) - setReaderLoading(false) + if (isMounted) { + setReaderContent({ + title: 'Error Loading Article', + html: `

Failed to load article: ${err instanceof Error ? err.message : 'Unknown error'}

`, + url: `nostr:${naddr}` + }) + setReaderLoading(false) + } } } loadArticle() - }, [naddr, relayPool, setSelectedUrl, setReaderContent, setReaderLoading, setIsCollapsed, setHighlights, setHighlightsLoading, setCurrentArticleCoordinate, setCurrentArticleEventId, setCurrentArticle, settings]) + + // Cleanup function to prevent state updates if component unmounts or effect re-runs + return () => { + isMounted = false + } + // Intentionally excluding setter functions from dependencies to prevent race conditions + // The setters are called inside the async function with isMounted checks + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [naddr, relayPool, settings]) } diff --git a/src/hooks/useExternalUrlLoader.ts b/src/hooks/useExternalUrlLoader.ts index e6de688b..bdced4d9 100644 --- a/src/hooks/useExternalUrlLoader.ts +++ b/src/hooks/useExternalUrlLoader.ts @@ -65,7 +65,12 @@ export function useExternalUrlLoader({ useEffect(() => { if (!relayPool || !url) return + // Track if this effect is still mounted to prevent state updates after unmount + let isMounted = true + const loadExternalUrl = async () => { + if (!isMounted) return + setReaderLoading(true) setReaderContent(undefined) setSelectedUrl(url) @@ -76,6 +81,10 @@ export function useExternalUrlLoader({ try { const content = await fetchReadableContent(url) + + // Check if still mounted before updating state + if (!isMounted) return + setReaderContent(content) @@ -84,6 +93,8 @@ export function useExternalUrlLoader({ // Fetch highlights for this URL asynchronously try { + if (!isMounted) return + setHighlightsLoading(true) // Seed with cached highlights first @@ -109,6 +120,9 @@ export function useExternalUrlLoader({ relayPool, url, (highlight) => { + // Only update if still mounted + if (!isMounted) return + if (seen.has(highlight.id)) return seen.add(highlight.id) setHighlights((prev) => { @@ -125,24 +139,35 @@ export function useExternalUrlLoader({ } catch (err) { console.error('Failed to fetch highlights:', err) } finally { - setHighlightsLoading(false) + if (isMounted) { + setHighlightsLoading(false) + } } } catch (err) { console.error('Failed to load external URL:', err) - // For videos and other media files, use the filename as the title - const filename = getFilenameFromUrl(url) - setReaderContent({ - title: filename, - html: `

Failed to load content: ${err instanceof Error ? err.message : 'Unknown error'}

`, - url - }) - setReaderLoading(false) + if (isMounted) { + // For videos and other media files, use the filename as the title + const filename = getFilenameFromUrl(url) + setReaderContent({ + title: filename, + html: `

Failed to load content: ${err instanceof Error ? err.message : 'Unknown error'}

`, + url + }) + setReaderLoading(false) + } } } loadExternalUrl() + + // Cleanup function to prevent state updates if component unmounts or effect re-runs + return () => { + isMounted = false + } + // Intentionally excluding setter functions from dependencies to prevent race conditions + // The setters are called inside the async function with isMounted checks // eslint-disable-next-line react-hooks/exhaustive-deps - }, [url, relayPool, eventStore, setSelectedUrl, setReaderContent, setReaderLoading, setIsCollapsed, setHighlights, setHighlightsLoading, setCurrentArticleCoordinate, setCurrentArticleEventId]) + }, [url, relayPool, eventStore, cachedUrlHighlights]) // Keep UI highlights synced with cached store updates without reloading content useEffect(() => { @@ -155,6 +180,8 @@ export function useExternalUrlLoader({ const next = [...additions, ...prev] return next.sort((a, b) => b.created_at - a.created_at) }) - }, [cachedUrlHighlights, url, setHighlights]) + // setHighlights is intentionally excluded from dependencies - it's stable + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [cachedUrlHighlights, url]) }