diff --git a/web/src/components/MemoDetailSidebar/MemoDetailSidebar.tsx b/web/src/components/MemoDetailSidebar/MemoDetailSidebar.tsx index 002f31ed..e85fbfe1 100644 --- a/web/src/components/MemoDetailSidebar/MemoDetailSidebar.tsx +++ b/web/src/components/MemoDetailSidebar/MemoDetailSidebar.tsx @@ -141,7 +141,15 @@ const MemoDetailSidebar = ({ memo, className, onShareImageOpen }: Props) => { const { data: creator } = useUser(memo.creator, { enabled: !!memo.creator }); const headings = useMemo(() => extractHeadings(memo.content), [memo.content]); const { referencing, referenced } = useMemo(() => getRelationBuckets(memo.relations, memo.name), [memo.relations, memo.name]); - const resolvedMemos = useResolvedRelationMemos(memo.relations); + const relationMemoNames = useMemo( + () => + [ + ...referencing.map((relation) => getRelationMemo(relation, "referencing")), + ...referenced.map((relation) => getRelationMemo(relation, "referenced")), + ].flatMap((relatedMemo) => (relatedMemo?.name && !relatedMemo.snippet ? [relatedMemo.name] : [])), + [referenced, referencing], + ); + const resolvedMemos = useResolvedRelationMemos(relationMemoNames); const createTime = memo.createTime ? timestampDate(memo.createTime) : undefined; const updateTime = memo.updateTime ? timestampDate(memo.updateTime) : undefined; diff --git a/web/src/components/MemoMetadata/Relation/RelationListEditor.tsx b/web/src/components/MemoMetadata/Relation/RelationListEditor.tsx index 1ded6b47..d30d33f9 100644 --- a/web/src/components/MemoMetadata/Relation/RelationListEditor.tsx +++ b/web/src/components/MemoMetadata/Relation/RelationListEditor.tsx @@ -40,7 +40,14 @@ const RelationItemCard: FC<{ const RelationListEditor: FC = ({ relations, onRelationsChange, parentPage, memoName }) => { const referenceRelations = useMemo(() => getEditorReferenceRelations(relations, memoName), [relations, memoName]); - const resolvedMemos = useResolvedRelationMemos(referenceRelations); + const relatedMemoNames = useMemo( + () => + referenceRelations.flatMap((relation) => + relation.relatedMemo?.name && !relation.relatedMemo.snippet ? [relation.relatedMemo.name] : [], + ), + [referenceRelations], + ); + const resolvedMemos = useResolvedRelationMemos(relatedMemoNames); const handleDeleteRelation = (memoName: string) => { if (onRelationsChange) { diff --git a/web/src/components/MemoMetadata/Relation/RelationListView.tsx b/web/src/components/MemoMetadata/Relation/RelationListView.tsx index 8b38c407..c006fb3f 100644 --- a/web/src/components/MemoMetadata/Relation/RelationListView.tsx +++ b/web/src/components/MemoMetadata/Relation/RelationListView.tsx @@ -17,7 +17,6 @@ interface RelationListViewProps { function RelationListView({ relations, currentMemoName, parentPage, className }: RelationListViewProps) { const t = useTranslate(); const [activeTab, setActiveTab] = useState<"referencing" | "referenced">("referencing"); - const resolvedMemos = useResolvedRelationMemos(relations); const { referencing: referencingRelations, referenced: referencedRelations } = useMemo( () => getRelationBuckets(relations, currentMemoName), @@ -33,6 +32,15 @@ function RelationListView({ relations, currentMemoName, parentPage, className }: const isReferencing = direction === "referencing"; const icon = isReferencing ? LinkIcon : MilestoneIcon; const activeRelations = isReferencing ? referencingRelations : referencedRelations; + const activeMemoNames = useMemo( + () => + activeRelations.flatMap((relation) => { + const memo = getRelationMemo(relation, direction); + return memo?.name && !memo.snippet ? [memo.name] : []; + }), + [activeRelations, direction], + ); + const resolvedMemos = useResolvedRelationMemos(activeMemoNames); return ( { +export const useResolvedRelationMemos = (memoNames: string[]) => { + const queryClient = useQueryClient(); const [resolvedMemos, setResolvedMemos] = useState>({}); const missingMemoNames = useMemo(() => { - const names = new Set(); - - for (const relation of relations) { - for (const memo of [relation.memo, relation.relatedMemo]) { - if (memo?.name && !memo.snippet && !resolvedMemos[memo.name]) { - names.add(memo.name); - } - } - } - - return [...names]; - }, [relations, resolvedMemos]); + return Array.from(new Set(memoNames)).filter((name) => name && !resolvedMemos[name]); + }, [memoNames, resolvedMemos]); useEffect(() => { if (missingMemoNames.length === 0) { @@ -32,7 +23,7 @@ export const useResolvedRelationMemos = (relations: MemoRelation[]) => { try { const memos = await Promise.all( missingMemoNames.map(async (name) => { - const memo = await memoServiceClient.getMemo({ name }); + const memo = findMemoInCollectionQueries(queryClient, name) ?? (await queryClient.fetchQuery(memoDetailQueryOptions(name))); return create(MemoRelation_MemoSchema, { name: memo.name, snippet: memo.snippet }); }), ); @@ -56,7 +47,7 @@ export const useResolvedRelationMemos = (relations: MemoRelation[]) => { return () => { cancelled = true; }; - }, [missingMemoNames]); + }, [missingMemoNames, queryClient]); return resolvedMemos; }; diff --git a/web/src/components/PagedMemoList/PagedMemoList.tsx b/web/src/components/PagedMemoList/PagedMemoList.tsx index 962e62ff..dd188590 100644 --- a/web/src/components/PagedMemoList/PagedMemoList.tsx +++ b/web/src/components/PagedMemoList/PagedMemoList.tsx @@ -1,16 +1,13 @@ -import { useQueryClient } from "@tanstack/react-query"; import { ArrowUpIcon, LoaderCircleIcon } from "lucide-react"; import { type ReactElement, type ReactNode, useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState } from "react"; import { MentionResolutionProvider } from "@/components/MemoContent/MentionResolutionContext"; import { Button } from "@/components/ui/button"; -import { userServiceClient } from "@/connect"; import { useMemoFilterContext } from "@/contexts/MemoFilterContext"; import { useNewMemo } from "@/contexts/NewMemoContext"; import { useView } from "@/contexts/ViewContext"; import { useDelayedFlag } from "@/hooks/useDelayedFlag"; import { useInfiniteMemos } from "@/hooks/useMemoQueries"; import { hoistMemoToFront } from "@/hooks/useMemoSorting"; -import { userKeys } from "@/hooks/useUserQueries"; import { DEFAULT_LIST_MEMOS_PAGE_SIZE, LOADING_INDICATOR_DELAY_MS } from "@/lib/constants"; import { cn } from "@/lib/utils"; import { State } from "@/types/proto/api/v1/common_pb"; @@ -101,7 +98,6 @@ function useAutoFetchWhenNotScrollable({ const PagedMemoList = (props: Props) => { const t = useTranslate(); - const queryClient = useQueryClient(); const { filters } = useMemoFilterContext(); const { maxColumns, compactMode } = useView(); // maxColumns is a ceiling: 1 = single reading column, 0 = as many as fit. The single @@ -154,26 +150,6 @@ const PagedMemoList = (props: Props) => { return hoistMemoToFront(sorted, newMemoName); }, [memos, props.listSort, newMemoName]); - // Prefetch creators when new data arrives to improve performance - useEffect(() => { - if (!data?.pages || !props.showCreator) return; - - const lastPage = data.pages[data.pages.length - 1]; - if (!lastPage?.memos) return; - - const uniqueCreators = Array.from(new Set(lastPage.memos.map((memo) => memo.creator))); - for (const creator of uniqueCreators) { - void queryClient.prefetchQuery({ - queryKey: userKeys.detail(creator), - queryFn: async () => { - const user = await userServiceClient.getUser({ name: creator }); - return user; - }, - staleTime: 1000 * 60 * 5, - }); - } - }, [data?.pages, props.showCreator, queryClient]); - // Auto-fetch hook: fetches more content when page isn't scrollable useAutoFetchWhenNotScrollable({ hasNextPage, diff --git a/web/src/hooks/useMemoQueries.ts b/web/src/hooks/useMemoQueries.ts index 73133598..23055313 100644 --- a/web/src/hooks/useMemoQueries.ts +++ b/web/src/hooks/useMemoQueries.ts @@ -1,7 +1,14 @@ import { create } from "@bufbuild/protobuf"; import { FieldMaskSchema } from "@bufbuild/protobuf/wkt"; -import type { InfiniteData } from "@tanstack/react-query"; -import { useInfiniteQuery, useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; +import { + type InfiniteData, + type QueryClient, + queryOptions, + useInfiniteQuery, + useMutation, + useQuery, + useQueryClient, +} from "@tanstack/react-query"; import { memoServiceClient } from "@/connect"; import { userKeys } from "@/hooks/useUserQueries"; import { DEFAULT_LIST_MEMOS_PAGE_SIZE } from "@/lib/constants"; @@ -19,6 +26,13 @@ export const memoKeys = { linkMetadata: (url: string) => [...memoKeys.all, "linkMetadata", url] as const, }; +export const memoDetailQueryOptions = (name: string) => + queryOptions({ + queryKey: memoKeys.detail(name), + queryFn: () => memoServiceClient.getMemo({ name }), + staleTime: 1000 * 10, + }); + type MemoPatch = Partial & Pick; type MemoCollectionQueryData = ListMemosResponse | InfiniteData; @@ -94,7 +108,7 @@ function findMemoInQueryData(data: unknown, name: string): Memo | undefined { return undefined; } -function findMemoInCollectionQueries(queryClient: ReturnType, name: string): Memo | undefined { +export function findMemoInCollectionQueries(queryClient: QueryClient, name: string): Memo | undefined { for (const [, data] of queryClient.getQueriesData({ queryKey: memoKeys.all })) { const memo = findMemoInQueryData(data, name); if (memo) { @@ -105,7 +119,7 @@ function findMemoInCollectionQueries(queryClient: ReturnType, update: MemoPatch) { +function patchMemoInCollectionQueries(queryClient: QueryClient, update: MemoPatch) { queryClient.setQueriesData({ queryKey: memoKeys.all }, (data) => patchMemoListQueryData(data, update)); } @@ -141,13 +155,8 @@ export function useInfiniteMemos(request: Partial = {}, option export function useMemo(name: string, options?: { enabled?: boolean }) { return useQuery({ - queryKey: memoKeys.detail(name), - queryFn: async () => { - const memo = await memoServiceClient.getMemo({ name }); - return memo; - }, + ...memoDetailQueryOptions(name), enabled: options?.enabled ?? true, - staleTime: 1000 * 10, // 10 seconds - reduced to prevent stale data in collaborative editing }); } diff --git a/web/src/hooks/useUserQueries.ts b/web/src/hooks/useUserQueries.ts index 6854961c..11502a76 100644 --- a/web/src/hooks/useUserQueries.ts +++ b/web/src/hooks/useUserQueries.ts @@ -1,6 +1,6 @@ import { create } from "@bufbuild/protobuf"; import { FieldMaskSchema } from "@bufbuild/protobuf/wkt"; -import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; +import { queryOptions, useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; import { shortcutServiceClient, userServiceClient } from "@/connect"; import useCurrentUser from "@/hooks/useCurrentUser"; import { buildUserSettingName } from "@/lib/resource-names"; @@ -16,6 +16,7 @@ import { } from "@/types/proto/api/v1/user_service_pb"; const BATCH_GET_USERS_LIMIT = 100; +const USER_PROFILE_STALE_TIME = 1000 * 60 * 5; type ListAllUserStatsQuery = Pick; // Query keys factory @@ -33,15 +34,17 @@ export const userKeys = { byUsernames: (usernames: string[]) => [...userKeys.all, "byUsernames", ...[...usernames].sort()] as const, }; +const userDetailQueryOptions = (name: string) => + queryOptions({ + queryKey: userKeys.detail(name), + queryFn: () => userServiceClient.getUser({ name }), + staleTime: USER_PROFILE_STALE_TIME, + }); + export function useUser(name: string, options?: { enabled?: boolean }) { return useQuery({ - queryKey: userKeys.detail(name), - queryFn: async () => { - const user = await userServiceClient.getUser({ name }); - return user; - }, + ...userDetailQueryOptions(name), enabled: options?.enabled ?? true, - staleTime: 1000 * 60 * 5, // 5 minutes - user profiles don't change often }); } @@ -255,6 +258,7 @@ export function useUpdateUserGeneralSetting(currentUserName?: string) { // Hook to fetch multiple users by names (returns Map) export function useUsersByNames(names: string[]) { + const queryClient = useQueryClient(); const enabled = names.length > 0; const uniqueNames = Array.from(new Set(names)); @@ -264,7 +268,7 @@ export function useUsersByNames(names: string[]) { const users = await Promise.all( uniqueNames.map(async (name) => { try { - const user = await userServiceClient.getUser({ name }); + const user = await queryClient.fetchQuery(userDetailQueryOptions(name)); return { name, user }; } catch { return { name, user: undefined }; @@ -279,12 +283,13 @@ export function useUsersByNames(names: string[]) { return userMap; }, enabled, - staleTime: 1000 * 60 * 5, // 5 minutes - user profiles don't change often + staleTime: USER_PROFILE_STALE_TIME, }); } // Hook to fetch multiple users by usernames (returns Map) export function useUsersByUsernames(usernames: string[], options?: { enabled?: boolean }) { + const queryClient = useQueryClient(); const enabled = (options?.enabled ?? true) && usernames.length > 0; const uniqueUsernames = Array.from(new Set(usernames)); @@ -297,7 +302,11 @@ export function useUsersByUsernames(usernames: string[], options?: { enabled?: b } const responses = await Promise.all(batches.map((batch) => userServiceClient.batchGetUsers({ usernames: batch }))); - const usersByUsername = new Map(responses.flatMap((response) => response.users).map((user) => [user.username, user] as const)); + const users = responses.flatMap((response) => response.users); + const usersByUsername = new Map(users.map((user) => [user.username, user] as const)); + for (const user of users) { + queryClient.setQueryData(userKeys.detail(user.name), user); + } const userMap = new Map(); for (const username of uniqueUsernames) { @@ -306,6 +315,6 @@ export function useUsersByUsernames(usernames: string[], options?: { enabled?: b return userMap; }, enabled, - staleTime: 1000 * 60 * 5, + staleTime: USER_PROFILE_STALE_TIME, }); } diff --git a/web/tests/query-deduplication.test.tsx b/web/tests/query-deduplication.test.tsx new file mode 100644 index 00000000..2711de7a --- /dev/null +++ b/web/tests/query-deduplication.test.tsx @@ -0,0 +1,115 @@ +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { renderHook, waitFor } from "@testing-library/react"; +import type { PropsWithChildren } from "react"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { useResolvedRelationMemos } from "@/components/MemoMetadata/Relation/useResolvedRelationMemos"; +import { memoKeys } from "@/hooks/useMemoQueries"; +import { useUser, userKeys, useUsersByNames, useUsersByUsernames } from "@/hooks/useUserQueries"; +import type { Memo } from "@/types/proto/api/v1/memo_service_pb"; +import type { User } from "@/types/proto/api/v1/user_service_pb"; + +const clients = vi.hoisted(() => ({ + batchGetUsers: vi.fn(), + getMemo: vi.fn(), + getUser: vi.fn(), +})); + +vi.mock("@/connect", () => ({ + memoServiceClient: { + getMemo: clients.getMemo, + }, + shortcutServiceClient: {}, + userServiceClient: { + batchGetUsers: clients.batchGetUsers, + getUser: clients.getUser, + }, +})); + +const createQueryClient = () => + new QueryClient({ + defaultOptions: { + queries: { + retry: false, + }, + }, + }); + +const createWrapper = (queryClient: QueryClient) => + function QueryWrapper({ children }: PropsWithChildren) { + return {children}; + }; + +describe("query deduplication", () => { + beforeEach(() => { + clients.batchGetUsers.mockReset(); + clients.getMemo.mockReset(); + clients.getUser.mockReset(); + }); + + it("fetches each user name once across individual and overlapping group queries", async () => { + clients.getUser.mockImplementation(async ({ name }: { name: string }) => ({ name, username: name }) as User); + const queryClient = createQueryClient(); + + const { result } = renderHook( + () => ({ + individual: useUser("users/1"), + firstGroup: useUsersByNames(["users/1", "users/2"]), + secondGroup: useUsersByNames(["users/2", "users/3"]), + }), + { wrapper: createWrapper(queryClient) }, + ); + + await waitFor(() => { + expect(result.current.individual.isSuccess).toBe(true); + expect(result.current.firstGroup.isSuccess).toBe(true); + expect(result.current.secondGroup.isSuccess).toBe(true); + }); + + expect(clients.getUser.mock.calls.map(([request]) => request.name).sort()).toEqual(["users/1", "users/2", "users/3"]); + }); + + it("seeds user detail queries from a username batch response", async () => { + const alice = { name: "users/1", username: "alice" } as User; + clients.batchGetUsers.mockResolvedValue({ users: [alice] }); + const queryClient = createQueryClient(); + const wrapper = createWrapper(queryClient); + + const batch = renderHook(() => useUsersByUsernames(["alice"]), { wrapper }); + await waitFor(() => expect(batch.result.current.isSuccess).toBe(true)); + + expect(queryClient.getQueryData(userKeys.detail(alice.name))).toBe(alice); + + const detail = renderHook(() => useUser(alice.name), { wrapper }); + await waitFor(() => expect(detail.result.current.isSuccess).toBe(true)); + expect(clients.getUser).not.toHaveBeenCalled(); + }); + + it("reuses memo list data and canonical detail queries when resolving relations", async () => { + const cachedMemo = { name: "memos/cached", snippet: "Already in the list" } as Memo; + const missingMemo = { name: "memos/missing", snippet: "Fetched once" } as Memo; + clients.getMemo.mockResolvedValue(missingMemo); + + const queryClient = createQueryClient(); + queryClient.setQueryData(memoKeys.list({}), { + pages: [{ memos: [cachedMemo], nextPageToken: "" }], + pageParams: [""], + }); + + const { result } = renderHook( + () => ({ + first: useResolvedRelationMemos([cachedMemo.name, missingMemo.name]), + second: useResolvedRelationMemos([missingMemo.name]), + }), + { wrapper: createWrapper(queryClient) }, + ); + + await waitFor(() => { + expect(result.current.first[cachedMemo.name]?.snippet).toBe(cachedMemo.snippet); + expect(result.current.first[missingMemo.name]?.snippet).toBe(missingMemo.snippet); + expect(result.current.second[missingMemo.name]?.snippet).toBe(missingMemo.snippet); + }); + + expect(clients.getMemo).toHaveBeenCalledTimes(1); + expect(clients.getMemo).toHaveBeenCalledWith({ name: missingMemo.name }); + }); +});