diff --git a/apps/desktop/src/renderer/src/components/tab-content.tsx b/apps/desktop/src/renderer/src/components/tab-content.tsx index fd5ede6f5..90e37b9e7 100644 --- a/apps/desktop/src/renderer/src/components/tab-content.tsx +++ b/apps/desktop/src/renderer/src/components/tab-content.tsx @@ -1,17 +1,17 @@ import { Activity, useEffect } from "react"; import { RouterProvider } from "react-router-dom"; -import { useTabStore, useActiveGroup } from "@/stores/tab-store"; +import { useActiveGroup } from "@/stores/tab-store"; import { TabNavigationProvider } from "@/platform/navigation"; import { useTabRouterSync } from "@/hooks/use-tab-router-sync"; +import type { Tab } from "@/stores/tab-store"; -/** Inner wrapper rendered inside each tab's RouterProvider. */ -function TabRouterInner({ tabId }: { tabId: string }) { - const tab = useTabStore((s) => { - if (!s.activeWorkspaceSlug) return null; - const group = s.byWorkspace[s.activeWorkspaceSlug]; - return group?.tabs.find((t) => t.id === tabId) ?? null; - }); - useTabRouterSync(tabId, tab!.router); +/** + * Inner wrapper rendered inside each tab's RouterProvider. The router + * reference is stable for a tab's lifetime, so passing it in directly + * (instead of re-deriving from the store) avoids needless re-renders. + */ +function TabRouterInner({ tab }: { tab: Tab }) { + useTabRouterSync(tab.id, tab.router); return null; } @@ -46,7 +46,7 @@ export function TabContent() { > - + ))} diff --git a/apps/desktop/src/renderer/src/hooks/use-tab-history.ts b/apps/desktop/src/renderer/src/hooks/use-tab-history.ts index f8f21d1c7..1c47fbd36 100644 --- a/apps/desktop/src/renderer/src/hooks/use-tab-history.ts +++ b/apps/desktop/src/renderer/src/hooks/use-tab-history.ts @@ -1,6 +1,6 @@ import { useCallback } from "react"; import type { DataRouter } from "react-router-dom"; -import { useActiveTab } from "@/stores/tab-store"; +import { useActiveTabRouter, useActiveTabHistory } from "@/stores/tab-store"; /** * Shared hint map so useTabRouterSync can distinguish back vs forward POP. @@ -10,28 +10,31 @@ export const popDirectionHints = new Map(); /** * Per-tab back/forward navigation derived from the active workspace's - * active tab. When the active workspace has no tab (pre-workspace state), - * both buttons disable gracefully. + * active tab. + * + * Subscribed via primitive selectors so this hook only re-renders when + * the numeric history state actually changes — path ticks on the active + * tab (which don't shift historyIndex) don't churn the back/forward + * buttons. */ export function useTabHistory() { - const activeTab = useActiveTab(); + const router = useActiveTabRouter(); + const { historyIndex, historyLength } = useActiveTabHistory(); - const canGoBack = (activeTab?.historyIndex ?? 0) > 0; - const canGoForward = - (activeTab?.historyIndex ?? 0) < (activeTab?.historyLength ?? 1) - 1; + const canGoBack = historyIndex > 0; + const canGoForward = historyIndex < historyLength - 1; const goBack = useCallback(() => { - if (!activeTab || activeTab.historyIndex <= 0) return; - popDirectionHints.set(activeTab.router, "back"); - activeTab.router.navigate(-1); - }, [activeTab]); + if (!router || historyIndex <= 0) return; + popDirectionHints.set(router, "back"); + router.navigate(-1); + }, [router, historyIndex]); const goForward = useCallback(() => { - if (!activeTab || activeTab.historyIndex >= activeTab.historyLength - 1) - return; - popDirectionHints.set(activeTab.router, "forward"); - activeTab.router.navigate(1); - }, [activeTab]); + if (!router || historyIndex >= historyLength - 1) return; + popDirectionHints.set(router, "forward"); + router.navigate(1); + }, [router, historyIndex, historyLength]); return { canGoBack, canGoForward, goBack, goForward }; } diff --git a/apps/desktop/src/renderer/src/platform/navigation.tsx b/apps/desktop/src/renderer/src/platform/navigation.tsx index 04c8596dd..df9f148da 100644 --- a/apps/desktop/src/renderer/src/platform/navigation.tsx +++ b/apps/desktop/src/renderer/src/platform/navigation.tsx @@ -6,7 +6,13 @@ import { } from "@multica/views/navigation"; import { useAuthStore } from "@multica/core/auth"; import { isReservedSlug } from "@multica/core/paths"; -import { useTabStore, resolveRouteIcon } from "@/stores/tab-store"; +import { + useTabStore, + resolveRouteIcon, + useActiveTabIdentity, + useActiveTabRouter, + getActiveTab, +} from "@/stores/tab-store"; import { useWindowOverlayStore } from "@/stores/window-overlay-store"; // Public web app URL — injected at build time via .env.production. Falls @@ -95,25 +101,25 @@ export function DesktopNavigationProvider({ }: { children: React.ReactNode; }) { - const activeTab = useTabStore((s) => { - if (!s.activeWorkspaceSlug) return null; - const group = s.byWorkspace[s.activeWorkspaceSlug]; - if (!group) return null; - return group.tabs.find((t) => t.id === group.activeTabId) ?? null; - }); - const [pathname, setPathname] = useState(activeTab?.path ?? "/"); + // Primitive-only subscriptions so this component doesn't re-render on + // unrelated store updates (e.g. an inactive tab's router tick). We + // resolve the active router here only to subscribe once per tab switch. + const { tabId: activeTabId } = useActiveTabIdentity(); + const router = useActiveTabRouter(); + const [pathname, setPathname] = useState( + router?.state.location.pathname ?? "/", + ); - // Subscribe to the active tab's router for pathname updates. useEffect(() => { - if (!activeTab) { + if (!router) { setPathname("/"); return; } - setPathname(activeTab.router.state.location.pathname); - return activeTab.router.subscribe((state) => { + setPathname(router.state.location.pathname); + return router.subscribe((state) => { setPathname(state.location.pathname); }); - }, [activeTab?.id]); // eslint-disable-line react-hooks/exhaustive-deps + }, [activeTabId, router]); const adapter: NavigationAdapter = useMemo( () => ({ @@ -160,11 +166,7 @@ export function DesktopNavigationProvider({ } function currentActiveTab() { - const state = useTabStore.getState(); - if (!state.activeWorkspaceSlug) return null; - const group = state.byWorkspace[state.activeWorkspaceSlug]; - if (!group) return null; - return group.tabs.find((t) => t.id === group.activeTabId) ?? null; + return getActiveTab(useTabStore.getState()); } /** diff --git a/apps/desktop/src/renderer/src/stores/tab-store.ts b/apps/desktop/src/renderer/src/stores/tab-store.ts index 9bd851693..6ca66893d 100644 --- a/apps/desktop/src/renderer/src/stores/tab-store.ts +++ b/apps/desktop/src/renderer/src/stores/tab-store.ts @@ -230,6 +230,10 @@ export const useTabStore = create()( byWorkspace: {}, switchWorkspace(slug, openPath) { + // Defensive no-op if slug is empty/invalid — callers like the + // NavigationAdapter's path-parser should already have filtered + // these, but belt-and-braces keeps garbage out of the store. + if (!slug) return; const { byWorkspace } = get(); const existing = byWorkspace[slug]; @@ -514,7 +518,14 @@ export const useTabStore = create()( // Persisted path may have come from a stale version or a // manual edit. Drop rather than rewrite so we never silently // put users on a path that doesn't match the group's slug. - if (!clean || extractWorkspaceSlug(clean) !== slug) continue; + if (!clean || extractWorkspaceSlug(clean) !== slug) { + // eslint-disable-next-line no-console + console.warn( + `[tab-store] dropping persisted tab "${pTab.path}" from ` + + `group "${slug}" — path/slug mismatch`, + ); + continue; + } tabs.push({ id: pTab.id, path: clean, @@ -622,19 +633,73 @@ export function migrateV1ToV2(v1: Partial): V2Persisted { // Selectors (convenience hooks) // --------------------------------------------------------------------------- -/** The active workspace's tab group, or null when no workspace is active. */ +/** + * Pure non-hook helper — useful from event handlers / effects that already + * need `.getState()`. For React subscriptions prefer the stable selectors + * below. + */ +export function getActiveTab(s: TabStore): Tab | null { + if (!s.activeWorkspaceSlug) return null; + const group = s.byWorkspace[s.activeWorkspaceSlug]; + if (!group) return null; + return group.tabs.find((t) => t.id === group.activeTabId) ?? null; +} + +/** + * The active workspace's tab group, or null when no workspace is active. + * + * Zustand compares selector returns with `Object.is`. Because `updateTab` + * / `updateTabHistory` replace the group object on every router tick + * (immutable update), this selector returns a new reference on every + * router event — that's fine for TabBar which needs to observe tab-list + * changes, but don't use this selector from components that only care + * about one primitive (use `useActiveTabHistory` / `useActiveTabRouter` + * instead). + */ export function useActiveGroup(): WorkspaceTabGroup | null { return useTabStore((s) => s.activeWorkspaceSlug ? (s.byWorkspace[s.activeWorkspaceSlug] ?? null) : null, ); } -/** The currently rendered tab, or null. */ -export function useActiveTab(): Tab | null { - return useTabStore((s) => { - if (!s.activeWorkspaceSlug) return null; - const group = s.byWorkspace[s.activeWorkspaceSlug]; - if (!group) return null; - return group.tabs.find((t) => t.id === group.activeTabId) ?? null; - }); +/** + * Active tab id + active workspace slug as a compact pair. Both primitives + * are stable across unrelated store updates — e.g. an inactive tab's + * router tick doesn't churn these, so consumers don't re-render. + * + * Useful anywhere you'd previously have reached for `useActiveTab()` and + * only needed the identity (for memoization, effect deps, ipc). + */ +export function useActiveTabIdentity(): { slug: string | null; tabId: string | null } { + const slug = useTabStore((s) => s.activeWorkspaceSlug); + const tabId = useTabStore((s) => + s.activeWorkspaceSlug + ? (s.byWorkspace[s.activeWorkspaceSlug]?.activeTabId ?? null) + : null, + ); + return { slug, tabId }; +} + +/** + * Active tab's router — a stable reference across tab updates, because + * routers are created once per tab and never replaced by `updateTab`. + * Subscribers only re-render when the active tab *changes*, not on + * router events within the current tab. + */ +export function useActiveTabRouter(): DataRouter | null { + return useTabStore((s) => getActiveTab(s)?.router ?? null); +} + +/** + * History tracking for the active tab as primitives. Subscribers re-render + * only when the numeric index / length change (i.e. on actual navigations), + * not on unrelated store updates. + */ +export function useActiveTabHistory(): { + historyIndex: number; + historyLength: number; +} { + const historyIndex = useTabStore((s) => getActiveTab(s)?.historyIndex ?? 0); + const historyLength = useTabStore((s) => getActiveTab(s)?.historyLength ?? 1); + return { historyIndex, historyLength }; }