From 96efdba990f9d776fc2f267a241abc9535ab4ea8 Mon Sep 17 00:00:00 2001 From: flamboh Date: Tue, 20 Oct 2026 05:38:09 +0000 Subject: [PATCH 1/2] fix(mobile): sidebar no longer re-registers shortcuts every render --- .../keyboard/hardwareKeyboardCommands.ts | 39 +-- .../keyboard/threadKeyboardShortcuts.test.tsx | 62 ++++ .../keyboard/threadKeyboardShortcuts.ts | 23 +- .../threads/ThreadNavigationSidebar.tsx | 274 +++++++++--------- apps/mobile/vite.config.ts | 1 + 5 files changed, 235 insertions(+), 164 deletions(-) create mode 100644 apps/mobile/src/features/keyboard/threadKeyboardShortcuts.test.tsx diff --git a/apps/mobile/src/features/keyboard/hardwareKeyboardCommands.ts b/apps/mobile/src/features/keyboard/hardwareKeyboardCommands.ts index cabe2949788ed..d1cb4dccd8cdd 100644 --- a/apps/mobile/src/features/keyboard/hardwareKeyboardCommands.ts +++ b/apps/mobile/src/features/keyboard/hardwareKeyboardCommands.ts @@ -23,29 +23,34 @@ const handlers = new Map>(); const registrationListeners = new Set<() => void>(); let registrationVersion = 0; -export function useHardwareKeyboardCommand( +export function registerHardwareKeyboardCommand( command: HardwareKeyboardCommand | ReadonlyArray, handler: CommandHandler, -): void { - useEffect(() => { - const commands = typeof command === "string" ? [command] : command; +): () => void { + const commands = typeof command === "string" ? [command] : command; + for (const command of commands) { + const commandHandlers = handlers.get(command) ?? new Set(); + commandHandlers.add(handler); + handlers.set(command, commandHandlers); + } + registrationVersion += 1; + registrationListeners.forEach((listener) => listener()); + return () => { for (const command of commands) { - const commandHandlers = handlers.get(command) ?? new Set(); - commandHandlers.add(handler); - handlers.set(command, commandHandlers); + const commandHandlers = handlers.get(command); + commandHandlers?.delete(handler); + if (commandHandlers?.size === 0) handlers.delete(command); } registrationVersion += 1; registrationListeners.forEach((listener) => listener()); - return () => { - for (const command of commands) { - const commandHandlers = handlers.get(command); - commandHandlers?.delete(handler); - if (commandHandlers?.size === 0) handlers.delete(command); - } - registrationVersion += 1; - registrationListeners.forEach((listener) => listener()); - }; - }, [command, handler]); + }; +} + +export function useHardwareKeyboardCommand( + command: HardwareKeyboardCommand | ReadonlyArray, + handler: CommandHandler, +): void { + useEffect(() => registerHardwareKeyboardCommand(command, handler), [command, handler]); } export function getRegisteredHardwareKeyboardCommands(): ReadonlySet { diff --git a/apps/mobile/src/features/keyboard/threadKeyboardShortcuts.test.tsx b/apps/mobile/src/features/keyboard/threadKeyboardShortcuts.test.tsx new file mode 100644 index 0000000000000..9fd3b9ff1fbc1 --- /dev/null +++ b/apps/mobile/src/features/keyboard/threadKeyboardShortcuts.test.tsx @@ -0,0 +1,62 @@ +import type { EnvironmentThreadShell } from "@t3tools/client-runtime/state/shell"; +import { act } from "react"; +import { createRoot, type Root } from "react-dom/client"; +import { afterEach, describe, expect, it, vi } from "vite-plus/test"; + +import type { ThreadListV2ListItem } from "../threads/threadListV2"; +import { + dispatchHardwareKeyboardCommand, + getHardwareKeyboardCommandRegistrationVersion, +} from "./hardwareKeyboardCommands"; +import { useThreadJumpShortcuts } from "./threadKeyboardShortcuts"; + +function threadItem(id: string): ThreadListV2ListItem { + const thread = { environmentId: "environment-1", id } as unknown as EnvironmentThreadShell; + return { + type: "v2-thread", + key: id, + item: { thread, variant: "card", snoozed: false, pinned: false, isLast: false, wokeAt: null }, + snoozeWakeLabelText: undefined, + timeLabel: "", + snoozePresetMinute: undefined, + showTrailingDivider: false, + hasQueuedMessages: false, + canMoveUp: false, + canMoveDown: false, + }; +} + +function Probe(props: { + readonly items: ReadonlyArray; + readonly onSelectThread: (thread: EnvironmentThreadShell) => void; +}) { + useThreadJumpShortcuts(props.items, props.onSelectThread); + return null; +} + +let root: Root | null = null; + +afterEach(() => { + act(() => root?.unmount()); + root = null; +}); + +describe("useThreadJumpShortcuts", () => { + it("registers once and jumps within the latest list", () => { + (globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + root = createRoot(document.createElement("div")); + act(() => root!.render()); + const registrationVersion = getHardwareKeyboardCommandRegistrationVersion(); + + const onSelectThread = vi.fn(); + for (const id of ["second", "third"]) { + act(() => root!.render()); + } + + expect(getHardwareKeyboardCommandRegistrationVersion()).toBe(registrationVersion); + expect(dispatchHardwareKeyboardCommand("thread.jump.1")).toBe(true); + expect(onSelectThread).toHaveBeenCalledExactlyOnceWith( + expect.objectContaining({ id: "third" }), + ); + }); +}); diff --git a/apps/mobile/src/features/keyboard/threadKeyboardShortcuts.ts b/apps/mobile/src/features/keyboard/threadKeyboardShortcuts.ts index 3143d164c1781..b014414e3dab6 100644 --- a/apps/mobile/src/features/keyboard/threadKeyboardShortcuts.ts +++ b/apps/mobile/src/features/keyboard/threadKeyboardShortcuts.ts @@ -1,10 +1,10 @@ import type { EnvironmentThreadShell } from "@t3tools/client-runtime/state/shell"; import { THREAD_JUMP_KEYBINDING_COMMANDS } from "@t3tools/contracts"; -import { useCallback } from "react"; +import { useEffect, useEffectEvent } from "react"; import type { ThreadListV2ListItem } from "../threads/threadListV2"; import { - useHardwareKeyboardCommand, + registerHardwareKeyboardCommand, type HardwareKeyboardCommand, } from "./hardwareKeyboardCommands"; @@ -31,13 +31,16 @@ export function useThreadJumpShortcuts( items: ReadonlyArray, onSelectThread: (thread: EnvironmentThreadShell) => void, ) { - const jumpToThread = useCallback( - (command: HardwareKeyboardCommand) => { - const thread = threadJumpTarget(items, command); - if (thread !== null) onSelectThread(thread); - return true; - }, - [items, onSelectThread], + const jumpToThread = useEffectEvent((command: HardwareKeyboardCommand) => { + const thread = threadJumpTarget(items, command); + if (thread !== null) onSelectThread(thread); + }); + useEffect( + () => + registerHardwareKeyboardCommand(THREAD_JUMP_KEYBINDING_COMMANDS, (command) => { + jumpToThread(command); + return true; + }), + [], ); - useHardwareKeyboardCommand(THREAD_JUMP_KEYBINDING_COMMANDS, jumpToThread); } diff --git a/apps/mobile/src/features/threads/ThreadNavigationSidebar.tsx b/apps/mobile/src/features/threads/ThreadNavigationSidebar.tsx index 9813f0d463338..5a2545baa66aa 100644 --- a/apps/mobile/src/features/threads/ThreadNavigationSidebar.tsx +++ b/apps/mobile/src/features/threads/ThreadNavigationSidebar.tsx @@ -14,7 +14,7 @@ import { import { LegendList } from "@legendapp/list/react-native"; import type { MenuAction } from "@react-native-menu/menu"; import { useAtomValue } from "@effect/atom-react"; -import { useEffect, useMemo, useRef, useState } from "react"; +import { useEffect, useRef, useState } from "react"; import type { LayoutChangeEvent, TextInputInstance } from "react-native"; import { Platform, StyleSheet, TextInput, View } from "react-native"; import { GestureDetector, useNativeGesture } from "react-native-gesture-handler"; @@ -125,6 +125,43 @@ function NativeSidebarContainer(props: ThreadNavigationSidebarProps) { ); } +function useSidebarListFilters(projects: ReadonlyArray) { + const { savedConnectionsById } = useSavedRemoteConnections(); + const environments = Object.values(savedConnectionsById) + .map((connection) => ({ + environmentId: connection.environmentId, + label: connection.environmentLabel, + })) + .sort((left, right) => left.label.localeCompare(right.label)); + const { options, setSelectedEnvironmentId, setSelectedProjectKey } = useHomeListOptions( + new Set(environments.map((environment) => environment.environmentId)), + ); + const projectScopeKey = options.selectedProjectKey; + const resolvedProjectKey = resolveScratchProjectScopeKey(projectScopeKey, projects, { + sidebarProjectGroupingMode: options.projectGroupingMode, + sidebarProjectGroupingOverrides: {}, + }); + useEffect(() => { + if (resolvedProjectKey !== projectScopeKey) { + setSelectedProjectKey(resolvedProjectKey); + } + }, [projectScopeKey, resolvedProjectKey, setSelectedProjectKey]); + const projectScopes = buildHomeProjectScopes({ + projects, + environmentId: options.selectedEnvironmentId, + projectGroupingMode: options.projectGroupingMode, + }); + return { + savedConnectionsById, + environments, + options, + setSelectedEnvironmentId, + setSelectedProjectKey, + projectScopes, + selectedProjectScope: projectScopes.find((scope) => scope.key === resolvedProjectKey) ?? null, + }; +} + function ThreadNavigationSidebarPane( props: ThreadNavigationSidebarProps & { readonly nativeChrome: boolean }, ) { @@ -136,7 +173,6 @@ function ThreadNavigationSidebarPane( const projects = useProjects(); const threads = useNavigationThreadShells(); const { environments: workspaceEnvironments, state: catalogState } = useWorkspaceState(); - const { savedConnectionsById } = useSavedRemoteConnections(); const searchInputRef = useRef(null); const searchBarRef = useRef(null); const openSwipeableRef = useRef(null); @@ -158,17 +194,15 @@ function ThreadNavigationSidebarPane( const pendingTasks = usePendingNewTasks(); const queuedThreadKeys = useQueuedThreadKeys(); const { confirmDeletePendingTask } = usePendingTaskListActions(); - const environments = Object.values(savedConnectionsById) - .map((connection) => ({ - environmentId: connection.environmentId, - label: connection.environmentLabel, - })) - .sort((left, right) => left.label.localeCompare(right.label)); - const availableEnvironmentIds = new Set( - environments.map((environment) => environment.environmentId), - ); - const { options, setSelectedEnvironmentId, setSelectedProjectKey } = - useHomeListOptions(availableEnvironmentIds); + const { + savedConnectionsById, + environments, + options, + setSelectedEnvironmentId, + setSelectedProjectKey, + projectScopes, + selectedProjectScope, + } = useSidebarListFilters(projects); const searchEnvironmentIds = options.selectedEnvironmentId === null ? workspaceEnvironments @@ -182,74 +216,7 @@ function ThreadNavigationSidebarPane( ? [options.selectedEnvironmentId] : []; const threadSearch = useThreadSearch(searchEnvironmentIds, props.searchQuery); - // These maps reach LegendList's extraData, which re-renders every row whenever it changes - // identity. The compiler leaves them unmemoized because they stay live across later hook calls. - const threadSearchMatchByKey = useMemo(() => { - const matches = new Map(); - for (const match of threadSearch.matches) { - if (match.source === "user" || match.source === "assistant") { - matches.set(threadSearchMatchKey(match), match); - } - } - return matches; - }, [threadSearch.matches]); - const matchedThreadKeys = new Set(threadSearch.matches.map(threadSearchMatchKey)); - const projectScopeKey = options.selectedProjectKey; - const resolvedProjectKey = resolveScratchProjectScopeKey(projectScopeKey, projects, { - sidebarProjectGroupingMode: options.projectGroupingMode, - sidebarProjectGroupingOverrides: {}, - }); - useEffect(() => { - if (resolvedProjectKey !== projectScopeKey) { - setSelectedProjectKey(resolvedProjectKey); - } - }, [projectScopeKey, resolvedProjectKey, setSelectedProjectKey]); - const projectScopes = useMemo( - () => - buildHomeProjectScopes({ - projects, - environmentId: options.selectedEnvironmentId, - projectGroupingMode: options.projectGroupingMode, - }), - [projects, options.selectedEnvironmentId, options.projectGroupingMode], - ); - const projectFilterOptions = projectScopes.map((scope) => ({ - key: scope.key, - label: scope.title, - })); - const projectTitleByProjectKey = useMemo( - () => - new Map( - projectScopes.flatMap((scope) => - scope.projectRefs.map( - (projectRef) => - [ - scopedProjectKey(projectRef.environmentId, projectRef.projectId), - scope.title, - ] as const, - ), - ), - ), - [projectScopes], - ); - const selectedProjectScope = - projectScopes.find((scope) => scope.key === resolvedProjectKey) ?? null; const selectedProjectKey = selectedProjectScope?.key ?? null; - const selectedProjectRefs = - selectedProjectScope === null - ? null - : new Set( - selectedProjectScope.projectRefs.map((projectRef) => - scopedProjectKey(projectRef.environmentId, projectRef.projectId), - ), - ); - const projectByKey = useMemo(() => { - const map = new Map(); - for (const project of projects) { - map.set(scopedProjectKey(project.environmentId, project.id), project); - } - return map; - }, [projects]); const [settledVisibleCount, setSettledVisibleCount] = useState( THREAD_LIST_V2_SETTLED_INITIAL_COUNT, @@ -297,6 +264,63 @@ function ThreadNavigationSidebarPane( } = listEnvironments; const resolveProviderInstance = useThreadRowProviderInstanceResolver(providersByEnvironmentId); const pendingOrder = usePendingThreadOrder(nowMinute, snoozeWakeTick); + + const [measuredHeaderHeight, setMeasuredHeaderHeight] = useState(null); + const { height, paddingTop, paddingBottom } = useToolbarLayout(); + const handleScrollBeginDrag = () => { + openSwipeableRef.current?.close(); + }; + const onFabScroll = useFabScroll(); + const { swipeEnabled, scrollGateHandlers } = useSwipeableScrollGate({ + onScroll: onFabScroll, + onScrollBeginDrag: handleScrollBeginDrag, + }); + const focusSearch = () => { + if (Platform.OS === "android") return false; + const focus = () => { + if (props.nativeChrome) { + searchBarRef.current?.focus(); + return; + } + searchInputRef.current?.focus(); + }; + if (!props.visible) { + props.onRequestVisibility(); + setTimeout(focus, 240); + } else { + focus(); + } + return true; + }; + useHardwareKeyboardCommand("focusSearch", focusSearch); + + const threadSearchMatchByKey = new Map(); + for (const match of threadSearch.matches) { + if (match.source === "user" || match.source === "assistant") { + threadSearchMatchByKey.set(threadSearchMatchKey(match), match); + } + } + const matchedThreadKeys = new Set(threadSearch.matches.map(threadSearchMatchKey)); + const projectTitleByProjectKey = new Map( + projectScopes.flatMap((scope) => + scope.projectRefs.map( + (projectRef) => + [scopedProjectKey(projectRef.environmentId, projectRef.projectId), scope.title] as const, + ), + ), + ); + const selectedProjectRefs = + selectedProjectScope === null + ? null + : new Set( + selectedProjectScope.projectRefs.map((projectRef) => + scopedProjectKey(projectRef.environmentId, projectRef.projectId), + ), + ); + const projectByKey = new Map(); + for (const project of projects) { + projectByKey.set(scopedProjectKey(project.environmentId, project.id), project); + } // Up/down menu availability for every card, computed once per section per // rebuild (see computeThreadMoveAvailability): per-thread planner calls made // list construction quadratic, and this list rebuilds on every minute tick. @@ -346,20 +370,6 @@ function ThreadNavigationSidebarPane( }); })(); const nextSnoozeWakeAt = threadListV2Layout.nextSnoozeWakeAt; - useEffect(() => { - if (nextSnoozeWakeAt === null) return; - const wakeAtMs = Date.parse(nextSnoozeWakeAt); - if (Number.isNaN(wakeAtMs)) return; - const delayMs = Math.min(Math.max(0, wakeAtMs - Date.now()) + 50, 2_147_483_647); - const id = setTimeout(() => { - setNow(new Date().toISOString()); - bumpSnoozeWakeTick((tick) => tick + 1); - }, delayMs); - return () => clearTimeout(id); - // snoozeWakeTick must re-arm the timer even when nextSnoozeWakeAt is - // unchanged: after a clamped fire (wake beyond the 32-bit setTimeout - // range) the boundary string is identical and the chain would die. - }, [nextSnoozeWakeAt, snoozeWakeTick]); const listItems = ((): readonly SidebarListItem[] => { const v2SearchQuery = props.searchQuery.trim().toLocaleLowerCase(); const v2PendingTasks = pendingTasks.filter( @@ -400,6 +410,39 @@ function ThreadNavigationSidebarPane( } return items; })(); + // These close the open row too, so they cannot be pass-throughs. The compiler would + // recache a closure over `props` whenever any prop changed. + const { onSelectThread, onOpenThreadPullRequests } = props; + const handleSelectThread = (thread: EnvironmentThreadShell) => { + onSelectThread(thread); + openSwipeableRef.current?.close(); + }; + const handleOpenThreadPullRequests = ( + thread: EnvironmentThreadShell, + pr: ThreadPrPresentation, + ) => { + onOpenThreadPullRequests(thread, pr); + openSwipeableRef.current?.close(); + }; + useThreadJumpShortcuts(listItems, handleSelectThread); + useEffect(() => { + if (nextSnoozeWakeAt === null) return; + const wakeAtMs = Date.parse(nextSnoozeWakeAt); + if (Number.isNaN(wakeAtMs)) return; + const delayMs = Math.min(Math.max(0, wakeAtMs - Date.now()) + 50, 2_147_483_647); + const id = setTimeout(() => { + setNow(new Date().toISOString()); + bumpSnoozeWakeTick((tick) => tick + 1); + }, delayMs); + return () => clearTimeout(id); + // snoozeWakeTick must re-arm the timer even when nextSnoozeWakeAt is + // unchanged: after a clamped fire (wake beyond the 32-bit setTimeout + // range) the boundary string is identical and the chain would die. + }, [nextSnoozeWakeAt, snoozeWakeTick]); + const projectFilterOptions = projectScopes.map((scope) => ({ + key: scope.key, + label: scope.title, + })); const listMenuActions = [ { id: "environment", @@ -473,8 +516,6 @@ function ThreadNavigationSidebarPane( } }; - const [measuredHeaderHeight, setMeasuredHeaderHeight] = useState(null); - const { height, paddingTop, paddingBottom } = useToolbarLayout(); // The sticky header (title row, search field, optional connection status) // is measured so the list inset always matches its real height — no // hardcoded per-variant constants. @@ -499,28 +540,6 @@ function ThreadNavigationSidebarPane( openSwipeableRef.current = null; } }; - // These close the open row too, so they cannot be pass-throughs. The compiler would - // recache a closure over `props` whenever any prop changed. - const { onSelectThread, onOpenThreadPullRequests } = props; - const handleSelectThread = (thread: EnvironmentThreadShell) => { - onSelectThread(thread); - openSwipeableRef.current?.close(); - }; - const handleOpenThreadPullRequests = ( - thread: EnvironmentThreadShell, - pr: ThreadPrPresentation, - ) => { - onOpenThreadPullRequests(thread, pr); - openSwipeableRef.current?.close(); - }; - const handleScrollBeginDrag = () => { - openSwipeableRef.current?.close(); - }; - const onFabScroll = useFabScroll(); - const { swipeEnabled, scrollGateHandlers } = useSwipeableScrollGate({ - onScroll: onFabScroll, - onScrollBeginDrag: handleScrollBeginDrag, - }); // The sticky header's project shells and search maps feed row props, so // they have to bust the recycler's memoization — otherwise a row keeps the // blank favicon and fallback title it was first rendered with. The minute @@ -535,7 +554,6 @@ function ThreadNavigationSidebarPane( threadSearchMatchByKey, workingShelfEnabled, }; - useThreadJumpShortcuts(listItems, handleSelectThread); const sidebarItemsAreEqual = (previous: SidebarListItem, item: SidebarListItem): boolean => { if (isThreadListV2ListItem(previous) && isThreadListV2ListItem(item)) { return threadListV2ListItemsAreEqual(previous, item); @@ -545,24 +563,6 @@ function ThreadNavigationSidebarPane( } return false; }; - const focusSearch = () => { - if (Platform.OS === "android") return false; - const focus = () => { - if (props.nativeChrome) { - searchBarRef.current?.focus(); - return; - } - searchInputRef.current?.focus(); - }; - if (!props.visible) { - props.onRequestVisibility(); - setTimeout(focus, 240); - } else { - focus(); - } - return true; - }; - useHardwareKeyboardCommand("focusSearch", focusSearch); const renderListItem = ({ item }: { readonly item: SidebarListItem }) => { switch (item.type) { case "v2-pending": { diff --git a/apps/mobile/vite.config.ts b/apps/mobile/vite.config.ts index 3f66714c68815..56aa4b9b3c606 100644 --- a/apps/mobile/vite.config.ts +++ b/apps/mobile/vite.config.ts @@ -3,6 +3,7 @@ import "vite-plus/test/config"; const reactTests = [ "src/components/ui/Dialog.android.test.tsx", + "src/features/keyboard/threadKeyboardShortcuts.test.tsx", "src/features/settings/SettingsServerControlsRouteScreen.test.tsx", "src/features/settings/components/BranchNamingSettings.test.tsx", "src/native/NativeHeaderToolbar.test.tsx", From d9866ac381357d267674c3297e702dfb1dfee296 Mon Sep 17 00:00:00 2001 From: flamboh Date: Wed, 21 Oct 2026 19:14:43 +0000 Subject: [PATCH 2/2] test(mobile): pin the compiler cache for the sidebar thread list --- apps/mobile/src/reactCompilerCoverage.test.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/apps/mobile/src/reactCompilerCoverage.test.ts b/apps/mobile/src/reactCompilerCoverage.test.ts index f51c729f319e4..0e0d2d80e80ad 100644 --- a/apps/mobile/src/reactCompilerCoverage.test.ts +++ b/apps/mobile/src/reactCompilerCoverage.test.ts @@ -191,6 +191,17 @@ describe("React Compiler coverage", () => { ).toEqual([true]); }); + // The compiler once silently dropped this cache, so every sidebar render + // rebuilt the list and re-registered the thread jump shortcuts. + it("caches the sidebar thread list", () => { + expect( + compilerCachedCalls( + "src/features/threads/ThreadNavigationSidebar.tsx", + "buildThreadListV2ListItems", + ), + ).toEqual([true]); + }); + it("lists only files that exist", () => { expect(Object.keys(KNOWN_BAILOUTS).filter((file) => !sourceFiles.includes(file))).toEqual([]); });