perf(renderer): render App once per conversation switch and keep the titles map stable
This commit is contained in:
@@ -4842,6 +4842,41 @@ describe("App", () => {
|
||||
expect(sidebarRenderProbes.row.mock.calls.length).toBeLessThanOrEqual(4);
|
||||
});
|
||||
|
||||
it("renders the App root once when switching between kept conversations (PERF-13)", async () => {
|
||||
const conversations = Array.from({ length: 3 }, (_, c) => ({
|
||||
id: `00000000-0000-4000-9${String(c).padStart(3, "0")}-000000000100`,
|
||||
projectId, title: `Warm switch ${c}`, updatedAt: 2_000 + c,
|
||||
messages: [{
|
||||
id: `00000000-0000-4000-9${String(c).padStart(3, "0")}-000000000101`,
|
||||
role: "assistant" as const, content: `Warm body ${c}`, createdAt: 2_000 + c, state: "complete" as const,
|
||||
}],
|
||||
}));
|
||||
vi.mocked(api.conversations.list).mockResolvedValueOnce(conversations);
|
||||
const { container } = render(<App />);
|
||||
const list = (): HTMLElement => container.querySelector<HTMLElement>(".conversation-list")!;
|
||||
fireEvent.click((await within(list()).findByText("Warm switch 0")).closest("button")!);
|
||||
await screen.findByText("Warm body 0");
|
||||
fireEvent.click(within(list()).getByText("Warm switch 1").closest("button")!);
|
||||
await screen.findByText("Warm body 1");
|
||||
// Let deferred loads (attachment drafts, artifact hydration) settle.
|
||||
await act(() => new Promise((resolve) => setTimeout(resolve, 50)));
|
||||
// useUnviewedCompletions runs once per App render: it counts App renders.
|
||||
assistantTasksProbe.mockClear();
|
||||
act(() => {
|
||||
fireEvent.click(within(list()).getByText("Warm switch 0").closest("button")!);
|
||||
});
|
||||
expect(document.querySelector(".conversation-item--active")).toHaveTextContent("Warm switch 0");
|
||||
// The pane order and composer popups no longer update state during render.
|
||||
expect(assistantTasksProbe).toHaveBeenCalledTimes(1);
|
||||
// A conversation not kept yet adds a pane; that is still one App render.
|
||||
assistantTasksProbe.mockClear();
|
||||
act(() => {
|
||||
fireEvent.click(within(list()).getByText("Warm switch 2").closest("button")!);
|
||||
});
|
||||
expect(document.querySelector(".conversation-item--active")).toHaveTextContent("Warm switch 2");
|
||||
expect(assistantTasksProbe).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("copies and exports a streaming conversation with the text shown on screen", async () => {
|
||||
const { container } = render(<App />);
|
||||
const composer = await screen.findByLabelText("向 GoodBuddy 提问");
|
||||
|
||||
@@ -139,7 +139,7 @@ import {
|
||||
messageRenderBatchSize,
|
||||
type ChatScrollSnapshot,
|
||||
} from "./ChatHistoryPane";
|
||||
import { reconcilePaneOrder } from "./pane-order";
|
||||
import { usePaneOrder } from "./pane-order";
|
||||
import { getConversationDisplayTitle, isUnusedConversation, type Conversation } from "./chat-conversation";
|
||||
import {
|
||||
ConversationListView,
|
||||
@@ -2377,8 +2377,8 @@ function App(): React.JSX.Element {
|
||||
);
|
||||
// Kept panes must keep a stable DOM order. Reordering keyed siblings makes
|
||||
// React detach and reinsert scroll containers, which resets their scrollTop.
|
||||
// The order lives in state and is reconciled during render; the helper
|
||||
// returns the previous array when nothing changed, so this settles at once.
|
||||
// The order is reconciled during render against the previous one (see
|
||||
// usePaneOrder), without a state update, so a switch renders App once.
|
||||
const cachedConversationIds = useMemo(
|
||||
() =>
|
||||
[activeId, ...cachedConversationViews.map((entry) => entry.key)].filter(
|
||||
@@ -2386,15 +2386,7 @@ function App(): React.JSX.Element {
|
||||
),
|
||||
[activeId, cachedConversationViews],
|
||||
);
|
||||
const [conversationPaneOrderState, setConversationPaneOrderState] =
|
||||
useState<readonly string[]>([]);
|
||||
const conversationPaneOrder = reconcilePaneOrder(
|
||||
conversationPaneOrderState,
|
||||
cachedConversationIds,
|
||||
);
|
||||
if (conversationPaneOrder !== conversationPaneOrderState) {
|
||||
setConversationPaneOrderState(conversationPaneOrder);
|
||||
}
|
||||
const conversationPaneOrder = usePaneOrder(cachedConversationIds);
|
||||
const activeRuntimeResolution = useMemo(
|
||||
() =>
|
||||
runtimeSettings
|
||||
|
||||
@@ -0,0 +1,41 @@
|
||||
import { act, renderHook } from "@testing-library/react";
|
||||
import { describe, expect, it } from "vitest";
|
||||
import type { Conversation } from "./chat-conversation";
|
||||
import { useConversationTitles } from "./conversation-selectors";
|
||||
import { createConversationStores } from "./conversation-store";
|
||||
import { usePaneOrder } from "./pane-order";
|
||||
|
||||
function conversation(id: string, title = id): Conversation {
|
||||
return { id, title, updatedAt: 1, messages: [] } as unknown as Conversation;
|
||||
}
|
||||
|
||||
describe("useConversationTitles", () => {
|
||||
it("keeps the same map while only non-title fields change", () => {
|
||||
const { conversations: store } = createConversationStores(
|
||||
[conversation("a"), conversation("b")],
|
||||
{ flushIntervalMs: 0 },
|
||||
);
|
||||
const { result } = renderHook(() => useConversationTitles(store, "New"));
|
||||
const first = result.current;
|
||||
act(() => store.set((current) => current.map((item) => ({ ...item, updatedAt: 2 }))));
|
||||
expect(result.current).toBe(first);
|
||||
act(() => store.set((current) => current.map((item) =>
|
||||
item.id === "b" ? { ...item, title: "Renamed" } : item)));
|
||||
expect(result.current).not.toBe(first);
|
||||
expect(result.current.get("b")).toBe("Renamed");
|
||||
});
|
||||
});
|
||||
|
||||
describe("usePaneOrder", () => {
|
||||
it("keeps existing panes in place and returns the same array when unchanged", () => {
|
||||
const { result, rerender } = renderHook(({ ids }) => usePaneOrder(ids), {
|
||||
initialProps: { ids: ["a", "b"] },
|
||||
});
|
||||
const first = result.current;
|
||||
expect(first).toEqual(["a", "b"]);
|
||||
rerender({ ids: ["b", "a"] });
|
||||
expect(result.current).toBe(first);
|
||||
rerender({ ids: ["c", "a", "b"] });
|
||||
expect(result.current).toEqual(["a", "b", "c"]);
|
||||
});
|
||||
});
|
||||
@@ -1,4 +1,4 @@
|
||||
import { useCallback } from "react";
|
||||
import { useCallback, useMemo } from "react";
|
||||
import type { AssistantTask } from "../../shared/assistant-contracts";
|
||||
import type { Message } from "./ChatTimeline";
|
||||
import { getConversationDisplayTitle, isUnusedConversation, type Conversation } from "./chat-conversation";
|
||||
@@ -21,19 +21,40 @@ export function useConversationCount(store: ConversationStore): number {
|
||||
return useConversationStoreSelector(store, selectCount);
|
||||
}
|
||||
|
||||
/** Display titles by conversation ID. */
|
||||
// Streaming replaces the list on every flush. The selector compares the
|
||||
// titles in order and keeps the previous map while none changed, instead of
|
||||
// building a new map each time only to compare it entry by entry.
|
||||
function createTitlesSelector(
|
||||
defaultTitle: string,
|
||||
): (conversations: Conversation[]) => ReadonlyMap<string, string> {
|
||||
const previous: { ids: string[]; titles: string[]; map?: ReadonlyMap<string, string> } = {
|
||||
ids: [],
|
||||
titles: [],
|
||||
};
|
||||
return (conversations) => {
|
||||
const titles = conversations.map((conversation) =>
|
||||
getConversationDisplayTitle(conversation, defaultTitle));
|
||||
if (
|
||||
previous.map &&
|
||||
previous.ids.length === conversations.length &&
|
||||
conversations.every((conversation, index) =>
|
||||
previous.ids[index] === conversation.id && previous.titles[index] === titles[index])
|
||||
) {
|
||||
return previous.map;
|
||||
}
|
||||
previous.ids = conversations.map((conversation) => conversation.id);
|
||||
previous.titles = titles;
|
||||
previous.map = new Map(previous.ids.map((id, index) => [id, titles[index]!]));
|
||||
return previous.map;
|
||||
};
|
||||
}
|
||||
|
||||
/** Display titles by conversation ID; the same map while no title changes. */
|
||||
export function useConversationTitles(
|
||||
store: ConversationStore,
|
||||
defaultTitle: string,
|
||||
): ReadonlyMap<string, string> {
|
||||
const selector = useCallback(
|
||||
(conversations: Conversation[]) =>
|
||||
new Map(conversations.map((conversation) => [
|
||||
conversation.id,
|
||||
getConversationDisplayTitle(conversation, defaultTitle),
|
||||
])),
|
||||
[defaultTitle],
|
||||
);
|
||||
const selector = useMemo(() => createTitlesSelector(defaultTitle), [defaultTitle]);
|
||||
return useConversationStoreSelector(store, selector, sameMapEntries);
|
||||
}
|
||||
|
||||
|
||||
@@ -1,3 +1,5 @@
|
||||
import { useMemo, useState } from "react";
|
||||
|
||||
/**
|
||||
* Reconciles a stable order for kept-alive panes. Ids already in `previous`
|
||||
* keep their relative position, ids missing from `ids` are dropped and new ids
|
||||
@@ -22,3 +24,19 @@ export function reconcilePaneOrder(
|
||||
}
|
||||
return next;
|
||||
}
|
||||
const previousOrders = new WeakMap<object, readonly string[]>();
|
||||
|
||||
/**
|
||||
* The reconciled pane order, derived during render. The previous order is
|
||||
* kept per hook slot (like useStableDerivedValue), so a conversation switch
|
||||
* renders App once instead of once more for a state update during render.
|
||||
*/
|
||||
export function usePaneOrder(ids: readonly string[]): readonly string[] {
|
||||
const [slot] = useState(() => ({}));
|
||||
const order = useMemo(
|
||||
() => reconcilePaneOrder(previousOrders.get(slot) ?? [], ids),
|
||||
[ids, slot],
|
||||
);
|
||||
previousOrders.set(slot, order);
|
||||
return order;
|
||||
}
|
||||
Reference in New Issue
Block a user