From cafc717ddeaa861862273724a303deea3b679a77 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Salavisa?= Date: Sun, 6 Sep 2026 09:45:29 +0100 Subject: [PATCH] perf(web): isolate sidebar task tree renders (#3435) Co-authored-by: Kandev Agent --- .../task-session-sidebar-grouped-view.test.ts | 56 ++++ .../task/task-session-sidebar-grouped-view.ts | 146 +++++++++- .../task-switcher-render-stability.test.tsx | 257 ++++++++++++++++++ .../web/components/task/task-switcher-row.tsx | 6 +- .../components/task/task-switcher-tree.tsx | 162 +++++++++-- apps/web/components/task/task-switcher.tsx | 147 ++++++++-- .../use-workspace-sidebar-tasks.test.ts | 61 +++++ .../kanban/use-workspace-sidebar-tasks.ts | 128 +++++++-- 8 files changed, 874 insertions(+), 89 deletions(-) create mode 100644 apps/web/components/task/task-session-sidebar-grouped-view.test.ts create mode 100644 apps/web/components/task/task-switcher-render-stability.test.tsx diff --git a/apps/web/components/task/task-session-sidebar-grouped-view.test.ts b/apps/web/components/task/task-session-sidebar-grouped-view.test.ts new file mode 100644 index 0000000000..3a2d6a5a4a --- /dev/null +++ b/apps/web/components/task/task-session-sidebar-grouped-view.test.ts @@ -0,0 +1,56 @@ +import { renderHook } from "@testing-library/react"; +import { describe, expect, it, vi } from "vitest"; +import type { TaskSwitcherItem } from "./task-switcher-types"; + +const view = { + id: "view-1", + name: "By workflow", + filters: [], + sort: { key: "title" as const, direction: "asc" as const }, + group: "workflow" as const, + collapsedGroups: [], +}; + +const prefs = { + pinnedTaskIds: [], + orderedTaskIds: [], + subtaskOrderByParentId: {}, + togglePinnedTask: vi.fn(), + handleReorderGroup: vi.fn(), + handleReorderSubtasks: vi.fn(), +}; + +vi.mock("react-i18next", () => ({ + useTranslation: () => ({ i18n: { language: "en" } }), +})); + +vi.mock("@/hooks/domains/sidebar/use-effective-sidebar-view", () => ({ + useEffectiveSidebarView: () => view, +})); + +vi.mock("@/hooks/domains/sidebar/use-sidebar-task-prefs", () => ({ + useSidebarTaskPrefs: () => prefs, +})); + +import { useGroupedSidebarView } from "./task-session-sidebar-grouped-view"; + +function task(id: string, workflowId: string): TaskSwitcherItem { + return { id, title: id, state: "IN_PROGRESS", workflowId, workflowName: workflowId }; +} + +describe("useGroupedSidebarView", () => { + it("preserves an unaffected group reference when a task in another group updates", () => { + const taskA = task("Task A", "Workflow A"); + const taskB = task("Task B", "Workflow B"); + const hook = renderHook(({ tasks }) => useGroupedSidebarView(tasks), { + initialProps: { tasks: [taskA, taskB] }, + }); + const unaffectedGroup = hook.result.current.grouped.groups.find( + (group) => group.key === "Workflow B", + ); + + hook.rerender({ tasks: [{ ...taskA, title: "Task A updated" }, { ...taskB }] }); + + expect(hook.result.current.grouped.groups[1]).toBe(unaffectedGroup); + }); +}); diff --git a/apps/web/components/task/task-session-sidebar-grouped-view.ts b/apps/web/components/task/task-session-sidebar-grouped-view.ts index b991f3af28..2091bfe2c0 100644 --- a/apps/web/components/task/task-session-sidebar-grouped-view.ts +++ b/apps/web/components/task/task-session-sidebar-grouped-view.ts @@ -1,11 +1,148 @@ "use client"; -import { useMemo } from "react"; +import { useMemo, useRef } from "react"; import { useTranslation } from "react-i18next"; -import { applyView } from "@/lib/sidebar/apply-view"; +import { applyView, type GroupedSidebarList, type SidebarGroup } from "@/lib/sidebar/apply-view"; import { useEffectiveSidebarView } from "@/hooks/domains/sidebar/use-effective-sidebar-view"; import { useSidebarTaskPrefs } from "@/hooks/domains/sidebar/use-sidebar-task-prefs"; -import type { TaskSwitcherItem } from "./task-switcher"; +import type { TaskSwitcherItem } from "./task-switcher-types"; + +function shallowObjectEqual(previous: object, next: object): boolean { + const previousRecord = previous as Record; + const nextRecord = next as Record; + const keys = Object.keys(previousRecord); + return ( + keys.length === Object.keys(nextRecord).length && + keys.every((key) => Object.is(previousRecord[key], nextRecord[key])) + ); +} + +function taskFieldEqual(previous: unknown, next: unknown): boolean { + if (Object.is(previous, next)) return true; + if (previous === null || next === null) return false; + if (Array.isArray(previous) && Array.isArray(next)) { + return ( + previous.length === next.length && + previous.every((value, index) => { + const nextValue = next[index]; + if (Object.is(value, nextValue)) return true; + return ( + typeof value === "object" && + value !== null && + typeof nextValue === "object" && + nextValue !== null && + shallowObjectEqual(value, nextValue) + ); + }) + ); + } + return ( + typeof previous === "object" && typeof next === "object" && shallowObjectEqual(previous, next) + ); +} + +function taskItemEqual(previous: TaskSwitcherItem, next: TaskSwitcherItem): boolean { + const previousRecord = previous as Record; + const nextRecord = next as Record; + const keys = Object.keys(previousRecord); + return ( + keys.length === Object.keys(nextRecord).length && + keys.every((key) => taskFieldEqual(previousRecord[key], nextRecord[key])) + ); +} + +function previousTasksById(grouped: GroupedSidebarList): Map { + const result = new Map(); + for (const group of grouped.groups) { + for (const task of group.tasks) result.set(task.id, task); + } + for (const tasks of grouped.subTasksByParentId.values()) { + for (const task of tasks) result.set(task.id, task); + } + return result; +} + +function shareTaskArray( + previous: TaskSwitcherItem[] | undefined, + next: TaskSwitcherItem[], + priorById: Map, +): TaskSwitcherItem[] { + const shared = next.map((task) => { + const prior = priorById.get(task.id); + if (!prior || prior === task) return task; + return taskItemEqual(prior, task) ? prior : task; + }); + return previous && + previous.length === shared.length && + previous.every((task, index) => task === shared[index]) + ? previous + : shared; +} + +function shareGroup( + previous: SidebarGroup | undefined, + next: SidebarGroup, + priorById: Map, +): SidebarGroup { + if (!previous || previous.label !== next.label) return next; + const tasks = shareTaskArray(previous.tasks, next.tasks, priorById); + return tasks === previous.tasks ? previous : { ...next, tasks }; +} + +function shareGroups( + previous: SidebarGroup[], + next: SidebarGroup[], + priorById: Map, +): SidebarGroup[] { + const previousByKey = new Map(previous.map((group) => [group.key, group])); + const shared = next.map((group) => shareGroup(previousByKey.get(group.key), group, priorById)); + return shared.length === previous.length && + shared.every((group, index) => group === previous[index]) + ? previous + : shared; +} + +function shareSubtaskMap( + previous: GroupedSidebarList["subTasksByParentId"], + next: GroupedSidebarList["subTasksByParentId"], + priorById: Map, +): GroupedSidebarList["subTasksByParentId"] { + let changed = previous.size !== next.size; + const shared = new Map(); + for (const [parentId, tasks] of next) { + const previousTasks = previous.get(parentId); + const value = shareTaskArray(previousTasks, tasks, priorById); + shared.set(parentId, value); + if (value !== previousTasks) changed = true; + } + return changed ? shared : previous; +} + +export function shareGroupedSidebarList( + previous: GroupedSidebarList | null, + next: GroupedSidebarList, +): GroupedSidebarList { + // Desktop and mobile build equivalent task models independently. Reusing + // equal models here gives both surfaces the same memoization boundary. + if (!previous || previous.groupKey !== next.groupKey) return next; + const priorById = previousTasksById(previous); + const groups = shareGroups(previous.groups, next.groups, priorById); + const subTasksByParentId = shareSubtaskMap( + previous.subTasksByParentId, + next.subTasksByParentId, + priorById, + ); + return groups === previous.groups && subTasksByParentId === previous.subTasksByParentId + ? previous + : { ...next, groups, subTasksByParentId }; +} + +export function useSharedGroupedSidebarList(next: GroupedSidebarList): GroupedSidebarList { + const previousRef = useRef(null); + const shared = shareGroupedSidebarList(previousRef.current, next); + previousRef.current = shared; + return shared; +} export function useGroupedSidebarView(displayTasks: TaskSwitcherItem[]) { const prefs = useSidebarTaskPrefs(); @@ -15,7 +152,7 @@ export function useGroupedSidebarView(displayTasks: TaskSwitcherItem[]) { // the catalog. Without the language in the deps the group heading keeps the // previous locale until task data changes. const { i18n } = useTranslation(); - const grouped = useMemo( + const nextGrouped = useMemo( () => applyView(displayTasks, effectiveView, { pinnedTaskIds, @@ -31,5 +168,6 @@ export function useGroupedSidebarView(displayTasks: TaskSwitcherItem[]) { i18n.language, ], ); + const grouped = useSharedGroupedSidebarList(nextGrouped); return { grouped, effectiveView, prefs }; } diff --git a/apps/web/components/task/task-switcher-render-stability.test.tsx b/apps/web/components/task/task-switcher-render-stability.test.tsx new file mode 100644 index 0000000000..f31590f286 --- /dev/null +++ b/apps/web/components/task/task-switcher-render-stability.test.tsx @@ -0,0 +1,257 @@ +import type { ReactElement, ReactNode } from "react"; +import { cleanup, fireEvent, render, screen } from "@testing-library/react"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import type { GroupedSidebarList } from "@/lib/sidebar/apply-view"; +import type { TaskSwitcherItem } from "./task-switcher-types"; + +const renderCounts = vi.hoisted(() => ({ + groups: new Map(), + nodes: new Map(), + rows: new Map(), +})); + +function increment(counts: Map, key: string) { + counts.set(key, (counts.get(key) ?? 0) + 1); +} + +vi.mock("./task-item", () => ({ + TaskItem: ({ title, onClick }: { title: string; onClick?: () => void }) => { + increment(renderCounts.rows, title); + return ; + }, +})); + +vi.mock("./task-switcher-context-menu", () => ({ + TaskItemWithContextMenu: ({ children }: { children: ReactElement }) => children, +})); + +vi.mock("./task-switcher-subtask-dnd", () => ({ + SortableTaskLevel: ({ + tasks, + renderNode, + }: { + tasks: TaskSwitcherItem[]; + renderNode: (task: TaskSwitcherItem, isDraggable: boolean) => ReactNode; + }) => <>{tasks.map((task) => renderNode(task, false))}, + SortableTaskNode: ({ + taskId, + handle, + nested, + }: { + taskId: string; + handle: ReactNode; + nested?: ReactNode; + }) => { + increment(renderCounts.nodes, taskId); + return ( + <> + {handle} + {nested} + + ); + }, + TaskTreeDndGroup: ({ + groupTasks, + children, + }: { + groupTasks: TaskSwitcherItem[]; + children: (nestTargetIds: Set, activeDragId: string | null) => ReactNode; + }) => { + increment(renderCounts.groups, groupTasks[0]?.workflowId ?? "empty"); + return <>{children(new Set(), null)}; + }, + NestDropZone: () => null, +})); + +import { TaskSwitcher } from "./task-switcher"; + +const onSelectTask = vi.fn(); +const onToggleSubtasks = vi.fn(); +const TASK_A = "Task A"; +const TASK_A_UPDATED = "Task A updated"; +const TASK_B = "Task B"; +const WORKFLOW_A = "Workflow A"; +const WORKFLOW_B = "Workflow B"; + +function task(id: string, workflowId: string): TaskSwitcherItem { + return { id, title: id, state: "IN_PROGRESS", workflowId, workflowName: workflowId }; +} + +function groupedByWorkflow(tasks: TaskSwitcherItem[]): GroupedSidebarList { + return { + groups: tasks.map((item) => ({ + key: item.workflowId!, + label: item.workflowName!, + tasks: [item], + })), + subTasksByParentId: new Map(), + groupKey: "workflow", + }; +} + +function groupedTogether(tasks: TaskSwitcherItem[]): GroupedSidebarList { + return { + groups: [{ key: "__all__", label: "All", tasks }], + subTasksByParentId: new Map(), + groupKey: "none", + }; +} + +function groupedTrees( + entries: Array<{ root: TaskSwitcherItem; child: TaskSwitcherItem }>, + groupKey: "workflow" | "none", +): GroupedSidebarList { + return { + groups: + groupKey === "workflow" + ? entries.map(({ root }) => ({ + key: root.workflowId!, + label: root.workflowName!, + tasks: [root], + })) + : [{ key: "__all__", label: "All", tasks: entries.map(({ root }) => root) }], + subTasksByParentId: new Map(entries.map(({ root, child }) => [root.id, [child]])), + groupKey, + }; +} + +function switcher(grouped: GroupedSidebarList, selectTask = onSelectTask) { + return ( + + ); +} + +beforeEach(() => { + renderCounts.groups.clear(); + renderCounts.nodes.clear(); + renderCounts.rows.clear(); +}); + +afterEach(() => cleanup()); + +describe("TaskSwitcher render stability", () => { + it("does not rerender an unaffected expanded group when another group task updates", () => { + const taskA = task(TASK_A, WORKFLOW_A); + const taskB = task(TASK_B, WORKFLOW_B); + const initial = groupedByWorkflow([taskA, taskB]); + const view = render(switcher(initial)); + const updated = groupedByWorkflow([{ ...taskA, title: TASK_A_UPDATED }, { ...taskB }]); + + view.rerender(switcher(updated)); + + expect(renderCounts.groups.get(WORKFLOW_B)).toBe(1); + }); + + it("does not rerender an unaffected row beside an updated task", () => { + const taskA = task(TASK_A, WORKFLOW_A); + const taskB = task(TASK_B, WORKFLOW_A); + const initial = groupedTogether([taskA, taskB]); + const view = render(switcher(initial)); + const updated = groupedTogether([{ ...taskA, title: TASK_A_UPDATED }, { ...taskB }]); + + view.rerender(switcher(updated)); + + expect({ + updated: renderCounts.rows.get(TASK_A_UPDATED), + unaffected: renderCounts.rows.get(TASK_B), + }).toEqual({ updated: 1, unaffected: 1 }); + }); + + it("does not rerender an unaffected row when task-derived handlers are recreated", () => { + const taskA = task(TASK_A, WORKFLOW_A); + const taskB = task(TASK_B, WORKFLOW_A); + const initial = groupedTogether([taskA, taskB]); + const view = render(switcher(initial, vi.fn())); + const updated = groupedTogether([{ ...taskA, title: TASK_A_UPDATED }, { ...taskB }]); + + view.rerender(switcher(updated, vi.fn())); + + expect(renderCounts.rows.get(TASK_B)).toBe(1); + }); + + it("routes a skipped row action to the latest committed handler", () => { + const taskA = task(TASK_A, WORKFLOW_A); + const taskB = task(TASK_B, WORKFLOW_A); + const initial = groupedTogether([taskA, taskB]); + const view = render(switcher(initial, vi.fn())); + const latestHandler = vi.fn(); + + view.rerender( + switcher(groupedTogether([{ ...taskA, title: TASK_A_UPDATED }, { ...taskB }]), latestHandler), + ); + fireEvent.click(screen.getByRole("button", { name: TASK_B })); + + expect(latestHandler).toHaveBeenCalledWith(TASK_B); + }); +}); + +describe("TaskSwitcher nested render stability", () => { + it("does not rerender an unaffected expanded group when another group subtask updates", () => { + const parentA = task("Parent A", WORKFLOW_A); + const childA = { ...task("Child A", WORKFLOW_A), parentTaskId: parentA.id }; + const parentB = task("Parent B", WORKFLOW_B); + const childB = { ...task("Child B", WORKFLOW_B), parentTaskId: parentB.id }; + const initial = groupedTrees( + [ + { root: parentA, child: childA }, + { root: parentB, child: childB }, + ], + "workflow", + ); + const view = render(switcher(initial)); + const updated = groupedTrees( + [ + { root: { ...parentA }, child: { ...childA, title: "Child A updated" } }, + { root: { ...parentB }, child: { ...childB } }, + ], + "workflow", + ); + + view.rerender(switcher(updated)); + + expect(renderCounts.groups.get(WORKFLOW_B)).toBe(1); + }); + + it("does not rerender an unaffected expandable row beside an updated subtask", () => { + const parentA = task("Parent A", WORKFLOW_A); + const childA = { ...task("Child A", WORKFLOW_A), parentTaskId: parentA.id }; + const parentB = task("Parent B", WORKFLOW_A); + const childB = { ...task("Child B", WORKFLOW_A), parentTaskId: parentB.id }; + const initial = groupedTrees( + [ + { root: parentA, child: childA }, + { root: parentB, child: childB }, + ], + "none", + ); + const view = render(switcher(initial)); + const updated = groupedTrees( + [ + { root: { ...parentA }, child: { ...childA, title: "Child A updated" } }, + { root: { ...parentB }, child: { ...childB } }, + ], + "none", + ); + + view.rerender(switcher(updated)); + + expect(renderCounts.rows.get("Parent B")).toBe(1); + }); + + it("does not rebuild an unaffected sibling tree node when a task updates", () => { + const taskA = task(TASK_A, WORKFLOW_A); + const taskB = task(TASK_B, WORKFLOW_A); + const initial = groupedTogether([taskA, taskB]); + const view = render(switcher(initial)); + + view.rerender(switcher(groupedTogether([{ ...taskA, title: TASK_A_UPDATED }, { ...taskB }]))); + + expect(renderCounts.nodes.get(TASK_B)).toBe(1); + }); +}); diff --git a/apps/web/components/task/task-switcher-row.tsx b/apps/web/components/task/task-switcher-row.tsx index e715819f76..4f91def51d 100644 --- a/apps/web/components/task/task-switcher-row.tsx +++ b/apps/web/components/task/task-switcher-row.tsx @@ -1,6 +1,6 @@ "use client"; -import type { ReactNode } from "react"; +import { memo, type ReactNode } from "react"; import type { StepDef, TaskLinkHandler, TaskSwitcherItem } from "./task-switcher-types"; import { TaskItem } from "./task-item"; import { TaskItemWithContextMenu } from "./task-switcher-context-menu"; @@ -198,7 +198,7 @@ function TaskRowItem({ ); } -export function TaskRow(props: TaskRowProps) { +export const TaskRow = memo(function TaskRow(props: TaskRowProps) { const { task, isSubTask, depth, subtaskToggle, workflows, stepsByWorkflowId } = props; const isArchived = task.isArchived === true; return ( @@ -218,4 +218,4 @@ export function TaskRow(props: TaskRowProps) { /> ); -} +}); diff --git a/apps/web/components/task/task-switcher-tree.tsx b/apps/web/components/task/task-switcher-tree.tsx index 478a6ed6cd..fa219115bb 100644 --- a/apps/web/components/task/task-switcher-tree.tsx +++ b/apps/web/components/task/task-switcher-tree.tsx @@ -1,6 +1,6 @@ "use client"; -import { useMemo } from "react"; +import { memo, useCallback, useMemo } from "react"; import { countGroupTasks, type SidebarGroup } from "@/lib/sidebar/apply-view"; import { SortableTaskLevel, @@ -28,34 +28,90 @@ type TaskTreeContext = { onNestTask?: (taskId: string, parentTaskId: string) => void; // Rows in nestTargetIds render a nest drop zone while a drag is active. nestTargetIds: Set; - activeDragId: string | null; // The group renders one DndContext spanning every level (TaskTreeDndGroup), // so levels must not create their own. externalDragContext: boolean; }; -function TaskTreeNode({ - task, - depth, - ctx, - isDraggable, -}: { +type TaskTreeNodeProps = { task: TaskSwitcherItem; depth: number; ctx: TaskTreeContext; isDraggable: boolean; -}) { +}; + +function nodeSubtreeEqual( + taskId: string, + previous: TaskTreeContext, + next: TaskTreeContext, + visited: Set, +): boolean { + if (visited.has(taskId)) return true; + visited.add(taskId); + if ( + previous.collapsedSubs.has(taskId) !== next.collapsedSubs.has(taskId) || + previous.nestTargetIds.has(taskId) !== next.nestTargetIds.has(taskId) + ) { + return false; + } + const previousChildren = previous.subTasksByParentId.get(taskId); + const nextChildren = next.subTasksByParentId.get(taskId); + if (!previousChildren || !nextChildren) return previousChildren === nextChildren; + if ( + previousChildren.length !== nextChildren.length || + previousChildren.some((task, index) => task !== nextChildren[index]) + ) { + return false; + } + return previousChildren.every((task) => nodeSubtreeEqual(task.id, previous, next, visited)); +} + +function taskTreeNodeEqual(previous: TaskTreeNodeProps, next: TaskTreeNodeProps): boolean { + // A node owns its rendered descendants, so its equality includes only that + // branch's collapse, nest-target, and task references from the shared maps. + if ( + previous.task !== next.task || + previous.depth !== next.depth || + previous.isDraggable !== next.isDraggable || + previous.ctx.rowProps !== next.ctx.rowProps || + previous.ctx.onToggleSubtasks !== next.ctx.onToggleSubtasks || + previous.ctx.onReorderGroup !== next.ctx.onReorderGroup || + previous.ctx.onReorderSubtasks !== next.ctx.onReorderSubtasks || + previous.ctx.onNestTask !== next.ctx.onNestTask || + previous.ctx.externalDragContext !== next.ctx.externalDragContext || + (previous.depth === 0 && + previous.ctx.pinnedSet.has(previous.task.id) !== next.ctx.pinnedSet.has(next.task.id)) + ) { + return false; + } + return nodeSubtreeEqual(previous.task.id, previous.ctx, next.ctx, new Set()); +} + +const TaskTreeNode = memo(function TaskTreeNode({ + task, + depth, + ctx, + isDraggable, +}: TaskTreeNodeProps) { const subs = ctx.subTasksByParentId.get(task.id); const hasSubs = !!subs?.length; const subsHidden = hasSubs && !!ctx.onToggleSubtasks && ctx.collapsedSubs.has(task.id); - const toggleInfo: SubtaskToggleInfo | undefined = - hasSubs && ctx.onToggleSubtasks - ? { - subtaskCount: countGroupTasks(subs, ctx.subTasksByParentId), - subtasksCollapsed: subsHidden, - onToggleSubtasks: () => ctx.onToggleSubtasks!(task.id), - } - : undefined; + const subtaskCount = hasSubs ? countGroupTasks(subs, ctx.subTasksByParentId) : 0; + const handleToggleSubtasks = useCallback( + () => ctx.onToggleSubtasks?.(task.id), + [ctx.onToggleSubtasks, task.id], + ); + const toggleInfo: SubtaskToggleInfo | undefined = useMemo( + () => + hasSubs && ctx.onToggleSubtasks + ? { + subtaskCount, + subtasksCollapsed: subsHidden, + onToggleSubtasks: handleToggleSubtasks, + } + : undefined, + [handleToggleSubtasks, hasSubs, ctx.onToggleSubtasks, subsHidden, subtaskCount], + ); const isRoot = depth === 0; const isNestTarget = ctx.nestTargetIds.has(task.id); const handle = ( @@ -87,7 +143,7 @@ function TaskTreeNode({ isDraggable={isDraggable} /> ); -} +}, taskTreeNodeEqual); function TaskTreeLevel({ parentTaskId, @@ -131,7 +187,7 @@ export type GroupSectionProps = { rowProps: TaskRowBaseProps; pinnedSet: Set; isCollapsed: boolean; - onToggleCollapsed: () => void; + onToggleGroup?: (groupKey: string) => void; collapsedSubtaskParentIds?: string[]; onToggleSubtasks?: (parentTaskId: string) => void; showHeader: boolean; @@ -140,6 +196,63 @@ export type GroupSectionProps = { onNestTask?: (taskId: string, parentTaskId: string) => void; }; +function sameTaskSubtree( + previousTasks: TaskSwitcherItem[], + nextTasks: TaskSwitcherItem[], + previousMap: Map, + nextMap: Map, + visited = new Set(), +): boolean { + if ( + previousTasks.length !== nextTasks.length || + previousTasks.some((task, index) => task !== nextTasks[index]) + ) { + return false; + } + for (const task of previousTasks) { + if (visited.has(task.id)) continue; + visited.add(task.id); + const previousChildren = previousMap.get(task.id); + const nextChildren = nextMap.get(task.id); + if (!previousChildren && !nextChildren) continue; + if ( + !previousChildren || + !nextChildren || + !sameTaskSubtree(previousChildren, nextChildren, previousMap, nextMap, visited) + ) { + return false; + } + } + return true; +} + +function groupSectionEqual(previous: GroupSectionProps, next: GroupSectionProps): boolean { + if ( + previous.group !== next.group || + previous.rowProps !== next.rowProps || + previous.pinnedSet !== next.pinnedSet || + previous.isCollapsed !== next.isCollapsed || + previous.onToggleGroup !== next.onToggleGroup || + previous.collapsedSubtaskParentIds !== next.collapsedSubtaskParentIds || + previous.onToggleSubtasks !== next.onToggleSubtasks || + previous.showHeader !== next.showHeader || + previous.onReorderGroup !== next.onReorderGroup || + previous.onReorderSubtasks !== next.onReorderSubtasks || + previous.onNestTask !== next.onNestTask + ) { + return false; + } + return ( + previous.subTasksByParentId === next.subTasksByParentId || + sameTaskSubtree( + previous.group.tasks, + next.group.tasks, + previous.subTasksByParentId, + next.subTasksByParentId, + ) + ); +} + /** * Flattens one group's subtree (roots + every descendant, in rendered order) * so the group-spanning DndContext can resolve reorder levels and nest @@ -170,13 +283,13 @@ function flattenGroupTasks( * Renders one sidebar group's task tree (header + the recursive tree wrapped * in the group-spanning DndContext that enables nest-by-drag). */ -export function GroupSection({ +export const GroupSection = memo(function GroupSection({ group, subTasksByParentId, rowProps, pinnedSet, isCollapsed, - onToggleCollapsed, + onToggleGroup, collapsedSubtaskParentIds, onToggleSubtasks, showHeader, @@ -190,7 +303,7 @@ export function GroupSection({ [group.tasks, subTasksByParentId], ); - const renderTree = (nestTargetIds: Set, activeDragId: string | null) => { + const renderTree = (nestTargetIds: Set) => { if (isCollapsed) return null; const ctx: TaskTreeContext = { subTasksByParentId, @@ -202,7 +315,6 @@ export function GroupSection({ onReorderSubtasks, onNestTask, nestTargetIds, - activeDragId, externalDragContext: true, }; return ; @@ -216,7 +328,7 @@ export function GroupSection({ groupKey={group.key} count={totalCount} isCollapsed={isCollapsed} - onToggle={onToggleCollapsed} + onToggle={() => onToggleGroup?.(group.key)} /> )} ); -} +}, groupSectionEqual); diff --git a/apps/web/components/task/task-switcher.tsx b/apps/web/components/task/task-switcher.tsx index ce6170ecfc..e7f4cb6068 100644 --- a/apps/web/components/task/task-switcher.tsx +++ b/apps/web/components/task/task-switcher.tsx @@ -1,9 +1,10 @@ "use client"; -import { memo, useMemo } from "react"; +import { memo, useLayoutEffect, useMemo, useRef } from "react"; import { useTranslation } from "react-i18next"; import { TaskSwitcherSkeleton } from "./task-switcher-group"; import { GroupSection, type GroupSectionProps, type TaskRowBaseProps } from "./task-switcher-tree"; +import { useSharedGroupedSidebarList } from "./task-session-sidebar-grouped-view"; import type { TaskSwitcherProps } from "./task-switcher-types"; export type { @@ -23,7 +24,69 @@ function shouldShowRowRepository(grouped: TaskSwitcherProps["grouped"]): boolean return grouped.groupKey !== "repository"; } -function buildTaskRowProps(props: TaskSwitcherProps): TaskRowBaseProps { +const TASK_ROW_HANDLER_KEYS = [ + "onSelectTask", + "onEditTask", + "onRenameTask", + "onArchiveTask", + "onCreateSubtask", + "onDeleteTask", + "onDetachTask", + "onLinkPullRequest", + "onLinkIssue", + "onLinkMergeRequest", + "onLinkJiraTicket", + "onLinkLinearIssue", + "onLinkSentryIssue", + "onMoveToStep", + "onTogglePin", + "onToggleSelectTask", + "onSelectTaskRange", + "onBulkArchive", + "onBulkDelete", + "onBulkPin", + "onBulkMove", + "onClearSelection", +] as const; + +type TaskRowHandlerKey = (typeof TASK_ROW_HANDLER_KEYS)[number]; +type StableTaskRowHandlers = { + [Key in TaskRowHandlerKey]-?: NonNullable; +}; + +function useStableTaskRowHandlers(props: TaskSwitcherProps): StableTaskRowHandlers { + // A memoized row must keep its function identities without retaining a + // range-selection or dialog handler from an older committed task tree. + const committedPropsRef = useRef(props); + useLayoutEffect(() => { + committedPropsRef.current = props; + }, [props]); + return useMemo(() => { + const handlers: Partial unknown>> = {}; + for (const key of TASK_ROW_HANDLER_KEYS) { + handlers[key] = (...args: unknown[]) => { + const handler = committedPropsRef.current[key]; + if (typeof handler === "function") { + return (handler as (...handlerArgs: unknown[]) => unknown)(...args); + } + }; + } + return handlers as StableTaskRowHandlers; + }, []); +} + +function optionalHandler( + props: TaskSwitcherProps, + handlers: StableTaskRowHandlers, + key: Key, +): StableTaskRowHandlers[Key] | undefined { + return props[key] ? handlers[key] : undefined; +} + +function buildTaskRowProps( + props: TaskSwitcherProps, + handlers: StableTaskRowHandlers, +): TaskRowBaseProps { return { workflows: props.workflows, stepsByWorkflowId: props.stepsByWorkflowId, @@ -32,39 +95,59 @@ function buildTaskRowProps(props: TaskSwitcherProps): TaskRowBaseProps { showActivityTime: props.showActivityTime, taskRowPresentation: props.taskRowPresentation, showRepository: shouldShowRowRepository(props.grouped), - onSelectTask: props.onSelectTask, - onEditTask: props.onEditTask, - onRenameTask: props.onRenameTask, - onArchiveTask: props.onArchiveTask, - onCreateSubtask: props.onCreateSubtask, - onDeleteTask: props.onDeleteTask, - onDetachTask: props.onDetachTask, - onLinkPullRequest: props.onLinkPullRequest, - onLinkIssue: props.onLinkIssue, - onLinkMergeRequest: props.onLinkMergeRequest, - onLinkJiraTicket: props.onLinkJiraTicket, - onLinkLinearIssue: props.onLinkLinearIssue, - onLinkSentryIssue: props.onLinkSentryIssue, - onMoveToStep: props.onMoveToStep, - onTogglePin: props.onTogglePin, + onSelectTask: handlers.onSelectTask, + onEditTask: optionalHandler(props, handlers, "onEditTask"), + onRenameTask: optionalHandler(props, handlers, "onRenameTask"), + onArchiveTask: optionalHandler(props, handlers, "onArchiveTask"), + onCreateSubtask: optionalHandler(props, handlers, "onCreateSubtask"), + onDeleteTask: optionalHandler(props, handlers, "onDeleteTask"), + onDetachTask: optionalHandler(props, handlers, "onDetachTask"), + onLinkPullRequest: optionalHandler(props, handlers, "onLinkPullRequest"), + onLinkIssue: optionalHandler(props, handlers, "onLinkIssue"), + onLinkMergeRequest: optionalHandler(props, handlers, "onLinkMergeRequest"), + onLinkJiraTicket: optionalHandler(props, handlers, "onLinkJiraTicket"), + onLinkLinearIssue: optionalHandler(props, handlers, "onLinkLinearIssue"), + onLinkSentryIssue: optionalHandler(props, handlers, "onLinkSentryIssue"), + onMoveToStep: optionalHandler(props, handlers, "onMoveToStep"), + onTogglePin: optionalHandler(props, handlers, "onTogglePin"), pinnedTaskIds: props.pinnedTaskIds, deletingTaskId: props.deletingTaskId, archivingTaskId: props.archivingTaskId, isArchiving: props.isArchiving, selectedTaskIds: props.selectedTaskIds, - onToggleSelectTask: props.onToggleSelectTask, - onSelectTaskRange: props.onSelectTaskRange, - onBulkArchive: props.onBulkArchive, - onBulkDelete: props.onBulkDelete, - onBulkPin: props.onBulkPin, - onBulkMove: props.onBulkMove, - onClearSelection: props.onClearSelection, + onToggleSelectTask: optionalHandler(props, handlers, "onToggleSelectTask"), + onSelectTaskRange: optionalHandler(props, handlers, "onSelectTaskRange"), + onBulkArchive: optionalHandler(props, handlers, "onBulkArchive"), + onBulkDelete: optionalHandler(props, handlers, "onBulkDelete"), + onBulkPin: optionalHandler(props, handlers, "onBulkPin"), + onBulkMove: optionalHandler(props, handlers, "onBulkMove"), + onClearSelection: optionalHandler(props, handlers, "onClearSelection"), isMixedWorkflowSelection: props.isMixedWorkflowSelection, }; } +function shallowRowPropsEqual(previous: TaskRowBaseProps, next: TaskRowBaseProps): boolean { + const keys = Object.keys(previous) as Array; + return ( + keys.length === Object.keys(next).length && + keys.every((key) => Object.is(previous[key], next[key])) + ); +} + +function useTaskRowProps(props: TaskSwitcherProps): TaskRowBaseProps { + const handlers = useStableTaskRowHandlers(props); + const previousRef = useRef(null); + const next = buildTaskRowProps(props, handlers); + if (previousRef.current && shallowRowPropsEqual(previousRef.current, next)) { + return previousRef.current; + } + previousRef.current = next; + return next; +} + function buildGroupSectionProps( props: TaskSwitcherProps, + grouped: TaskSwitcherProps["grouped"], options: { group: GroupSectionProps["group"]; rowProps: TaskRowBaseProps; @@ -76,11 +159,11 @@ function buildGroupSectionProps( const { group, rowProps, pinnedSet, collapsedSet, showHeader } = options; return { group, - subTasksByParentId: props.grouped.subTasksByParentId, + subTasksByParentId: grouped.subTasksByParentId, rowProps, pinnedSet, isCollapsed: collapsedSet.has(group.key), - onToggleCollapsed: () => props.onToggleGroup?.(group.key), + onToggleGroup: props.onToggleGroup, collapsedSubtaskParentIds: props.collapsedSubtaskParentIds, onToggleSubtasks: props.onToggleSubtasks, showHeader, @@ -117,8 +200,14 @@ function LoadErrorNotice({ export const TaskSwitcher = memo(function TaskSwitcher(props: TaskSwitcherProps) { const { t } = useTranslation("sidebar"); - const { grouped, isLoading = false, loadError, onRetryLoad, retryLabel, totalTaskCount } = props; + const { isLoading = false, loadError, onRetryLoad, retryLabel, totalTaskCount } = props; + const grouped = useSharedGroupedSidebarList(props.grouped); const pinnedSet = useMemo(() => new Set(props.pinnedTaskIds ?? []), [props.pinnedTaskIds]); + const rowProps = useTaskRowProps(props); + const collapsedSet = useMemo( + () => new Set(props.collapsedGroupKeys ?? []), + [props.collapsedGroupKeys], + ); if (isLoading) return ; @@ -141,11 +230,9 @@ export const TaskSwitcher = memo(function TaskSwitcher(props: TaskSwitcherProps) ); } - const collapsedSet = new Set(props.collapsedGroupKeys ?? []); const showHeaders = grouped.groups.length > 1 || (grouped.groups.length === 1 && grouped.groups[0].key !== "__all__"); - const rowProps = buildTaskRowProps(props); return (
@@ -153,7 +240,7 @@ export const TaskSwitcher = memo(function TaskSwitcher(props: TaskSwitcherProps) {grouped.groups.map((group) => ( { }); }); +describe("useWorkspaceSidebarTasks reference stability", () => { + beforeEach(() => { + vi.clearAllMocks(); + mockState = { + kanbanMulti: { snapshots: {}, isLoading: false }, + workflows: { items: [] }, + kanban: { workflowId: null, tasks: [], steps: [] }, + }; + }); + + it("preserves an unaffected task reference when a sibling task updates", () => { + const snapshot = makeSnapshot("wf-A", "Alpha", ["t-a1", "t-a2"]); + setMockState({ + workflows: { items: [{ id: "wf-A", workspaceId: "ws-1", name: "Alpha" }] }, + kanbanMulti: { snapshots: { "wf-A": snapshot }, isLoading: false }, + }); + const view = renderHook(() => useWorkspaceSidebarTasks("ws-1")); + const unaffectedTask = view.result.current.allTasks[1]; + + setMockState({ + kanbanMulti: { + snapshots: { + "wf-A": { + ...snapshot, + tasks: [{ ...snapshot.tasks[0], title: "Updated" }, snapshot.tasks[1]], + }, + }, + isLoading: false, + }, + }); + view.rerender(); + + expect(view.result.current.allTasks[1]).toBe(unaffectedTask); + }); + + it("preserves workflow step references when only a task updates", () => { + const snapshot = makeSnapshot("wf-A", "Alpha", ["t-a1", "t-a2"]); + setMockState({ + workflows: { items: [{ id: "wf-A", workspaceId: "ws-1", name: "Alpha" }] }, + kanbanMulti: { snapshots: { "wf-A": snapshot }, isLoading: false }, + }); + const view = renderHook(() => useWorkspaceSidebarTasks("ws-1")); + const previousSteps = view.result.current.stepsByWorkflowId; + + setMockState({ + kanbanMulti: { + snapshots: { + "wf-A": { + ...snapshot, + tasks: [{ ...snapshot.tasks[0], title: "Updated" }, snapshot.tasks[1]], + }, + }, + isLoading: false, + }, + }); + view.rerender(); + + expect(view.result.current.stepsByWorkflowId).toBe(previousSteps); + }); +}); + describe("useWorkspaceSidebarTasks WIP queue", () => { beforeEach(() => { mockState = { diff --git a/apps/web/hooks/domains/kanban/use-workspace-sidebar-tasks.ts b/apps/web/hooks/domains/kanban/use-workspace-sidebar-tasks.ts index ff077e28d8..d554038fce 100644 --- a/apps/web/hooks/domains/kanban/use-workspace-sidebar-tasks.ts +++ b/apps/web/hooks/domains/kanban/use-workspace-sidebar-tasks.ts @@ -1,4 +1,4 @@ -import { useMemo } from "react"; +import { useMemo, useRef } from "react"; import { useAppStore } from "@/components/state-provider"; import { useAllWorkflowSnapshots } from "@/hooks/domains/kanban/use-all-workflow-snapshots"; import { useSidebarArchivedTasks } from "@/hooks/domains/kanban/use-sidebar-archived-tasks"; @@ -19,6 +19,93 @@ export type WorkspaceSidebarTasksResult = AggregatedSidebarTasks & { retryArchivedTasks: () => void; }; +type SidebarTask = AggregatedSidebarTasks["allTasks"][number]; + +function shallowTaskEqual(previous: SidebarTask, next: SidebarTask): boolean { + const previousKeys = Object.keys(previous) as Array; + const nextKeys = Object.keys(next) as Array; + return ( + previousKeys.length === nextKeys.length && + nextKeys.every((key) => Object.is(previous[key], next[key])) + ); +} + +function reuseUnchangedTasks(previous: SidebarTask[], next: SidebarTask[]): SidebarTask[] { + // Aggregation stamps `_workflowId` onto fresh objects. Restore the store's + // per-task structural sharing so downstream view models can stay granular. + const previousById = new Map(previous.map((task) => [task.id, task])); + let changed = previous.length !== next.length; + const shared = next.map((task, index) => { + const prior = previousById.get(task.id); + const value = prior && shallowTaskEqual(prior, task) ? prior : task; + if (value !== previous[index]) changed = true; + return value; + }); + return changed ? shared : previous; +} + +function reuseReferenceArray(previous: T[], next: T[]): T[] { + return previous.length === next.length && previous.every((value, index) => value === next[index]) + ? previous + : next; +} + +function reuseStepRecord( + previous: AggregatedSidebarTasks["stepsByWorkflowId"], + next: AggregatedSidebarTasks["stepsByWorkflowId"], +): AggregatedSidebarTasks["stepsByWorkflowId"] { + const previousKeys = Object.keys(previous); + const nextKeys = Object.keys(next); + let changed = previousKeys.length !== nextKeys.length; + const shared: AggregatedSidebarTasks["stepsByWorkflowId"] = {}; + for (const workflowId of nextKeys) { + const steps = reuseReferenceArray(previous[workflowId] ?? [], next[workflowId]); + shared[workflowId] = steps; + if (steps !== previous[workflowId]) changed = true; + } + return changed ? shared : previous; +} + +function useSharedStepMetadata(aggregated: AggregatedSidebarTasks) { + const previousAllStepsRef = useRef([]); + const allSteps = reuseReferenceArray(previousAllStepsRef.current, aggregated.allSteps); + previousAllStepsRef.current = allSteps; + const previousStepsByWorkflowRef = useRef({}); + const stepsByWorkflowId = reuseStepRecord( + previousStepsByWorkflowRef.current, + aggregated.stepsByWorkflowId, + ); + previousStepsByWorkflowRef.current = stepsByWorkflowId; + return { allSteps, stepsByWorkflowId }; +} + +function buildWipQueueByTaskId( + allTasks: SidebarTask[], + allSteps: AggregatedSidebarTasks["allSteps"], + stepsByWorkflowId: AggregatedSidebarTasks["stepsByWorkflowId"], +): Map { + const result = new Map(); + const activeTasks = allTasks.filter((task) => task.isArchived !== true); + const destinationStepIds = new Set( + activeTasks.map((task) => task.queuedForStepId).filter((stepId): stepId is string => !!stepId), + ); + for (const stepId of destinationStepIds) { + for (const entry of getDestinationQueue(activeTasks, stepId)) { + const task = entry.task; + const stepTitle = + stepsByWorkflowId[task._workflowId]?.find((step) => step.id === stepId)?.title ?? + allSteps.find((step) => step.id === stepId)?.title ?? + stepId; + result.set(task.id, { + position: entry.position, + total: entry.total, + destinationTitle: stepTitle, + }); + } + } + return result; +} + export function mergeSidebarArchivedTasks( activeTasks: AggregatedSidebarTasks["allTasks"], archivedTasks: KanbanState["tasks"], @@ -112,40 +199,25 @@ export function useWorkspaceSidebarTasks(workspaceId: string | null): WorkspaceS [scopedSnapshots, fallbackWorkflowId, activeKanbanTasks, activeKanbanSteps], ); + const { allSteps, stepsByWorkflowId } = useSharedStepMetadata(aggregated); + + const previousTasksRef = useRef([]); const allTasks = useMemo(() => { - return mergeSidebarArchivedTasks( + const merged = mergeSidebarArchivedTasks( aggregated.allTasks, archived.tasks, workspaceId, needsArchivedTasks, ); + const shared = reuseUnchangedTasks(previousTasksRef.current, merged); + previousTasksRef.current = shared; + return shared; }, [aggregated.allTasks, archived.tasks, needsArchivedTasks, workspaceId]); - const wipQueueByTaskId = useMemo(() => { - const result = new Map(); - const activeTasks = allTasks.filter((task) => task.isArchived !== true); - const destinationStepIds = new Set( - activeTasks - .map((task) => task.queuedForStepId) - .filter((stepId): stepId is string => !!stepId), - ); - for (const stepId of destinationStepIds) { - for (const entry of getDestinationQueue(activeTasks, stepId)) { - const task = entry.task; - const stepTitle = - aggregated.stepsByWorkflowId[task._workflowId]?.find((step) => step.id === stepId) - ?.title ?? - aggregated.allSteps.find((step) => step.id === stepId)?.title ?? - stepId; - result.set(task.id, { - position: entry.position, - total: entry.total, - destinationTitle: stepTitle, - }); - } - } - return result; - }, [allTasks, aggregated.allSteps, aggregated.stepsByWorkflowId]); + const wipQueueByTaskId = useMemo( + () => buildWipQueueByTaskId(allTasks, allSteps, stepsByWorkflowId), + [allTasks, allSteps, stepsByWorkflowId], + ); const workspaceWorkflows = useMemo( () => filteredWorkflows.map((w) => ({ id: w.id, name: w.name, hidden: w.hidden })), @@ -160,6 +232,8 @@ export function useWorkspaceSidebarTasks(workspaceId: string | null): WorkspaceS return { ...aggregated, allTasks, + allSteps, + stepsByWorkflowId, wipQueueByTaskId, workflows: workspaceWorkflows, isLoading,