mirror of
https://github.com/purrgrammer/grimoire.git
synced 2026-04-12 08:27:27 +02:00
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
This commit is contained in:
@@ -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);
|
||||
|
||||
@@ -237,31 +237,37 @@ function TreeContents({
|
||||
|
||||
return (
|
||||
<div>
|
||||
{sortedEntries.dirs.map((dir) => (
|
||||
<TreeNode
|
||||
key={dir.hash}
|
||||
name={dir.name}
|
||||
hash={dir.hash}
|
||||
path={basePath ? `${basePath}/${dir.name}` : dir.name}
|
||||
isDirectory={true}
|
||||
content={dir.content}
|
||||
onFileSelect={onFileSelect}
|
||||
selectedPath={selectedPath}
|
||||
depth={depth}
|
||||
/>
|
||||
))}
|
||||
{sortedEntries.files.map((file) => (
|
||||
<TreeNode
|
||||
key={file.hash}
|
||||
name={file.name}
|
||||
hash={file.hash}
|
||||
path={basePath ? `${basePath}/${file.name}` : file.name}
|
||||
isDirectory={false}
|
||||
onFileSelect={onFileSelect}
|
||||
selectedPath={selectedPath}
|
||||
depth={depth}
|
||||
/>
|
||||
))}
|
||||
{sortedEntries.dirs.map((dir) => {
|
||||
const dirPath = basePath ? `${basePath}/${dir.name}` : dir.name;
|
||||
return (
|
||||
<TreeNode
|
||||
key={dirPath}
|
||||
name={dir.name}
|
||||
hash={dir.hash}
|
||||
path={dirPath}
|
||||
isDirectory={true}
|
||||
content={dir.content}
|
||||
onFileSelect={onFileSelect}
|
||||
selectedPath={selectedPath}
|
||||
depth={depth}
|
||||
/>
|
||||
);
|
||||
})}
|
||||
{sortedEntries.files.map((file) => {
|
||||
const filePath = basePath ? `${basePath}/${file.name}` : file.name;
|
||||
return (
|
||||
<TreeNode
|
||||
key={filePath}
|
||||
name={file.name}
|
||||
hash={file.hash}
|
||||
path={filePath}
|
||||
isDirectory={false}
|
||||
onFileSelect={onFileSelect}
|
||||
selectedPath={selectedPath}
|
||||
depth={depth}
|
||||
/>
|
||||
);
|
||||
})}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
@@ -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<Error | null>(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 {
|
||||
|
||||
@@ -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<Error | null>(null);
|
||||
const [serverUrl, setServerUrl] = useState<string | null>(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,
|
||||
|
||||
@@ -9,6 +9,7 @@ import { createOnigurumaEngine } from "shiki/engine/oniguruma";
|
||||
let highlighter: HighlighterCore | null = null;
|
||||
let highlighterPromise: Promise<HighlighterCore> | null = null;
|
||||
const loadedLanguages = new Set<string>();
|
||||
const failedLanguages = new Set<string>();
|
||||
|
||||
/**
|
||||
* Grimoire dark theme - minimalistic grayscale with high contrast
|
||||
@@ -461,6 +462,7 @@ export async function getHighlighter(): Promise<HighlighterCore> {
|
||||
*/
|
||||
async function loadLanguage(lang: string): Promise<boolean> {
|
||||
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<boolean> {
|
||||
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`,
|
||||
);
|
||||
|
||||
Reference in New Issue
Block a user