From 177e6e798af26f033424989d0016483ad08092be Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 30 Jan 2026 09:34:20 +0000 Subject: [PATCH] fix: address code review issues - useGitTree: use useStableArray for cloneUrls to fix dependency tracking - useGitTree/useGitBlob: add isMounted checks to prevent state updates after unmount - RepositoryFilesSection: remove unnecessary useMemo for language - FileTreeView: use path instead of hash for React keys - shiki: track failed languages to avoid repeated console warnings --- .../nostr/kinds/RepositoryFilesSection.tsx | 9 ++- src/components/ui/FileTreeView.tsx | 56 ++++++++++--------- src/hooks/useGitBlob.ts | 19 ++++++- src/hooks/useGitTree.ts | 36 +++++++++--- src/lib/shiki.ts | 5 +- 5 files changed, 85 insertions(+), 40 deletions(-) diff --git a/src/components/nostr/kinds/RepositoryFilesSection.tsx b/src/components/nostr/kinds/RepositoryFilesSection.tsx index 01130b9..8acfef7 100644 --- a/src/components/nostr/kinds/RepositoryFilesSection.tsx +++ b/src/components/nostr/kinds/RepositoryFilesSection.tsx @@ -1,4 +1,4 @@ -import { useState, useMemo } from "react"; +import { useState } from "react"; import { FolderGit2, AlertCircle, FileQuestion, Binary } from "lucide-react"; import { useGitTree } from "@/hooks/useGitTree"; import { useGitBlob } from "@/hooks/useGitBlob"; @@ -67,10 +67,9 @@ export function RepositoryFilesSection({ }); // Get the language for syntax highlighting from the file extension - const language = useMemo(() => { - if (!selectedFile) return null; - return getExtension(selectedFile.name) || null; - }, [selectedFile]); + const language = selectedFile + ? getExtension(selectedFile.name) || null + : null; const handleFileSelect = (file: SelectedFile) => { setSelectedFile(file); diff --git a/src/components/ui/FileTreeView.tsx b/src/components/ui/FileTreeView.tsx index 46a0615..7dd3716 100644 --- a/src/components/ui/FileTreeView.tsx +++ b/src/components/ui/FileTreeView.tsx @@ -237,31 +237,37 @@ function TreeContents({ return (
- {sortedEntries.dirs.map((dir) => ( - - ))} - {sortedEntries.files.map((file) => ( - - ))} + {sortedEntries.dirs.map((dir) => { + const dirPath = basePath ? `${basePath}/${dir.name}` : dir.name; + return ( + + ); + })} + {sortedEntries.files.map((file) => { + const filePath = basePath ? `${basePath}/${file.name}` : file.name; + return ( + + ); + })}
); } diff --git a/src/hooks/useGitBlob.ts b/src/hooks/useGitBlob.ts index 89a8df2..25bdc16 100644 --- a/src/hooks/useGitBlob.ts +++ b/src/hooks/useGitBlob.ts @@ -1,4 +1,4 @@ -import { useState, useEffect, useCallback } from "react"; +import { useState, useEffect, useRef, useCallback } from "react"; import { getObject } from "@fiatjaf/git-natural-api"; interface UseGitBlobOptions { @@ -61,6 +61,9 @@ export function useGitBlob({ const [error, setError] = useState(null); const [isBinary, setIsBinary] = useState(false); + // Track mounted state to prevent state updates after unmount + const isMountedRef = useRef(true); + const fetchBlob = useCallback(async () => { if (!serverUrl || !hash) { return; @@ -75,6 +78,8 @@ export function useGitBlob({ try { const object = await getObject(serverUrl, hash); + if (!isMountedRef.current) return; + if (!object || !object.data) { throw new Error("Empty or invalid blob"); } @@ -97,18 +102,28 @@ export function useGitBlob({ } } } catch (e) { + if (!isMountedRef.current) return; + const err = e instanceof Error ? e : new Error(String(e)); console.warn(`[useGitBlob] Failed to fetch blob ${hash}:`, err.message); setError(err); } finally { - setLoading(false); + if (isMountedRef.current) { + setLoading(false); + } } }, [serverUrl, hash]); useEffect(() => { + isMountedRef.current = true; + if (enabled && serverUrl && hash) { fetchBlob(); } + + return () => { + isMountedRef.current = false; + }; }, [enabled, serverUrl, hash, fetchBlob]); return { diff --git a/src/hooks/useGitTree.ts b/src/hooks/useGitTree.ts index fdad32b..1c6a0a6 100644 --- a/src/hooks/useGitTree.ts +++ b/src/hooks/useGitTree.ts @@ -1,9 +1,10 @@ -import { useState, useEffect, useCallback } from "react"; +import { useState, useEffect, useRef, useCallback } from "react"; import { getInfoRefs, getDirectoryTreeAt, MissingCapability, } from "@fiatjaf/git-natural-api"; +import { useStableArray } from "@/hooks/useStable"; import type { DirectoryTree } from "@/lib/git-types"; interface UseGitTreeOptions { @@ -51,8 +52,14 @@ export function useGitTree({ const [error, setError] = useState(null); const [serverUrl, setServerUrl] = useState(null); + // Stabilize cloneUrls to prevent unnecessary re-fetches + const stableCloneUrls = useStableArray(cloneUrls); + + // Track mounted state to prevent state updates after unmount + const isMountedRef = useRef(true); + const fetchTree = useCallback(async () => { - if (cloneUrls.length === 0) { + if (stableCloneUrls.length === 0) { setError(new Error("No clone URLs provided")); return; } @@ -64,11 +71,16 @@ export function useGitTree({ const errors: Error[] = []; - for (const url of cloneUrls) { + for (const url of stableCloneUrls) { + // Check if still mounted before each iteration + if (!isMountedRef.current) return; + try { // Get server info to check capabilities and resolve refs const info = await getInfoRefs(url); + if (!isMountedRef.current) return; + // Only use servers that support filter capability (lightweight fetch) // Skip servers that would require downloading all blobs if (!info.capabilities.includes("filter")) { @@ -105,6 +117,8 @@ export function useGitTree({ // Fetch the tree using lightweight filter (tree only, no blobs) const fetchedTree = await getDirectoryTreeAt(url, resolvedRef); + if (!isMountedRef.current) return; + setTree(fetchedTree); setServerUrl(url); setLoading(false); @@ -128,20 +142,28 @@ export function useGitTree({ } } + if (!isMountedRef.current) return; + // All URLs failed const message = errors.length === 1 ? errors[0].message - : `All ${cloneUrls.length} servers failed`; + : `All ${stableCloneUrls.length} servers failed`; setError(new Error(message)); setLoading(false); - }, [cloneUrls, ref]); + }, [stableCloneUrls, ref]); useEffect(() => { - if (enabled && cloneUrls.length > 0) { + isMountedRef.current = true; + + if (enabled && stableCloneUrls.length > 0) { fetchTree(); } - }, [enabled, fetchTree, cloneUrls.length]); + + return () => { + isMountedRef.current = false; + }; + }, [enabled, fetchTree, stableCloneUrls]); return { tree, diff --git a/src/lib/shiki.ts b/src/lib/shiki.ts index 8f7a7e3..718dd93 100644 --- a/src/lib/shiki.ts +++ b/src/lib/shiki.ts @@ -9,6 +9,7 @@ import { createOnigurumaEngine } from "shiki/engine/oniguruma"; let highlighter: HighlighterCore | null = null; let highlighterPromise: Promise | null = null; const loadedLanguages = new Set(); +const failedLanguages = new Set(); /** * Grimoire dark theme - minimalistic grayscale with high contrast @@ -461,6 +462,7 @@ export async function getHighlighter(): Promise { */ async function loadLanguage(lang: string): Promise { if (lang === "text" || loadedLanguages.has(lang)) return true; + if (failedLanguages.has(lang)) return false; const hl = await getHighlighter(); @@ -471,7 +473,8 @@ async function loadLanguage(lang: string): Promise { loadedLanguages.add(lang); return true; } catch { - // Language not available + // Language not available - track to avoid repeated warnings + failedLanguages.add(lang); console.warn( `[shiki] Language "${lang}" not available, falling back to plaintext`, );