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
This commit is contained in:
Gigi
2025-10-20 15:00:39 +02:00
parent f7e50023a3
commit 21335d56dc
2 changed files with 72 additions and 19 deletions

View File

@@ -39,7 +39,12 @@ export function useArticleLoader({
useEffect(() => { useEffect(() => {
if (!relayPool || !naddr) return if (!relayPool || !naddr) return
// Track if this effect is still mounted to prevent state updates after unmount
let isMounted = true
const loadArticle = async () => { const loadArticle = async () => {
if (!isMounted) return
setReaderLoading(true) setReaderLoading(true)
setReaderContent(undefined) setReaderContent(undefined)
setSelectedUrl(`nostr:${naddr}`) setSelectedUrl(`nostr:${naddr}`)
@@ -48,6 +53,10 @@ export function useArticleLoader({
try { try {
const article = await fetchArticleByNaddr(relayPool, naddr, false, settings) const article = await fetchArticleByNaddr(relayPool, naddr, false, settings)
// Check if still mounted before updating state
if (!isMounted) return
setReaderContent({ setReaderContent({
title: article.title, title: article.title,
markdown: article.markdown, markdown: article.markdown,
@@ -72,6 +81,8 @@ export function useArticleLoader({
// Fetch highlights asynchronously without blocking article display // Fetch highlights asynchronously without blocking article display
// Stream them as they arrive for instant rendering // Stream them as they arrive for instant rendering
try { try {
if (!isMounted) return
setHighlightsLoading(true) setHighlightsLoading(true)
setHighlights([]) // Clear old highlights setHighlights([]) // Clear old highlights
@@ -80,6 +91,9 @@ export function useArticleLoader({
articleCoordinate, articleCoordinate,
article.event.id, article.event.id,
(highlight) => { (highlight) => {
// Only update if still mounted
if (!isMounted) return
// Merge streaming results with existing UI state to preserve locally created highlights // Merge streaming results with existing UI state to preserve locally created highlights
setHighlights((prev: Highlight[]) => { setHighlights((prev: Highlight[]) => {
if (prev.some((h: Highlight) => h.id === highlight.id)) return prev if (prev.some((h: Highlight) => h.id === highlight.id)) return prev
@@ -92,19 +106,31 @@ export function useArticleLoader({
} catch (err) { } catch (err) {
console.error('Failed to fetch highlights:', err) console.error('Failed to fetch highlights:', err)
} finally { } finally {
setHighlightsLoading(false) if (isMounted) {
setHighlightsLoading(false)
}
} }
} catch (err) { } catch (err) {
console.error('Failed to load article:', err) console.error('Failed to load article:', err)
setReaderContent({ if (isMounted) {
title: 'Error Loading Article', setReaderContent({
html: `<p>Failed to load article: ${err instanceof Error ? err.message : 'Unknown error'}</p>`, title: 'Error Loading Article',
url: `nostr:${naddr}` html: `<p>Failed to load article: ${err instanceof Error ? err.message : 'Unknown error'}</p>`,
}) url: `nostr:${naddr}`
setReaderLoading(false) })
setReaderLoading(false)
}
} }
} }
loadArticle() 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])
} }

View File

@@ -65,7 +65,12 @@ export function useExternalUrlLoader({
useEffect(() => { useEffect(() => {
if (!relayPool || !url) return if (!relayPool || !url) return
// Track if this effect is still mounted to prevent state updates after unmount
let isMounted = true
const loadExternalUrl = async () => { const loadExternalUrl = async () => {
if (!isMounted) return
setReaderLoading(true) setReaderLoading(true)
setReaderContent(undefined) setReaderContent(undefined)
setSelectedUrl(url) setSelectedUrl(url)
@@ -76,6 +81,10 @@ export function useExternalUrlLoader({
try { try {
const content = await fetchReadableContent(url) const content = await fetchReadableContent(url)
// Check if still mounted before updating state
if (!isMounted) return
setReaderContent(content) setReaderContent(content)
@@ -84,6 +93,8 @@ export function useExternalUrlLoader({
// Fetch highlights for this URL asynchronously // Fetch highlights for this URL asynchronously
try { try {
if (!isMounted) return
setHighlightsLoading(true) setHighlightsLoading(true)
// Seed with cached highlights first // Seed with cached highlights first
@@ -109,6 +120,9 @@ export function useExternalUrlLoader({
relayPool, relayPool,
url, url,
(highlight) => { (highlight) => {
// Only update if still mounted
if (!isMounted) return
if (seen.has(highlight.id)) return if (seen.has(highlight.id)) return
seen.add(highlight.id) seen.add(highlight.id)
setHighlights((prev) => { setHighlights((prev) => {
@@ -125,24 +139,35 @@ export function useExternalUrlLoader({
} catch (err) { } catch (err) {
console.error('Failed to fetch highlights:', err) console.error('Failed to fetch highlights:', err)
} finally { } finally {
setHighlightsLoading(false) if (isMounted) {
setHighlightsLoading(false)
}
} }
} catch (err) { } catch (err) {
console.error('Failed to load external URL:', err) console.error('Failed to load external URL:', err)
// For videos and other media files, use the filename as the title if (isMounted) {
const filename = getFilenameFromUrl(url) // For videos and other media files, use the filename as the title
setReaderContent({ const filename = getFilenameFromUrl(url)
title: filename, setReaderContent({
html: `<p>Failed to load content: ${err instanceof Error ? err.message : 'Unknown error'}</p>`, title: filename,
url html: `<p>Failed to load content: ${err instanceof Error ? err.message : 'Unknown error'}</p>`,
}) url
setReaderLoading(false) })
setReaderLoading(false)
}
} }
} }
loadExternalUrl() 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 // 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 // Keep UI highlights synced with cached store updates without reloading content
useEffect(() => { useEffect(() => {
@@ -155,6 +180,8 @@ export function useExternalUrlLoader({
const next = [...additions, ...prev] const next = [...additions, ...prev]
return next.sort((a, b) => b.created_at - a.created_at) 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])
} }