refactor: extract common isMounted pattern into reusable useMountedState hook

- Create useMountedState hook to track component mount status
- Refactor useArticleLoader to use shared hook
- Refactor useExternalUrlLoader to use shared hook
- Remove duplicated isMounted pattern across both loaders
- Cleaner, more DRY code with same functionality
This commit is contained in:
Gigi
2025-10-20 16:33:05 +02:00
parent 21335d56dc
commit 811d96dee0
3 changed files with 51 additions and 54 deletions

View File

@@ -6,6 +6,7 @@ import { ReadableContent } from '../services/readerService'
import { Highlight } from '../types/highlights' import { Highlight } from '../types/highlights'
import { NostrEvent } from 'nostr-tools' import { NostrEvent } from 'nostr-tools'
import { UserSettings } from '../services/settingsService' import { UserSettings } from '../services/settingsService'
import { useMountedState } from './useMountedState'
interface UseArticleLoaderProps { interface UseArticleLoaderProps {
naddr: string | undefined naddr: string | undefined
@@ -36,26 +37,23 @@ export function useArticleLoader({
setCurrentArticle, setCurrentArticle,
settings settings
}: UseArticleLoaderProps) { }: UseArticleLoaderProps) {
const isMounted = useMountedState()
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 if (!isMounted()) return
setReaderLoading(true) setReaderLoading(true)
setReaderContent(undefined) setReaderContent(undefined)
setSelectedUrl(`nostr:${naddr}`) setSelectedUrl(`nostr:${naddr}`)
setIsCollapsed(true) setIsCollapsed(true)
// Keep highlights panel collapsed by default - only open on user interaction
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
if (!isMounted) return
setReaderContent({ setReaderContent({
title: article.title, title: article.title,
@@ -72,29 +70,22 @@ export function useArticleLoader({
setCurrentArticleCoordinate(articleCoordinate) setCurrentArticleCoordinate(articleCoordinate)
setCurrentArticleEventId(article.event.id) setCurrentArticleEventId(article.event.id)
setCurrentArticle?.(article.event) setCurrentArticle?.(article.event)
// Set reader loading to false immediately after article content is ready
// Don't wait for highlights to finish loading
setReaderLoading(false) setReaderLoading(false)
// Fetch highlights asynchronously without blocking article display // Fetch highlights asynchronously without blocking article display
// Stream them as they arrive for instant rendering
try { try {
if (!isMounted) return if (!isMounted()) return
setHighlightsLoading(true) setHighlightsLoading(true)
setHighlights([]) // Clear old highlights setHighlights([])
await fetchHighlightsForArticle( await fetchHighlightsForArticle(
relayPool, relayPool,
articleCoordinate, articleCoordinate,
article.event.id, article.event.id,
(highlight) => { (highlight) => {
// Only update if still mounted if (!isMounted()) return
if (!isMounted) return
// 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
const next = [highlight, ...prev] const next = [highlight, ...prev]
@@ -106,13 +97,13 @@ export function useArticleLoader({
} catch (err) { } catch (err) {
console.error('Failed to fetch highlights:', err) console.error('Failed to fetch highlights:', err)
} finally { } finally {
if (isMounted) { if (isMounted()) {
setHighlightsLoading(false) setHighlightsLoading(false)
} }
} }
} catch (err) { } catch (err) {
console.error('Failed to load article:', err) console.error('Failed to load article:', err)
if (isMounted) { if (isMounted()) {
setReaderContent({ setReaderContent({
title: 'Error Loading Article', title: 'Error Loading Article',
html: `<p>Failed to load article: ${err instanceof Error ? err.message : 'Unknown error'}</p>`, html: `<p>Failed to load article: ${err instanceof Error ? err.message : 'Unknown error'}</p>`,
@@ -124,13 +115,7 @@ export function useArticleLoader({
} }
loadArticle() loadArticle()
// 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 // 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
}, [naddr, relayPool, settings]) }, [naddr, relayPool, settings, isMounted])
} }

View File

@@ -7,6 +7,7 @@ import { Highlight } from '../types/highlights'
import { useStoreTimeline } from './useStoreTimeline' import { useStoreTimeline } from './useStoreTimeline'
import { eventToHighlight } from '../services/highlightEventProcessor' import { eventToHighlight } from '../services/highlightEventProcessor'
import { KINDS } from '../config/kinds' import { KINDS } from '../config/kinds'
import { useMountedState } from './useMountedState'
// Helper to extract filename from URL // Helper to extract filename from URL
function getFilenameFromUrl(url: string): string { function getFilenameFromUrl(url: string): string {
@@ -48,6 +49,8 @@ export function useExternalUrlLoader({
setCurrentArticleCoordinate, setCurrentArticleCoordinate,
setCurrentArticleEventId setCurrentArticleEventId
}: UseExternalUrlLoaderProps) { }: UseExternalUrlLoaderProps) {
const isMounted = useMountedState()
// Load cached URL-specific highlights from event store // Load cached URL-specific highlights from event store
const urlFilter = useMemo(() => { const urlFilter = useMemo(() => {
if (!url) return null if (!url) return null
@@ -65,42 +68,33 @@ 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 if (!isMounted()) return
setReaderLoading(true) setReaderLoading(true)
setReaderContent(undefined) setReaderContent(undefined)
setSelectedUrl(url) setSelectedUrl(url)
setIsCollapsed(true) setIsCollapsed(true)
// Clear article-specific state
setCurrentArticleCoordinate(undefined) setCurrentArticleCoordinate(undefined)
setCurrentArticleEventId(undefined) setCurrentArticleEventId(undefined)
try { try {
const content = await fetchReadableContent(url) const content = await fetchReadableContent(url)
// Check if still mounted before updating state if (!isMounted()) return
if (!isMounted) return
setReaderContent(content) setReaderContent(content)
// Set reader loading to false immediately after content is ready
setReaderLoading(false) setReaderLoading(false)
// Fetch highlights for this URL asynchronously // Fetch highlights for this URL asynchronously
try { try {
if (!isMounted) return if (!isMounted()) return
setHighlightsLoading(true) setHighlightsLoading(true)
// Seed with cached highlights first // Seed with cached highlights first
if (cachedUrlHighlights.length > 0) { if (cachedUrlHighlights.length > 0) {
setHighlights((prev) => { setHighlights((prev) => {
// Seed with cache but keep any locally created highlights already in state
const seen = new Set<string>(cachedUrlHighlights.map(h => h.id)) const seen = new Set<string>(cachedUrlHighlights.map(h => h.id))
const localOnly = prev.filter(h => !seen.has(h.id)) const localOnly = prev.filter(h => !seen.has(h.id))
const next = [...cachedUrlHighlights, ...localOnly] const next = [...cachedUrlHighlights, ...localOnly]
@@ -110,18 +104,15 @@ export function useExternalUrlLoader({
setHighlights([]) setHighlights([])
} }
// Check if fetchHighlightsForUrl exists, otherwise skip
if (typeof fetchHighlightsForUrl === 'function') { if (typeof fetchHighlightsForUrl === 'function') {
const seen = new Set<string>() const seen = new Set<string>()
// Seed with cached IDs
cachedUrlHighlights.forEach(h => seen.add(h.id)) cachedUrlHighlights.forEach(h => seen.add(h.id))
await fetchHighlightsForUrl( await fetchHighlightsForUrl(
relayPool, relayPool,
url, url,
(highlight) => { (highlight) => {
// Only update if still mounted if (!isMounted()) return
if (!isMounted) return
if (seen.has(highlight.id)) return if (seen.has(highlight.id)) return
seen.add(highlight.id) seen.add(highlight.id)
@@ -131,22 +122,21 @@ export function useExternalUrlLoader({
return next.sort((a, b) => b.created_at - a.created_at) return next.sort((a, b) => b.created_at - a.created_at)
}) })
}, },
undefined, // settings undefined,
false, // force false,
eventStore || undefined eventStore || undefined
) )
} }
} catch (err) { } catch (err) {
console.error('Failed to fetch highlights:', err) console.error('Failed to fetch highlights:', err)
} finally { } finally {
if (isMounted) { if (isMounted()) {
setHighlightsLoading(false) setHighlightsLoading(false)
} }
} }
} catch (err) { } catch (err) {
console.error('Failed to load external URL:', err) console.error('Failed to load external URL:', err)
if (isMounted) { if (isMounted()) {
// For videos and other media files, use the filename as the title
const filename = getFilenameFromUrl(url) const filename = getFilenameFromUrl(url)
setReaderContent({ setReaderContent({
title: filename, title: filename,
@@ -159,15 +149,9 @@ export function useExternalUrlLoader({
} }
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 // 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, cachedUrlHighlights]) }, [url, relayPool, eventStore, cachedUrlHighlights, isMounted])
// Keep UI highlights synced with cached store updates without reloading content // Keep UI highlights synced with cached store updates without reloading content
useEffect(() => { useEffect(() => {

View File

@@ -0,0 +1,28 @@
import { useRef, useEffect, useCallback } from 'react'
/**
* Hook to track if component is mounted and prevent state updates after unmount.
* Returns a function to check if still mounted.
*
* @example
* const isMounted = useMountedState()
*
* async function loadData() {
* const data = await fetch(...)
* if (isMounted()) {
* setState(data)
* }
* }
*/
export function useMountedState(): () => boolean {
const mountedRef = useRef(true)
useEffect(() => {
return () => {
mountedRef.current = false
}
}, [])
return useCallback(() => mountedRef.current, [])
}