fix(FN-2573): prevent stale-closure writes in useFavorites
- Refactor useFavorites callbacks to read latest state instead of closing over stale favorites values - Ensure add/remove/toggle flows update local state and persistence from a consistent source of truth - Add regression tests that reproduce stale-closure timing scenarios and verify favorites remain correct - Expand hook test coverage for rapid consecutive updates to guard against future closure regressions
This commit is contained in:
@@ -93,4 +93,72 @@ describe("useFavorites", () => {
|
||||
|
||||
expect(result.current.favoriteModels).toEqual(["gpt-4o"]);
|
||||
});
|
||||
|
||||
it("toggle model favorite preserves a prior provider favorite change", async () => {
|
||||
const { result } = renderHook(() => useFavorites());
|
||||
|
||||
await waitFor(() => {
|
||||
expect(result.current.favoriteProviders).toEqual(["openai"]);
|
||||
expect(result.current.favoriteModels).toEqual(["gpt-4o"]);
|
||||
});
|
||||
|
||||
await act(async () => {
|
||||
await result.current.toggleFavoriteProvider("anthropic");
|
||||
});
|
||||
|
||||
await act(async () => {
|
||||
await result.current.toggleFavoriteModel("claude-sonnet-4-5");
|
||||
});
|
||||
|
||||
expect(mockUpdateGlobalSettings).toHaveBeenNthCalledWith(2, {
|
||||
favoriteProviders: ["anthropic", "openai"],
|
||||
favoriteModels: ["claude-sonnet-4-5", "gpt-4o"],
|
||||
});
|
||||
});
|
||||
|
||||
it("toggle provider favorite preserves a prior model favorite change", async () => {
|
||||
const { result } = renderHook(() => useFavorites());
|
||||
|
||||
await waitFor(() => {
|
||||
expect(result.current.favoriteProviders).toEqual(["openai"]);
|
||||
expect(result.current.favoriteModels).toEqual(["gpt-4o"]);
|
||||
});
|
||||
|
||||
await act(async () => {
|
||||
await result.current.toggleFavoriteModel("claude-sonnet-4-5");
|
||||
});
|
||||
|
||||
await act(async () => {
|
||||
await result.current.toggleFavoriteProvider("anthropic");
|
||||
});
|
||||
|
||||
expect(mockUpdateGlobalSettings).toHaveBeenNthCalledWith(2, {
|
||||
favoriteProviders: ["anthropic", "openai"],
|
||||
favoriteModels: ["claude-sonnet-4-5", "gpt-4o"],
|
||||
});
|
||||
});
|
||||
|
||||
it("rapid toggles of both model and provider favorites persist correctly", async () => {
|
||||
const { result } = renderHook(() => useFavorites());
|
||||
|
||||
await waitFor(() => {
|
||||
expect(result.current.favoriteProviders).toEqual(["openai"]);
|
||||
expect(result.current.favoriteModels).toEqual(["gpt-4o"]);
|
||||
});
|
||||
|
||||
await act(async () => {
|
||||
const toggleModelPromise = result.current.toggleFavoriteModel("claude-sonnet-4-5");
|
||||
const toggleProviderPromise = result.current.toggleFavoriteProvider("anthropic");
|
||||
await Promise.all([toggleModelPromise, toggleProviderPromise]);
|
||||
});
|
||||
|
||||
expect(mockUpdateGlobalSettings).toHaveBeenNthCalledWith(1, {
|
||||
favoriteProviders: ["openai"],
|
||||
favoriteModels: ["claude-sonnet-4-5", "gpt-4o"],
|
||||
});
|
||||
expect(mockUpdateGlobalSettings).toHaveBeenNthCalledWith(2, {
|
||||
favoriteProviders: ["anthropic", "openai"],
|
||||
favoriteModels: ["claude-sonnet-4-5", "gpt-4o"],
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { useCallback, useEffect, useState } from "react";
|
||||
import { useCallback, useEffect, useRef, useState } from "react";
|
||||
import { fetchModels, updateGlobalSettings, type ModelInfo } from "../api";
|
||||
|
||||
/**
|
||||
@@ -19,11 +19,15 @@ export function useFavorites(): UseFavoritesResult {
|
||||
const [availableModels, setAvailableModels] = useState<ModelInfo[]>([]);
|
||||
const [favoriteProviders, setFavoriteProviders] = useState<string[]>([]);
|
||||
const [favoriteModels, setFavoriteModels] = useState<string[]>([]);
|
||||
const favoriteProvidersRef = useRef<string[]>(favoriteProviders);
|
||||
const favoriteModelsRef = useRef<string[]>(favoriteModels);
|
||||
|
||||
useEffect(() => {
|
||||
fetchModels()
|
||||
.then((response) => {
|
||||
setAvailableModels(response.models);
|
||||
favoriteProvidersRef.current = response.favoriteProviders;
|
||||
favoriteModelsRef.current = response.favoriteModels;
|
||||
setFavoriteProviders(response.favoriteProviders);
|
||||
setFavoriteModels(response.favoriteModels);
|
||||
})
|
||||
@@ -32,45 +36,57 @@ export function useFavorites(): UseFavoritesResult {
|
||||
});
|
||||
}, []);
|
||||
|
||||
const toggleFavoriteProvider = useCallback(async (provider: string) => {
|
||||
const currentFavorites = favoriteProviders;
|
||||
const isFavorite = currentFavorites.includes(provider);
|
||||
const nextFavorites = isFavorite
|
||||
? currentFavorites.filter((p) => p !== provider)
|
||||
: [provider, ...currentFavorites];
|
||||
useEffect(() => {
|
||||
favoriteProvidersRef.current = favoriteProviders;
|
||||
}, [favoriteProviders]);
|
||||
|
||||
setFavoriteProviders(nextFavorites);
|
||||
useEffect(() => {
|
||||
favoriteModelsRef.current = favoriteModels;
|
||||
}, [favoriteModels]);
|
||||
|
||||
const toggleFavoriteProvider = useCallback(async (provider: string) => {
|
||||
const previousFavorites = favoriteProvidersRef.current;
|
||||
const isFavorite = previousFavorites.includes(provider);
|
||||
const nextFavorites = isFavorite
|
||||
? previousFavorites.filter((p) => p !== provider)
|
||||
: [provider, ...previousFavorites];
|
||||
|
||||
favoriteProvidersRef.current = nextFavorites;
|
||||
setFavoriteProviders(() => nextFavorites);
|
||||
|
||||
try {
|
||||
await updateGlobalSettings({
|
||||
favoriteProviders: nextFavorites,
|
||||
favoriteModels,
|
||||
favoriteModels: favoriteModelsRef.current,
|
||||
});
|
||||
} catch (error) {
|
||||
setFavoriteProviders(currentFavorites);
|
||||
favoriteProvidersRef.current = previousFavorites;
|
||||
setFavoriteProviders(() => previousFavorites);
|
||||
throw error;
|
||||
}
|
||||
}, [favoriteProviders, favoriteModels]);
|
||||
}, []);
|
||||
|
||||
const toggleFavoriteModel = useCallback(async (modelId: string) => {
|
||||
const currentFavorites = favoriteModels;
|
||||
const isFavorite = currentFavorites.includes(modelId);
|
||||
const previousFavorites = favoriteModelsRef.current;
|
||||
const isFavorite = previousFavorites.includes(modelId);
|
||||
const nextFavorites = isFavorite
|
||||
? currentFavorites.filter((id) => id !== modelId)
|
||||
: [modelId, ...currentFavorites];
|
||||
? previousFavorites.filter((id) => id !== modelId)
|
||||
: [modelId, ...previousFavorites];
|
||||
|
||||
setFavoriteModels(nextFavorites);
|
||||
favoriteModelsRef.current = nextFavorites;
|
||||
setFavoriteModels(() => nextFavorites);
|
||||
|
||||
try {
|
||||
await updateGlobalSettings({
|
||||
favoriteProviders,
|
||||
favoriteProviders: favoriteProvidersRef.current,
|
||||
favoriteModels: nextFavorites,
|
||||
});
|
||||
} catch (error) {
|
||||
setFavoriteModels(currentFavorites);
|
||||
favoriteModelsRef.current = previousFavorites;
|
||||
setFavoriteModels(() => previousFavorites);
|
||||
throw error;
|
||||
}
|
||||
}, [favoriteModels, favoriteProviders]);
|
||||
}, []);
|
||||
|
||||
return {
|
||||
availableModels,
|
||||
|
||||
Reference in New Issue
Block a user