From 33d6e5882d01aa97d64c0ede853f1d46afef69e2 Mon Sep 17 00:00:00 2001 From: Gigi Date: Sat, 25 Oct 2025 01:00:36 +0200 Subject: [PATCH] refactor: simplify highlight loading code - Remove redundant fallback mechanisms and backup effects - Remove unnecessary parameters from useArticleLoader interface - Keep only essential highlight loading logic - Maintain DRY principle by eliminating duplicate code - Simplify the codebase while preserving functionality --- src/components/Bookmarks.tsx | 5 +- src/hooks/useArticleLoader.ts | 116 ++++++---------------------------- 2 files changed, 21 insertions(+), 100 deletions(-) diff --git a/src/components/Bookmarks.tsx b/src/components/Bookmarks.tsx index cfc7e86e..d5de67a0 100644 --- a/src/components/Bookmarks.tsx +++ b/src/components/Bookmarks.tsx @@ -246,10 +246,7 @@ const Bookmarks: React.FC = ({ setCurrentArticleCoordinate, setCurrentArticleEventId, setCurrentArticle, - settings, - currentArticleCoordinate, - currentArticleEventId, - highlightsLoading + settings }) // Load external URL if /r/* route is used diff --git a/src/hooks/useArticleLoader.ts b/src/hooks/useArticleLoader.ts index 1d8bb403..d8207835 100644 --- a/src/hooks/useArticleLoader.ts +++ b/src/hooks/useArticleLoader.ts @@ -34,9 +34,6 @@ interface UseArticleLoaderProps { setCurrentArticleEventId: (id: string | undefined) => void setCurrentArticle?: (article: NostrEvent) => void settings?: UserSettings - currentArticleCoordinate?: string - currentArticleEventId?: string - highlightsLoading?: boolean } export function useArticleLoader({ @@ -52,10 +49,7 @@ export function useArticleLoader({ setCurrentArticleCoordinate, setCurrentArticleEventId, setCurrentArticle, - settings, - currentArticleCoordinate, - currentArticleEventId, - highlightsLoading + settings }: UseArticleLoaderProps) { const location = useLocation() const mountedRef = useRef(true) @@ -236,8 +230,8 @@ export function useArticleLoader({ setCurrentArticle?.(article.event) } - // Fetch highlights after content is shown - ensure this happens reliably - const fetchHighlightsForCurrentArticle = async () => { + // Fetch highlights after content is shown + try { if (!mountedRef.current) return const le = latestEvent as NostrEvent | null @@ -246,7 +240,6 @@ export function useArticleLoader({ const eventId = le ? le.id : undefined if (coord && eventId) { - console.log('Loading highlights for article:', coord, eventId) setHighlightsLoading(true) // Clear highlights that don't belong to this article coordinate setHighlights((prev) => { @@ -256,40 +249,28 @@ export function useArticleLoader({ }) }) - try { - await fetchHighlightsForArticle( - relayPool, - coord, - eventId, - (highlight) => { - if (!mountedRef.current) return - if (currentRequestIdRef.current !== requestId) return - console.log('Received highlight:', highlight.id, highlight.content.substring(0, 50)) - setHighlights((prev: Highlight[]) => { - if (prev.some((h: Highlight) => h.id === highlight.id)) return prev - const next = [highlight, ...prev] - return next.sort((a, b) => b.created_at - a.created_at) - }) - }, - settingsRef.current, - false, // force - eventStore || undefined - ) - console.log('Finished loading highlights for article:', coord) - } catch (err) { - console.error('Failed to fetch highlights for article:', coord, err) - } + await fetchHighlightsForArticle( + relayPool, + coord, + eventId, + (highlight) => { + if (!mountedRef.current) return + if (currentRequestIdRef.current !== requestId) return + setHighlights((prev: Highlight[]) => { + if (prev.some((h: Highlight) => h.id === highlight.id)) return prev + const next = [highlight, ...prev] + return next.sort((a, b) => b.created_at - a.created_at) + }) + }, + settingsRef.current, + false, // force + eventStore || undefined + ) } else { - console.log('No article coordinate or event ID available for highlights') // No article event to fetch highlights for - clear and don't show loading setHighlights([]) setHighlightsLoading(false) } - } - - // Always try to fetch highlights, even if we don't have the latest event yet - try { - await fetchHighlightsForCurrentArticle() } catch (err) { console.error('Failed to fetch highlights:', err) } finally { @@ -297,24 +278,6 @@ export function useArticleLoader({ setHighlightsLoading(false) } } - - // Add a fallback mechanism to ensure highlights are loaded - // This helps with cases where the initial highlight loading might fail - const fallbackTimeout = setTimeout(async () => { - if (mountedRef.current && currentRequestIdRef.current === requestId) { - console.log('Fallback: Attempting to load highlights again...') - try { - await fetchHighlightsForCurrentArticle() - } catch (err) { - console.error('Fallback highlight loading failed:', err) - } - } - }, 2000) // Retry after 2 seconds - - // Clean up timeout if component unmounts or new article loads - return () => { - clearTimeout(fallbackTimeout) - } } catch (err) { console.error('Failed to load article:', err) if (mountedRef.current && currentRequestIdRef.current === requestId) { @@ -349,43 +312,4 @@ export function useArticleLoader({ setCurrentArticle ]) - // Additional effect to ensure highlights are loaded when article coordinate changes - // This provides a backup mechanism in case the main loading doesn't work - useEffect(() => { - if (!relayPool || !eventStore) return - - const loadHighlightsIfNeeded = async () => { - // Only load if we have a coordinate but no highlights are loading - if (currentArticleCoordinate && currentArticleEventId && !highlightsLoading) { - console.log('Backup: Loading highlights for coordinate:', currentArticleCoordinate) - try { - setHighlightsLoading(true) - await fetchHighlightsForArticle( - relayPool, - currentArticleCoordinate, - currentArticleEventId, - (highlight) => { - setHighlights((prev: Highlight[]) => { - if (prev.some((h: Highlight) => h.id === highlight.id)) return prev - const next = [highlight, ...prev] - return next.sort((a, b) => b.created_at - a.created_at) - }) - }, - settingsRef.current, - false, // force - eventStore - ) - } catch (err) { - console.error('Backup highlight loading failed:', err) - } finally { - setHighlightsLoading(false) - } - } - } - - // Small delay to ensure the main loading has a chance to work first - const timeout = setTimeout(loadHighlightsIfNeeded, 1000) - - return () => clearTimeout(timeout) - }, [currentArticleCoordinate, currentArticleEventId, relayPool, eventStore, highlightsLoading]) }