From 3ae5ab1eb672154700ed58fe63f32dd4df99fb13 Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Wed, 9 Sep 2026 13:42:16 -0400 Subject: [PATCH 1/4] refactor(router): one nav-tree model The navigation tree shape - routes[0] of the root stack is the 'loggedIn' tab navigator, routes[1..] holds modals and phone-pushed screens - was re-derived by hand in eight functions across constants/router and router-v2/linking, and had already drifted: navToThread's phone branch omitted the `index: 0` that makeChatConversationState set on the loggedIn child state. constants/nav-tree owns the shape now. Readers (visiblePath, visibleScreen, modalStack, activeStack, currentTab, tabNavigatorState, isLoggedIn) are pure functions of a NavState; builders (tabState, modalState, pushedAboveTabs) return a PartialNavState. The modal-name registry and tabRoots live there too, so modal-ness-by-name and tab roots are stated once. No navigationRef, no dispatch, no chat. Two literal differences were resolved in favour of one shape: - `index: 0` on the loggedIn child state is now always spelled out. It is what react-navigation computes anyway (a tab router rehydrates a missing index to 0), but a stack router rehydrates a missing index to the LAST route, so leaving it off means the same omission reads differently at different depths. - the tab-root route under a pushed screen now carries no `params`, matching what the fs deep link already built. chatRoot declares `initialParams: {}`, so StackRouter rehydrates `{...{}, ...undefined}` to the same `{}` the old `params: {}` produced. The store's `navState?: unknown` is typed by NavState, which removes the casts at its three read sites. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015rccpV5nLxxC5opF5xzrz7 --- shared/chat/inbox-and-conversation-shared.tsx | 3 +- shared/constants/init/shared.tsx | 6 +- shared/constants/nav-tree.tsx | 209 ++++++++++++++ shared/constants/router.tsx | 163 ++--------- shared/constants/tests/nav-tree.test.ts | 267 ++++++++++++++++++ shared/constants/tests/router-visible.test.ts | 105 ------- shared/fs/common/daemon.tsx | 2 +- shared/router-v2/linking-state.test.ts | 4 +- shared/router-v2/linking.tsx | 98 +------ shared/router-v2/routes.tsx | 15 +- shared/stores/router.tsx | 13 +- 11 files changed, 526 insertions(+), 359 deletions(-) create mode 100644 shared/constants/nav-tree.tsx create mode 100644 shared/constants/tests/nav-tree.test.ts delete mode 100644 shared/constants/tests/router-visible.test.ts diff --git a/shared/chat/inbox-and-conversation-shared.tsx b/shared/chat/inbox-and-conversation-shared.tsx index a97c68440f83..14cc052ca727 100644 --- a/shared/chat/inbox-and-conversation-shared.tsx +++ b/shared/chat/inbox-and-conversation-shared.tsx @@ -8,7 +8,6 @@ import Conversation from '@/chat/conversation/container' import InfoPanel, {type Panel} from '@/chat/conversation/info-panel' import type {ThreadSearchRouteProps} from '@/chat/conversation/thread-search-route' import {useInboxLayoutState} from '@/chat/inbox/layout-state' -import type {NavState} from '@/constants/router' import logger from '@/logger' export type InboxAndConversationProps = ThreadSearchRouteProps & { @@ -30,7 +29,7 @@ export function InboxAndConversationShell(props: Props) { const validConvoID = conversationIDKey && conversationIDKey !== Chat.noConversationIDKey const lastValidCIDRef = React.useRef(validConvoID ? conversationIDKey : '') const chatTabSelected = C.useRouterState(s => { - const storedTab = C.Router2.getTab(s.navState as NavState | undefined) + const storedTab = C.Router2.getTab(s.navState) return (storedTab ?? C.Router2.getTab()) === C.Tabs.chatTab }) const firstSmallTeam = useInboxLayoutState(s => { diff --git a/shared/constants/init/shared.tsx b/shared/constants/init/shared.tsx index 31ded8d2e135..74711eef175b 100644 --- a/shared/constants/init/shared.tsx +++ b/shared/constants/init/shared.tsx @@ -467,11 +467,9 @@ export const listenForPushTaps = (): (() => void) => { }; const onNavStateChanged = ( - nextNavState: RouterState["navState"], - previousNavState: RouterState["navState"], + next: RouterState["navState"], + prev: RouterState["navState"], ) => { - const next = nextNavState as Util.NavState; - const prev = previousNavState as Util.NavState; if (prev === next) return; // Clear critical update when we nav away from tab diff --git a/shared/constants/nav-tree.tsx b/shared/constants/nav-tree.tsx new file mode 100644 index 000000000000..2c26f443553b --- /dev/null +++ b/shared/constants/nav-tree.tsx @@ -0,0 +1,209 @@ +// The shape of the navigation tree, in one place. +// +// Every screen lives at one of three depths: +// root stack routes[0] is the root screen - the 'loggedIn' tab navigator when +// signed in, otherwise 'loggedOut' (or 'loading' on desktop); +// routes[1+] hold modals AND non-modal screens pushed above the +// tab bar on phones. +// tab navigator one route per tab; its index selects the visible tab. +// tab stack the screens pushed inside that tab, rooted at tabRoots[tab]. +// +// Readers take a plain NavState and are pure. Builders return a PartialNavState suitable +// for CommonActions.reset or React Navigation's linking getStateFromPath. Nothing here +// touches a navigator, dispatches, or knows anything about chat. +import * as Tabs from './tabs' +import type {Immutable} from 'immer' +import type {NavigationState} from '@react-navigation/core' +import type {RootParamList} from '@/router-v2/route-params' + +export type Route = NavigationState['routes'][0] +// still a little paranoid about some things being missing in this type +export type NavState = Partial + +export type PartialRoute = { + name: string + params?: Record + state?: PartialNavState +} + +export type PartialNavState = { + routes: Array + index?: number +} + +export type ScreenSpec = {name: string; params?: Record} + +// The root screen of each tab's stack. Kept here rather than with the route table so the +// tree shape has no dependency on the (very heavy) router config; router-v2/routes +// re-exports this for the navigator definitions. +export const tabRoots = { + [Tabs.peopleTab]: 'peopleRoot', + [Tabs.chatTab]: 'chatRoot', + [Tabs.cryptoTab]: 'cryptoRoot', + [Tabs.fsTab]: 'fsRoot', + [Tabs.teamsTab]: 'teamsRoot', + [Tabs.gitTab]: 'gitRoot', + [Tabs.devicesTab]: 'devicesRoot', + [Tabs.settingsTab]: 'settingsRoot', + + [Tabs.loginTab]: '', + [Tabs.searchTab]: '', +} as const + +// Modal route names, registered at startup from the router config (the single source +// of truth — see modalRoutes in router-v2/routes). A serialized NavigationState route +// does not carry its `presentation`, so we cannot detect modals structurally: a route +// living in the root stack (alongside the tab navigator) is a modal iff its name is in +// this set. Everything else there (e.g. chatConversation, and any other non-modal screen +// pushed above the tab bar on phones) is a genuinely-visible screen. +let modalRouteNames: ReadonlySet | undefined +export const setModalRouteNames = (names: Iterable) => { + modalRouteNames = new Set(names) +} +export const isModalRouteName = (name: string) => { + if (!modalRouteNames) { + throw new Error('modalRouteNames not registered; call setModalRouteNames at startup') + } + return modalRouteNames.has(name) +} + +// ---- Readers ---- + +export const isLoggedIn = (state?: Immutable) => state?.routes?.[0]?.name === 'loggedIn' + +export const currentTab = (state?: Immutable): Tabs.Tab | undefined => { + const loggedInRoute = state?.routes?.[0] + if (loggedInRoute?.name === 'loggedIn') { + // eslint-disable-next-line + return loggedInRoute.state?.routes?.[loggedInRoute.state.index ?? 0]?.name as Tabs.Tab + } + return undefined +} + +// The tab navigator's own state - routes[0] of the root stack. Undefined when logged out. +export const tabNavigatorState = (state?: Immutable): Immutable | undefined => + isLoggedIn(state) ? state?.routes?.[0]?.state : undefined + +// The routes in the root stack above the tab navigator that are real modals. +export const modalStack = (state?: Immutable): Immutable> => { + if (!state || !isLoggedIn(state)) { + return [] + } + return (state.routes?.slice(1) ?? []).filter(r => isModalRouteName(r.name)) as Immutable> +} + +// The innermost stack the user is looking at - the one a push/pop should target. +export const activeStack = (state?: Immutable): Immutable | undefined => { + const descend = (s: Immutable | undefined, depth: number): Immutable | undefined => { + if (!s?.routes || s.index === undefined) { + return undefined + } + if (depth === 0) { + const topModal = (s.routes.slice(1) as Array).filter(route => isModalRouteName(route.name)).at(-1) + if (topModal) { + return descend(topModal.state, depth + 1) ?? s + } + const loggedInRoute = s.routes[0] as Route | undefined + return descend(loggedInRoute?.state, depth + 1) ?? (s.type === 'stack' ? s : undefined) + } + const childRoute = s.routes[s.index] as Route | undefined + return descend(childRoute?.state, depth + 1) ?? (s.type === 'stack' ? s : undefined) + } + return descend(state, 0) +} + +// loggedIn/tab/stack items, plus whatever sits above the tab navigator. +export const visiblePath = ( + state?: Immutable, + opts?: {includeModals?: boolean} +): Immutable> => { + const includeModals = opts?.includeModals ?? true + + const findVisibleRoute = ( + arr: Immutable>, + s: Immutable, + depth: number + ): Immutable> => { + if (!s?.routes || s.index === undefined) { + return arr + } + let childRoute = s.routes[s.index] as Route | undefined + if (!childRoute) { + return arr + } + + let toAdd: Array + let toAddModals: Array = [] + // special handling of modals, we keep them to the side to add them later, then go down the visible tab + if (depth === 0) { + childRoute = s.routes[0] as Route + toAdd = [childRoute] + // routes[1+] holds both real modals and root non-modal screens (e.g. + // chatConversation on phones, stacked above the tab bar). The latter are + // genuinely visible, so always include them; only gate real modals on includeModals. + const rest = s.routes.slice(1) as Array + toAddModals = includeModals ? rest : rest.filter(r => !isModalRouteName(r.name)) + } else { + // include items in the stack + if (s.type === 'stack') { + toAdd = s.routes as Array + } else { + toAdd = [childRoute] + } + } + + const nextArr = [...arr, ...toAdd] + const children = findVisibleRoute(nextArr, childRoute.state, depth + 1) + return [...children, ...toAddModals] + } + + if (!state) return [] + return findVisibleRoute([], state, 0) +} + +export const visibleScreen = (state?: Immutable, opts?: {includeModals?: boolean}) => + visiblePath(state, opts).at(-1) + +// ---- Builders ---- + +// Tabs at the root, `tab` selected, optionally with screens pushed inside that tab's stack. +export const tabState = (tab: Tabs.Tab, screenStack?: ReadonlyArray): PartialNavState => { + const tabRoute: PartialRoute = {name: tab} + if (screenStack?.length) { + tabRoute.state = {index: screenStack.length - 1, routes: [...screenStack]} + } + return { + index: 0, + routes: [{name: 'loggedIn', state: {index: 0, routes: [tabRoute]}}], + } +} + +// A modal in the root stack. underTab selects which tab sits beneath it; without it +// loggedIn falls back to the initial (people) tab. +export const modalState = ( + modalName: string, + params?: Record, + underTab?: Tabs.AppTab +): PartialNavState => ({ + index: 1, + routes: [ + underTab ? {name: 'loggedIn', state: {index: 0, routes: [{name: underTab}]}} : {name: 'loggedIn'}, + {name: modalName, ...(params ? {params} : {})}, + ], +}) + +// Phone shape: the tab navigator sits at the root with `tab` selected on its root screen, +// and `screen` is pushed above it so it covers the tab bar. +export const pushedAboveTabs = (tab: Tabs.AppTab, screen: ScreenSpec): PartialNavState => ({ + index: 1, + routes: [ + { + name: 'loggedIn', + state: { + index: 0, + routes: [{name: tab, state: {index: 0, routes: [{name: tabRoots[tab]}]}}], + }, + }, + screen, + ], +}) diff --git a/shared/constants/router.tsx b/shared/constants/router.tsx index 84865b086506..77e9b6a2c730 100644 --- a/shared/constants/router.tsx +++ b/shared/constants/router.tsx @@ -12,10 +12,10 @@ import { type NavigationContainerRef, NavigationContext, createNavigationContainerRef, - type NavigationState, } from '@react-navigation/core' import type {StaticScreenProps} from '@react-navigation/core' import type {NavigateAppendType, RouteKeys, RootParamList as KBRootParamList} from '@/router-v2/route-params' +import * as NavTree from './nav-tree' import type {GetOptionsRet, RouteDef} from './types/router' import {isSplit, threadRouteName} from './chat/layout' import {ignorePromise, shallowEqual} from './utils' @@ -59,28 +59,14 @@ registerDebugClear(() => { navigationRef.current = null }) -export type Route = NavigationState['routes'][0] -// still a little paranoid about some things being missing in this type -export type NavState = Partial +export type {Route, NavState} from './nav-tree' +type Route = NavTree.Route +type NavState = NavTree.NavState export type Navigator = NavigationContainerRef +export {setModalRouteNames} from './nav-tree' + const DEBUG_NAV = __DEV__ && (false as boolean) -// Modal route names, registered at startup from the router config (the single source -// of truth — see modalRoutes in router-v2/routes). A serialized NavigationState route -// does not carry its `presentation`, so we cannot detect modals structurally: a route -// living in the root stack (alongside the tab navigator) is a modal iff its name is in -// this set. Everything else there (e.g. chatConversation, and any other non-modal screen -// pushed above the tab bar on phones) is a genuinely-visible screen. -let modalRouteNames: ReadonlySet | undefined -export const setModalRouteNames = (names: Iterable) => { - modalRouteNames = new Set(names) -} -const isRootModalRoute = (name: string) => { - if (!modalRouteNames) { - throw new Error('modalRouteNames not registered; call setModalRouteNames at startup') - } - return modalRouteNames.has(name) -} const uiParticipantsToParticipantInfo = ( uiParticipants: ReadonlyArray @@ -104,116 +90,26 @@ export const getRootState = (): NavState | undefined => { return navigationRef.getRootState() } -export const getTab = (navState?: T.Immutable): undefined | Tabs.Tab => { - const s = navState || getRootState() - const loggedInRoute = s?.routes?.[0] - if (loggedInRoute?.name === 'loggedIn') { - // eslint-disable-next-line - return loggedInRoute.state?.routes?.[loggedInRoute.state.index ?? 0]?.name as Tabs.Tab - } - return undefined -} - -const _isLoggedIn = (s: T.Immutable) => { - if (!s) { - return false - } - return s.routes?.[0]?.name === 'loggedIn' -} +export const getTab = (navState?: T.Immutable): undefined | Tabs.Tab => + NavTree.currentTab(navState || getRootState()) export const _getNavigator = () => { return navigationRef.isReady() ? navigationRef : undefined } -const getActiveStackState = (navState?: T.Immutable): T.Immutable | undefined => { - const rs = navState || getRootState() - const findActiveStackState = ( - state: T.Immutable | undefined, - depth: number - ): T.Immutable | undefined => { - if (!state?.routes || state.index === undefined) { - return undefined - } - if (depth === 0) { - const topModal = (state.routes.slice(1) as Array) - .filter(route => isRootModalRoute(route.name)) - .at(-1) - if (topModal) { - return findActiveStackState(topModal.state, depth + 1) ?? state - } - const loggedInRoute = state.routes[0] as Route | undefined - return findActiveStackState(loggedInRoute?.state, depth + 1) ?? (state.type === 'stack' ? state : undefined) - } - const childRoute = state.routes[state.index] as Route | undefined - return findActiveStackState(childRoute?.state, depth + 1) ?? (state.type === 'stack' ? state : undefined) - } - return findActiveStackState(rs, 0) -} +const getActiveStackState = (navState?: T.Immutable) => + NavTree.activeStack(navState || getRootState()) // Public API // gives you loggedin/tab/stackitems + modals -export const getVisiblePath = (navState?: T.Immutable, _inludeModals?: boolean) => { - const rs = navState || getRootState() - const inludeModals = _inludeModals ?? true - - const findVisibleRoute = ( - arr: T.Immutable>, - s: T.Immutable, - depth: number - ): T.Immutable> => { - if (!s?.routes || s.index === undefined) { - return arr - } - let childRoute = s.routes[s.index] as Route | undefined - if (!childRoute) { - return arr - } +export const getVisiblePath = (navState?: T.Immutable, includeModals?: boolean) => + NavTree.visiblePath(navState || getRootState(), {includeModals}) - let toAdd: Array - let toAddModals: Array = [] - // special handling of modals, we keep them to the side to add them later, then go down the visible tab - if (depth === 0) { - childRoute = s.routes[0] as Route - toAdd = [childRoute] - // routes[1+] holds both real modals and root non-modal screens (e.g. - // chatConversation on phones, stacked above the tab bar). The latter are - // genuinely visible, so always include them; only gate real modals on includeModals. - const rest = s.routes.slice(1) as Array - toAddModals = inludeModals ? rest : rest.filter(r => !isRootModalRoute(r.name)) - } else { - // include items in the stack - if (s.type === 'stack') { - toAdd = s.routes as Array - } else { - toAdd = [childRoute] - } - } - - const nextArr = [...arr, ...toAdd] - const children = findVisibleRoute(nextArr, childRoute.state, depth + 1) - return [...children, ...toAddModals] - } - - if (!rs) return [] - const vs = findVisibleRoute([], rs, 0) - return vs -} - -export const getModalStack = (navState?: T.Immutable) => { - const rs = navState || getRootState() - if (!rs) { - return [] - } - if (!_isLoggedIn(rs)) { - return [] - } - return (rs.routes?.slice(1) ?? []).filter(r => isRootModalRoute(r.name)) -} +export const getModalStack = (navState?: T.Immutable) => + NavTree.modalStack(navState || getRootState()) -export const getVisibleScreen = (navState?: T.Immutable, _inludeModals?: boolean) => { - const visible = getVisiblePath(navState, _inludeModals ?? true) - return visible.at(-1) -} +export const getVisibleScreen = (navState?: T.Immutable, includeModals?: boolean) => + NavTree.visibleScreen(navState || getRootState(), {includeModals}) export const logState = () => { const rs = getRootState() @@ -221,7 +117,7 @@ export const logState = () => { ps.map(p => ({key: p.key, name: p.name})) const modals = safePaths(getModalStack(rs)) const visible = safePaths(getVisiblePath(rs)) - return {loggedIn: _isLoggedIn(rs), modals, visible} + return {loggedIn: NavTree.isLoggedIn(rs), modals, visible} } // if a toast is inside of a portal then its not in nav so useFocusEffect would throw, @@ -296,13 +192,11 @@ export const clearModals = () => { const n = _getNavigator() if (!n) return const ns = getRootState() - if (!_isLoggedIn(ns)) { + if (!NavTree.isLoggedIn(ns)) { return } const rootRoutes = ns?.routes ?? [] - const keepRoutes = rootRoutes.filter( - (route, index) => index === 0 || !isRootModalRoute(route.name) - ) + const keepRoutes = rootRoutes.filter((route, index) => index === 0 || !NavTree.isModalRouteName(route.name)) if (keepRoutes.length !== rootRoutes.length) { n.dispatch({ ...CommonActions.reset({ @@ -489,8 +383,7 @@ export const switchTab = (name: Tabs.AppTab) => { } const n = _getNavigator() if (!n) return - const ns = getRootState() - const tabNavState = ns?.routes?.[0]?.state + const tabNavState = NavTree.tabNavigatorState(getRootState()) if (!tabNavState?.key) return n.dispatch({ ...TabActions.jumpTo(name), @@ -766,8 +659,7 @@ export const setChatRootParams = ( ): boolean => { const n = _getNavigator() if (!n) return false - const rs = getRootState() - const tabNavState = rs?.routes?.[0]?.state + const tabNavState = NavTree.tabNavigatorState(getRootState()) if (!tabNavState?.key) return false const tabRoutes = tabNavState.routes as Array const chatTabIndex = tabRoutes.findIndex(r => r.name === Tabs.chatTab) @@ -898,18 +790,7 @@ const navToThread = ( return setChatRootParams(params) } else { // Phone: switch to the chat tab, then push the conversation above the tabs. - const nextState = { - index: 1, - routes: [ - { - name: 'loggedIn', - state: { - routes: [{name: Tabs.chatTab, state: {index: 0, routes: [{name: 'chatRoot', params: {}}]}}], - }, - }, - {name: 'chatConversation', params}, - ], - } + const nextState = NavTree.pushedAboveTabs(Tabs.chatTab, {name: 'chatConversation', params}) n.dispatch({ ...CommonActions.reset(nextState as Parameters[0]), target: rs.key, diff --git a/shared/constants/tests/nav-tree.test.ts b/shared/constants/tests/nav-tree.test.ts new file mode 100644 index 000000000000..58b533077332 --- /dev/null +++ b/shared/constants/tests/nav-tree.test.ts @@ -0,0 +1,267 @@ +/// +import * as Tabs from '@/constants/tabs' +import { + activeStack, + currentTab, + isLoggedIn, + modalStack, + modalState, + pushedAboveTabs, + setModalRouteNames, + tabNavigatorState, + tabState, + visiblePath, + visibleScreen, +} from '../nav-tree' + +// Mirror what router-v2 does at startup: register the modal route names so the tree +// can tell real modals from genuinely-visible pushed screens (e.g. chatConversation). +beforeEach(() => { + setModalRouteNames(['chatInfoPanel']) +}) + +// Module-level state — clear it so it can't leak into other tests in this worker. +afterEach(() => { + setModalRouteNames([]) +}) + +// On phones, chatConversation lives in the root stack as a sibling of the tab +// navigator (above the tab bar), not inside a tab. getSelectedConversation calls +// visibleScreen with includeModals=false, so the visible path must still surface +// chatConversation even though it sits at routes[1+] alongside real modals. +const makePhoneNavState = (extraRootRoutes: ReadonlyArray<{name: string; params?: object}> = []) => + ({ + index: extraRootRoutes.length, + key: 'root', + type: 'stack', + routes: [ + { + key: 'loggedIn', + name: 'loggedIn', + state: { + index: 0, + key: 'tabs', + type: 'tab', + routes: [ + { + key: 'chatTab', + name: 'tabs.chatTab', + state: { + index: 0, + key: 'chatStack', + type: 'stack', + routes: [{key: 'chatRoot', name: 'chatRoot'}], + }, + }, + ], + }, + }, + ...extraRootRoutes.map((r, i) => ({key: `extra-${i}`, name: r.name, params: r.params})), + ], + }) as any + +test('visibleScreen with includeModals=false surfaces chatConversation in the phone root stack', () => { + const navState = makePhoneNavState([{name: 'chatConversation', params: {conversationIDKey: 'CONV'}}]) + + const visible = visibleScreen(navState, {includeModals: false}) + + expect(visible?.name).toBe('chatConversation') + expect((visible?.params as {conversationIDKey?: string} | undefined)?.conversationIDKey).toBe('CONV') +}) + +test('visiblePath with includeModals=false includes chatConversation but excludes real modals', () => { + const navState = makePhoneNavState([ + {name: 'chatConversation', params: {conversationIDKey: 'CONV'}}, + {name: 'chatInfoPanel'}, + ]) + + const path = visiblePath(navState, {includeModals: false}).map(r => r.name) + + expect(path).toContain('chatConversation') + expect(path).not.toContain('chatInfoPanel') +}) + +test('visibleScreen returns the topmost convo when multiple are pushed', () => { + const navState = makePhoneNavState([ + {name: 'chatConversation', params: {conversationIDKey: 'CONV1'}}, + {name: 'chatConversation', params: {conversationIDKey: 'CONV2'}}, + ]) + + const visible = visibleScreen(navState, {includeModals: false}) + + expect(visible?.name).toBe('chatConversation') + expect((visible?.params as {conversationIDKey?: string} | undefined)?.conversationIDKey).toBe('CONV2') +}) + +test('visibleScreen(includeModals=false) still surfaces the convo under a modal', () => { + const navState = makePhoneNavState([ + {name: 'chatConversation', params: {conversationIDKey: 'CONV'}}, + {name: 'chatInfoPanel'}, + ]) + + // includeModals=false ignores the modal layered on top and reports the convo, + // matching desktop where the conversation lives in the base (non-modal) layer. + expect(visibleScreen(navState, {includeModals: false})?.name).toBe('chatConversation') + expect(visibleScreen(navState)?.name).toBe('chatInfoPanel') +}) + +test('visiblePath defaults to including real modals', () => { + const navState = makePhoneNavState([ + {name: 'chatConversation', params: {conversationIDKey: 'CONV'}}, + {name: 'chatInfoPanel'}, + ]) + + const path = visiblePath(navState).map(r => r.name) + + expect(path).toContain('chatConversation') + expect(path).toContain('chatInfoPanel') +}) + +test('visiblePath of an empty state is empty', () => { + expect(visiblePath(undefined)).toEqual([]) +}) + +// ---- currentTab / isLoggedIn / modalStack ---- + +test('currentTab reads the selected tab, and is undefined when logged out', () => { + expect(currentTab(makePhoneNavState())).toBe(Tabs.chatTab) + expect(currentTab({index: 0, routes: [{key: 'l', name: 'loggedOut'}]} as any)).toBeUndefined() + expect(currentTab(undefined)).toBeUndefined() +}) + +test('isLoggedIn is true only when the tab navigator is the root route', () => { + expect(isLoggedIn(makePhoneNavState())).toBe(true) + expect(isLoggedIn({index: 0, routes: [{key: 'l', name: 'loggedOut'}]} as any)).toBe(false) + expect(isLoggedIn(undefined)).toBe(false) +}) + +test('modalStack holds only the registered modal names above the tab navigator', () => { + const navState = makePhoneNavState([ + {name: 'chatConversation', params: {conversationIDKey: 'CONV'}}, + {name: 'chatInfoPanel'}, + ]) + + expect(modalStack(navState).map(r => r.name)).toEqual(['chatInfoPanel']) + expect(modalStack(makePhoneNavState())).toEqual([]) + expect(modalStack({index: 0, routes: [{key: 'l', name: 'loggedOut'}]} as any)).toEqual([]) +}) + +test('tabNavigatorState is the tab navigator, and nothing when logged out', () => { + expect(tabNavigatorState(makePhoneNavState())?.key).toBe('tabs') + expect(tabNavigatorState({index: 0, routes: [{key: 'l', name: 'loggedOut', state: {key: 'out'}}]} as any)).toBeUndefined() + expect(tabNavigatorState(undefined)).toBeUndefined() +}) + +// ---- activeStack ---- + +test('activeStack descends to the stack inside the selected tab', () => { + expect(activeStack(makePhoneNavState())?.key).toBe('chatStack') +}) + +// A non-modal screen pushed above the tab bar (the phone thread) is not a stack of its +// own, so pushes still target the selected tab's stack. +test('activeStack ignores non-modal screens pushed above the tabs', () => { + const navState = makePhoneNavState([{name: 'chatConversation', params: {conversationIDKey: 'CONV'}}]) + + expect(activeStack(navState)?.key).toBe('chatStack') +}) + +// A modal has no nested stack state of its own here, so the root stack is what a pop +// would act on. +test('activeStack stops at the root stack when a modal is on top', () => { + const navState = makePhoneNavState([{name: 'chatInfoPanel'}]) + + expect(activeStack(navState)?.key).toBe('root') +}) + +test('activeStack of an empty state is undefined', () => { + expect(activeStack(undefined)).toBeUndefined() + expect(activeStack({routes: []} as any)).toBeUndefined() +}) + +// ---- builders ---- + +test('tabState selects a tab with no screens pushed inside it', () => { + expect(tabState(Tabs.fsTab)).toEqual({ + index: 0, + routes: [{name: 'loggedIn', state: {index: 0, routes: [{name: Tabs.fsTab}]}}], + }) +}) + +test('tabState pushes a screen stack inside the tab and selects its last entry', () => { + expect(tabState(Tabs.peopleTab, [{name: 'peopleRoot'}, {name: 'profile', params: {username: 'testuser'}}])).toEqual( + { + index: 0, + routes: [ + { + name: 'loggedIn', + state: { + index: 0, + routes: [ + { + name: Tabs.peopleTab, + state: { + index: 1, + routes: [{name: 'peopleRoot'}, {name: 'profile', params: {username: 'testuser'}}], + }, + }, + ], + }, + }, + ], + } + ) +}) + +test('modalState without underTab leaves loggedIn on its initial tab', () => { + expect(modalState('settingsPushPrompt')).toEqual({ + index: 1, + routes: [{name: 'loggedIn'}, {name: 'settingsPushPrompt'}], + }) +}) + +test('modalState parks the requested tab beneath the modal and carries params', () => { + expect(modalState('incomingShareNew', {selectedConversationIDKey: 'CONV'}, Tabs.chatTab)).toEqual({ + index: 1, + routes: [ + {name: 'loggedIn', state: {index: 0, routes: [{name: Tabs.chatTab}]}}, + {name: 'incomingShareNew', params: {selectedConversationIDKey: 'CONV'}}, + ], + }) +}) + +// The phone shape: the tab navigator sits at routes[0] on the tab's own root screen, and +// the pushed screen covers it at routes[1]. Every index is spelled out - a tab navigator +// rehydrates a missing index to 0 while a stack rehydrates it to the last route, so +// leaving them off means the shape reads differently at different depths. +test('pushedAboveTabs puts the tab root under a screen pushed above the tab bar', () => { + expect(pushedAboveTabs(Tabs.chatTab, {name: 'chatConversation', params: {conversationIDKey: 'CONV'}})).toEqual({ + index: 1, + routes: [ + { + name: 'loggedIn', + state: { + index: 0, + routes: [{name: Tabs.chatTab, state: {index: 0, routes: [{name: 'chatRoot'}]}}], + }, + }, + {name: 'chatConversation', params: {conversationIDKey: 'CONV'}}, + ], + }) +}) + +test('pushedAboveTabs uses each tab own root screen', () => { + expect(pushedAboveTabs(Tabs.fsTab, {name: 'fsBrowse', params: {path: '/keybase/private/testuser'}})).toEqual({ + index: 1, + routes: [ + { + name: 'loggedIn', + state: { + index: 0, + routes: [{name: Tabs.fsTab, state: {index: 0, routes: [{name: 'fsRoot'}]}}], + }, + }, + {name: 'fsBrowse', params: {path: '/keybase/private/testuser'}}, + ], + }) +}) diff --git a/shared/constants/tests/router-visible.test.ts b/shared/constants/tests/router-visible.test.ts deleted file mode 100644 index ad3b898a7ac5..000000000000 --- a/shared/constants/tests/router-visible.test.ts +++ /dev/null @@ -1,105 +0,0 @@ -/// -import {getVisiblePath, getVisibleScreen, setModalRouteNames} from '../router' - -// Mirror what router-v2 does at startup: register the modal route names so the router -// can tell real modals from genuinely-visible pushed screens (e.g. chatConversation). -beforeEach(() => { - setModalRouteNames(['chatInfoPanel']) -}) - -// Module-level state — clear it so it can't leak into other tests in this worker. -afterEach(() => { - setModalRouteNames([]) -}) - -// On phones, chatConversation lives in the root stack as a sibling of the tab -// navigator (above the tab bar), not inside a tab. getSelectedConversation calls -// getVisibleScreen with includeModals=false, so the visible path must still surface -// chatConversation even though it sits at routes[1+] alongside real modals. -const makePhoneNavState = (extraRootRoutes: ReadonlyArray<{name: string; params?: object}> = []) => - ({ - index: extraRootRoutes.length, - key: 'root', - type: 'stack', - routes: [ - { - key: 'loggedIn', - name: 'loggedIn', - state: { - index: 0, - key: 'tabs', - type: 'tab', - routes: [ - { - key: 'chatTab', - name: 'tabs.chatTab', - state: { - index: 0, - key: 'chatStack', - type: 'stack', - routes: [{key: 'chatRoot', name: 'chatRoot'}], - }, - }, - ], - }, - }, - ...extraRootRoutes.map((r, i) => ({key: `extra-${i}`, name: r.name, params: r.params})), - ], - }) as any - -test('getVisibleScreen with includeModals=false surfaces chatConversation in the phone root stack', () => { - const navState = makePhoneNavState([{name: 'chatConversation', params: {conversationIDKey: 'CONV'}}]) - - const visible = getVisibleScreen(navState, false) - - expect(visible?.name).toBe('chatConversation') - expect((visible?.params as {conversationIDKey?: string} | undefined)?.conversationIDKey).toBe('CONV') -}) - -test('getVisiblePath with includeModals=false includes chatConversation but excludes real modals', () => { - const navState = makePhoneNavState([ - {name: 'chatConversation', params: {conversationIDKey: 'CONV'}}, - {name: 'chatInfoPanel'}, - ]) - - const path = getVisiblePath(navState, false).map(r => r.name) - - expect(path).toContain('chatConversation') - expect(path).not.toContain('chatInfoPanel') -}) - -test('getVisibleScreen returns the topmost convo when multiple are pushed', () => { - const navState = makePhoneNavState([ - {name: 'chatConversation', params: {conversationIDKey: 'CONV1'}}, - {name: 'chatConversation', params: {conversationIDKey: 'CONV2'}}, - ]) - - const visible = getVisibleScreen(navState, false) - - expect(visible?.name).toBe('chatConversation') - expect((visible?.params as {conversationIDKey?: string} | undefined)?.conversationIDKey).toBe('CONV2') -}) - -test('getVisibleScreen(false) still surfaces the convo under a modal', () => { - const navState = makePhoneNavState([ - {name: 'chatConversation', params: {conversationIDKey: 'CONV'}}, - {name: 'chatInfoPanel'}, - ]) - - // includeModals=false ignores the modal layered on top and reports the convo, - // matching desktop where the conversation lives in the base (non-modal) layer. - expect(getVisibleScreen(navState, false)?.name).toBe('chatConversation') - expect(getVisibleScreen(navState, true)?.name).toBe('chatInfoPanel') -}) - -test('getVisiblePath with includeModals=true includes real modals', () => { - const navState = makePhoneNavState([ - {name: 'chatConversation', params: {conversationIDKey: 'CONV'}}, - {name: 'chatInfoPanel'}, - ]) - - const path = getVisiblePath(navState, true).map(r => r.name) - - expect(path).toContain('chatConversation') - expect(path).toContain('chatInfoPanel') -}) diff --git a/shared/fs/common/daemon.tsx b/shared/fs/common/daemon.tsx index df427e8e9031..7630874b90b0 100644 --- a/shared/fs/common/daemon.tsx +++ b/shared/fs/common/daemon.tsx @@ -75,7 +75,7 @@ export const FsDaemonProvider = ({children}: {children: React.ReactNode}) => { // Re-kick the watcher when the daemon handshake (re)completes: the watch loop exits // if the service dies, and a new handshake means RPCs work again. const handshakeDone = useDaemonState(s => s.handshakeState === 'done') - const navState = useRouterState(s => s.navState as RouterConstants.NavState | undefined) + const navState = useRouterState(s => s.navState) const [kbfsDaemonStatus, setKbfsDaemonStatus] = React.useState( Constants.unknownKbfsDaemonStatus ) diff --git a/shared/router-v2/linking-state.test.ts b/shared/router-v2/linking-state.test.ts index ce2c3ad5e3a7..b97bdc1087d8 100644 --- a/shared/router-v2/linking-state.test.ts +++ b/shared/router-v2/linking-state.test.ts @@ -15,8 +15,8 @@ test('an unknown path produces no navigation state', () => { expect(getStateFromPath('/')).toBeUndefined() }) -// spelled out here rather than reusing makeChatConversationState, so a bug shared -// by the builder and the path parser cannot pass unnoticed +// spelled out here rather than reusing makeChatConversationState or NavTree's builders, +// so a bug shared by the builder and the path parser cannot pass unnoticed const chatConversationState = (conversationIDKey: string) => ({ index: 0, routes: [ diff --git a/shared/router-v2/linking.tsx b/shared/router-v2/linking.tsx index 7b22cedf5732..c97ea1e927c6 100644 --- a/shared/router-v2/linking.tsx +++ b/shared/router-v2/linking.tsx @@ -8,6 +8,7 @@ import {useCurrentUserState} from '@/stores/current-user' import {navigationIntentLifetimeMs, useNavigationIntentsState} from '@/stores/navigation-intents' import {useRouterState} from '@/stores/router' import {usePushState} from '@/stores/push' +import * as NavTree from '@/constants/nav-tree' import type {LinkingOptions} from '@react-navigation/native' import type {RootParamList} from './route-params' import {Linking} from 'react-native' @@ -19,72 +20,13 @@ export {emitDeepLink, normalizeUrl} from './deep-link-emitter' // ---- State building helpers ---- -type PartialRoute = { - name: string - params?: Record - state?: PartialNavState -} - -type PartialNavState = { - routes: Array - index?: number -} - -// Build state for navigating to a screen within a tab -const makeTabState = ( - tab: string, - screenStack?: Array<{name: string; params?: Record}> -): PartialNavState => { - const tabRoute: PartialRoute = {name: tab} - if (screenStack && screenStack.length > 0) { - tabRoute.state = { - index: screenStack.length - 1, - routes: screenStack, - } - } - return { - index: 0, - routes: [{name: 'loggedIn', state: {index: 0, routes: [tabRoute]}}], - } -} - // Build state for navigating to a chat conversation -export const makeChatConversationState = (conversationIDKey: string): PartialNavState => { - if (isSplit) { - // Tablet/desktop: chatRoot with conversationIDKey param (split view) - return makeTabState(Tabs.chatTab, [{name: 'chatRoot', params: {conversationIDKey}}]) - } - // Phone: tabs at root, conversation pushed above them - return { - index: 1, - routes: [ - { - name: 'loggedIn', - state: { - index: 0, - routes: [{name: Tabs.chatTab, state: {index: 0, routes: [{name: 'chatRoot', params: {}}]}}], - }, - }, - {name: 'chatConversation', params: {conversationIDKey}}, - ], - } -} - -// Build state for a modal screen at root level. underTab selects which tab sits -// beneath the modal; without it loggedIn falls back to the initial (people) tab. -const makeModalState = ( - modalName: string, - params?: Record, - underTab?: Tabs.AppTab -): PartialNavState => ({ - index: 1, - routes: [ - underTab - ? {name: 'loggedIn', state: {index: 0, routes: [{name: underTab}]}} - : {name: 'loggedIn'}, - {name: modalName, ...(params ? {params} : {})}, - ], -}) +export const makeChatConversationState = (conversationIDKey: string): NavTree.PartialNavState => + isSplit + ? // Tablet/desktop: chatRoot with conversationIDKey param (split view) + NavTree.tabState(Tabs.chatTab, [{name: 'chatRoot', params: {conversationIDKey}}]) + : // Phone: tabs at root, conversation pushed above them + NavTree.pushedAboveTabs(Tabs.chatTab, {name: 'chatConversation', params: {conversationIDKey}}) // ---- URL pattern handling ---- @@ -194,7 +136,7 @@ export const subscribeNavigationIntents = ( const customGetStateFromPath = ( path: string, _options?: object -): PartialNavState | undefined => { +): NavTree.PartialNavState | undefined => { // path has prefix already stripped by React Navigation (e.g., "convid/abc123") const cleanPath = path.replace(/^\/+/, '').replace(/\?.*$/, '') if (!cleanPath) return undefined @@ -213,7 +155,7 @@ const customGetStateFromPath = ( // keybase://profile/show/{username} case 'profile': if (parts[1] === 'show' && parts[2]) { - return makeTabState(Tabs.peopleTab, [ + return NavTree.tabState(Tabs.peopleTab, [ {name: 'peopleRoot'}, {name: 'profile', params: {username: parts[2]}}, ]) @@ -259,23 +201,11 @@ const customGetStateFromPath = ( const path = `/keybase/${decoded}` if (isSplit) { // Tablet: push the folder above the Files tab root, inside the tab stack. - return makeTabState(Tabs.fsTab, [{name: 'fsRoot'}, {name: 'fsBrowse', params: {path}}]) + return NavTree.tabState(Tabs.fsTab, [{name: 'fsRoot'}, {name: 'fsBrowse', params: {path}}]) } // Phone: fsRoot is the only screen in the Files tab stack; folders open as // fsBrowse pushed on the root stack, above the tabs. - return { - index: 1, - routes: [ - { - name: 'loggedIn', - state: { - index: 0, - routes: [{name: Tabs.fsTab, state: {index: 0, routes: [{name: 'fsRoot'}]}}], - }, - }, - {name: 'fsBrowse', params: {path}}, - ], - } + return NavTree.pushedAboveTabs(Tabs.fsTab, {name: 'fsBrowse', params: {path}}) } catch {} break } @@ -285,7 +215,7 @@ const customGetStateFromPath = ( case 'incoming-share': // Share always ends in chat, so park the chat tab (inbox) beneath the modal; // otherwise dismissing/back lands on the initial people tab. - return makeModalState( + return NavTree.modalState( 'incomingShareNew', parts[1] ? {selectedConversationIDKey: stringToConversationIDKey(parts[1])} : undefined, Tabs.chatTab @@ -293,7 +223,7 @@ const customGetStateFromPath = ( // keybase://settingsPushPrompt case 'settingsPushPrompt': - return makeModalState('settingsPushPrompt') + return NavTree.modalState('settingsPushPrompt') // keybase://settingsAddPhone — where https://keybase.io/phone-app lands. Settings sits // under the modal so dismissing it leaves the invitee somewhere they can find it again. @@ -309,7 +239,7 @@ const customGetStateFromPath = ( case Tabs.cryptoTab: case Tabs.devicesTab: case Tabs.gitTab: - return makeTabState(root) + return NavTree.tabState(root) default: break diff --git a/shared/router-v2/routes.tsx b/shared/router-v2/routes.tsx index b8ee9d93b4b3..232be24005ac 100644 --- a/shared/router-v2/routes.tsx +++ b/shared/router-v2/routes.tsx @@ -12,7 +12,6 @@ import {newRoutes as teamsNewRoutes, newModalRoutes as teamsNewModalRoutes} from import {newModalRoutes as walletsNewModalRoutes} from '../wallets/routes' import {newModalRoutes as incomingShareNewModalRoutes} from '../incoming-share/routes' import type * as React from 'react' -import * as Tabs from '@/constants/tabs' import {defineRouteMap} from '@/constants/types/router' import type {GetOptions, GetOptionsParams, GetOptionsRet, RouteDef} from '@/constants/types/router' import type {NativeStackNavigationOptions} from '@react-navigation/native-stack' @@ -57,19 +56,7 @@ if (__DEV__) { ) } -export const tabRoots = { - [Tabs.peopleTab]: 'peopleRoot', - [Tabs.chatTab]: 'chatRoot', - [Tabs.cryptoTab]: 'cryptoRoot', - [Tabs.fsTab]: 'fsRoot', - [Tabs.teamsTab]: 'teamsRoot', - [Tabs.gitTab]: 'gitRoot', - [Tabs.devicesTab]: 'devicesRoot', - [Tabs.settingsTab]: 'settingsRoot', - - [Tabs.loginTab]: '', - [Tabs.searchTab]: '', -} as const +export {tabRoots} from '@/constants/nav-tree' export const modalRoutes = defineRouteMap({ ...chatNewModalRoutes, diff --git a/shared/stores/router.tsx b/shared/stores/router.tsx index ab421dbef069..c7aee6f79dbb 100644 --- a/shared/stores/router.tsx +++ b/shared/stores/router.tsx @@ -1,11 +1,12 @@ import type * as T from '@/constants/types' import * as Z from '@/util/zustand' -import type * as Util from '@/constants/router' +import {castDraft} from 'immer' +import type {NavState} from '@/constants/nav-tree' -export {type NavState} from '@/constants/router' +export {type NavState} from '@/constants/nav-tree' type Store = T.Immutable<{ - navState?: unknown + navState?: NavState }> const initialStore: Store = { @@ -15,7 +16,7 @@ const initialStore: Store = { export type State = Store & { dispatch: { resetState: () => void - setNavState: (ns: Util.NavState) => void + setNavState: (ns: T.Immutable) => void } } @@ -32,10 +33,10 @@ export const useRouterState = Z.createZustand('router', (set, get) => { if (DEBUG_NAV) { console.log('[Nav] setNavState') } - const prev = get().navState as Util.NavState + const prev = get().navState if (prev === next) return set(s => { - s.navState = next + s.navState = castDraft(next) }) }, } From f891b3c2ace32cc9ac703abe8ef2f7af1fef56e2 Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Mon, 28 Sep 2026 10:17:33 -0400 Subject: [PATCH 2/4] refactor(router): build master's devices and phone-app links from the nav tree --- shared/router-v2/linking-phone.test.ts | 2 +- shared/router-v2/linking.tsx | 22 ++++------------------ 2 files changed, 5 insertions(+), 19 deletions(-) diff --git a/shared/router-v2/linking-phone.test.ts b/shared/router-v2/linking-phone.test.ts index ef7328c218be..21f520a3e0dd 100644 --- a/shared/router-v2/linking-phone.test.ts +++ b/shared/router-v2/linking-phone.test.ts @@ -47,7 +47,7 @@ test('the phone shapes are in force -- a conversation opens above the tabs, not name: 'loggedIn', state: { index: 0, - routes: [{name: Tabs.chatTab, state: {index: 0, routes: [{name: 'chatRoot', params: {}}]}}], + routes: [{name: Tabs.chatTab, state: {index: 0, routes: [{name: 'chatRoot'}]}}], }, }, {name: 'chatConversation', params: {conversationIDKey: 'conv-1'}}, diff --git a/shared/router-v2/linking.tsx b/shared/router-v2/linking.tsx index c97ea1e927c6..2c8f01a3beb1 100644 --- a/shared/router-v2/linking.tsx +++ b/shared/router-v2/linking.tsx @@ -167,31 +167,17 @@ const customGetStateFromPath = ( // tablet, and in their own tab on desktop. case 'devices': if (!isMobile) { - return makeTabState(Tabs.devicesTab) + return NavTree.tabState(Tabs.devicesTab) } if (isSplit) { // Tablet: the Settings tab stack holds every settings route, so devices pushes // above the tab root, inside that stack. - return makeTabState(Tabs.settingsTab, [{name: 'settingsRoot'}, {name: Settings.settingsDevicesTab}]) + return NavTree.tabState(Tabs.settingsTab, [{name: 'settingsRoot'}, {name: Settings.settingsDevicesTab}]) } // Phone: settingsRoot is the only screen in the Settings tab stack, so a nested devices // route is filtered out on rehydrate and the tap lands on settingsRoot. Devices is // registered on the root stack there, above the tabs. - return { - index: 1, - routes: [ - { - name: 'loggedIn', - state: { - index: 0, - routes: [ - {name: Tabs.settingsTab, state: {index: 0, routes: [{name: 'settingsRoot'}]}}, - ], - }, - }, - {name: Settings.settingsDevicesTab}, - ], - } + return NavTree.pushedAboveTabs(Tabs.settingsTab, {name: Settings.settingsDevicesTab}) // KBFS paths: keybase://private/..., keybase://public/... case 'private': @@ -228,7 +214,7 @@ const customGetStateFromPath = ( // keybase://settingsAddPhone — where https://keybase.io/phone-app lands. Settings sits // under the modal so dismissing it leaves the invitee somewhere they can find it again. case 'settingsAddPhone': - return makeModalState('settingsAddPhone', undefined, Tabs.settingsTab) + return NavTree.modalState('settingsAddPhone', undefined, Tabs.settingsTab) // Tab switches: keybase://tabs.chatTab, etc. case Tabs.chatTab: From 847ab76e244be5d6c9846431d12996d25470d852 Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Mon, 28 Sep 2026 10:32:51 -0400 Subject: [PATCH 3/4] refactor(router): live-only facade getters; held states go through the pure nav-tree readers The facade getters mapped an undefined state to the live root, so a caller comparing a previous state against the new one read the new state twice when the previous was the store's initial undefined. The files daemon then never saw the first landing on a files screen. The getters now read only the live navigator, and callers holding a state use NavTree directly, where undefined means no state. --- shared/chat/inbox-and-conversation-shared.tsx | 3 +- shared/chat/inbox/metadata.tsx | 16 +++----- shared/constants/chat/common.tsx | 2 +- shared/constants/init/shared.tsx | 10 ++--- shared/constants/router.tsx | 24 +++++------ shared/fs/common/daemon.test.tsx | 40 +++++++++++++++++++ shared/fs/common/daemon.tsx | 6 +-- 7 files changed, 68 insertions(+), 33 deletions(-) create mode 100644 shared/fs/common/daemon.test.tsx diff --git a/shared/chat/inbox-and-conversation-shared.tsx b/shared/chat/inbox-and-conversation-shared.tsx index 14cc052ca727..c4f3354c106f 100644 --- a/shared/chat/inbox-and-conversation-shared.tsx +++ b/shared/chat/inbox-and-conversation-shared.tsx @@ -2,6 +2,7 @@ import * as C from '@/constants' import * as Chat from '@/constants/chat' import * as Kb from '@/common-adapters' +import * as NavTree from '@/constants/nav-tree' import * as React from 'react' import type * as T from '@/constants/types' import Conversation from '@/chat/conversation/container' @@ -29,7 +30,7 @@ export function InboxAndConversationShell(props: Props) { const validConvoID = conversationIDKey && conversationIDKey !== Chat.noConversationIDKey const lastValidCIDRef = React.useRef(validConvoID ? conversationIDKey : '') const chatTabSelected = C.useRouterState(s => { - const storedTab = C.Router2.getTab(s.navState) + const storedTab = NavTree.currentTab(s.navState) return (storedTab ?? C.Router2.getTab()) === C.Tabs.chatTab }) const firstSmallTeam = useInboxLayoutState(s => { diff --git a/shared/chat/inbox/metadata.tsx b/shared/chat/inbox/metadata.tsx index a3ba7f8f1dcf..24f5859b8d41 100644 --- a/shared/chat/inbox/metadata.tsx +++ b/shared/chat/inbox/metadata.tsx @@ -4,12 +4,8 @@ import {useInboxMetadataState, metasReceived, participantInfoReceived} from './m export {useInboxMetadataState, metasReceived, participantInfoReceived} from './metadata-store' import * as T from '@/constants/types' import type * as EngineGen from '@/constants/rpc' -import { - getModalStack, - getVisibleScreen, - navigateToInbox, - navigateToThread as routerNavigateToThread, -} from '@/constants/router' +import * as NavTree from '@/constants/nav-tree' +import {navigateToInbox, navigateToThread as routerNavigateToThread} from '@/constants/router' import type * as Router2 from '@/constants/router' import logger from '@/logger' import {ignorePromise, timeoutPromise} from '@/constants/utils' @@ -205,13 +201,13 @@ export const onChatRouteChanged = ( prev: T.Immutable, next: T.Immutable ) => { - const wasModal = prev && getModalStack(prev).length > 0 - const isModal = next && getModalStack(next).length > 0 + const wasModal = prev && NavTree.modalStack(prev).length > 0 + const isModal = next && NavTree.modalStack(next).length > 0 if (wasModal || isModal) { return } - const p = getVisibleScreen(prev) - const n = getVisibleScreen(next) + const p = NavTree.visibleScreen(prev) + const n = NavTree.visibleScreen(next) const wasChat = p?.name === Common.threadRouteName const isChat = n?.name === Common.threadRouteName if (!wasChat && !isChat) { diff --git a/shared/constants/chat/common.tsx b/shared/constants/chat/common.tsx index 934d22f10b50..f43b1e2ac625 100644 --- a/shared/constants/chat/common.tsx +++ b/shared/constants/chat/common.tsx @@ -6,7 +6,7 @@ import {isSplit, threadRouteName} from './layout' export const explodingModeGregorKeyPrefix = 'exploding:' export const getSelectedConversation = (allowUnderModal: boolean = false): T.Chat.ConversationIDKey => { - const maybeVisibleScreen = getVisibleScreen(undefined, allowUnderModal) + const maybeVisibleScreen = getVisibleScreen(allowUnderModal) if (maybeVisibleScreen?.name === threadRouteName) { const mParams = maybeVisibleScreen.params as undefined | {conversationIDKey?: T.Chat.ConversationIDKey} return mParams?.conversationIDKey ?? T.Chat.noConversationIDKey diff --git a/shared/constants/init/shared.tsx b/shared/constants/init/shared.tsx index 74711eef175b..4bd44f36790f 100644 --- a/shared/constants/init/shared.tsx +++ b/shared/constants/init/shared.tsx @@ -40,7 +40,7 @@ import { useSettingsContactsState } from "@/stores/settings-contacts"; import { useUsersState } from "@/stores/users"; import { useWaitingState } from "@/stores/waiting"; import { useRouterState } from "@/stores/router"; -import * as Util from "@/constants/router"; +import * as NavTree from "@/constants/nav-tree"; import { handleConvoEngineIncoming } from "@/chat/inbox/engine"; import { onChatRouteChanged, @@ -475,9 +475,9 @@ const onNavStateChanged = ( // Clear critical update when we nav away from tab if ( prev && - Util.getTab(prev) === Tabs.fsTab && + NavTree.currentTab(prev) === Tabs.fsTab && next && - Util.getTab(next) !== Tabs.fsTab && + NavTree.currentTab(next) !== Tabs.fsTab && useShellState.getState().fsCriticalUpdate ) { const { dispatch } = useShellState.getState(); @@ -486,9 +486,9 @@ const onNavStateChanged = ( if ( prev && - Util.getTab(prev) === Tabs.teamsTab && + NavTree.currentTab(prev) === Tabs.teamsTab && next && - Util.getTab(next) !== Tabs.teamsTab + NavTree.currentTab(next) !== Tabs.teamsTab ) { clearNavBadges(); } diff --git a/shared/constants/router.tsx b/shared/constants/router.tsx index 77e9b6a2c730..71436e706660 100644 --- a/shared/constants/router.tsx +++ b/shared/constants/router.tsx @@ -90,33 +90,31 @@ export const getRootState = (): NavState | undefined => { return navigationRef.getRootState() } -export const getTab = (navState?: T.Immutable): undefined | Tabs.Tab => - NavTree.currentTab(navState || getRootState()) +// These read the live navigator. To read a state you already hold (e.g. a previous one), use +// the pure readers in nav-tree directly: they treat undefined as no state, not as "now". +export const getTab = (): undefined | Tabs.Tab => NavTree.currentTab(getRootState()) export const _getNavigator = () => { return navigationRef.isReady() ? navigationRef : undefined } -const getActiveStackState = (navState?: T.Immutable) => - NavTree.activeStack(navState || getRootState()) +const getActiveStackState = () => NavTree.activeStack(getRootState()) // Public API // gives you loggedin/tab/stackitems + modals -export const getVisiblePath = (navState?: T.Immutable, includeModals?: boolean) => - NavTree.visiblePath(navState || getRootState(), {includeModals}) +export const getVisiblePath = (includeModals?: boolean) => NavTree.visiblePath(getRootState(), {includeModals}) -export const getModalStack = (navState?: T.Immutable) => - NavTree.modalStack(navState || getRootState()) +export const getModalStack = () => NavTree.modalStack(getRootState()) -export const getVisibleScreen = (navState?: T.Immutable, includeModals?: boolean) => - NavTree.visibleScreen(navState || getRootState(), {includeModals}) +export const getVisibleScreen = (includeModals?: boolean) => + NavTree.visibleScreen(getRootState(), {includeModals}) export const logState = () => { const rs = getRootState() const safePaths = (ps: ReadonlyArray<{key?: string; name?: string}>) => ps.map(p => ({key: p.key, name: p.name})) - const modals = safePaths(getModalStack(rs)) - const visible = safePaths(getVisiblePath(rs)) + const modals = safePaths(NavTree.modalStack(rs)) + const visible = safePaths(NavTree.visiblePath(rs)) return {loggedIn: NavTree.isLoggedIn(rs), modals, visible} } @@ -308,7 +306,7 @@ export function navigateAppend(path: NavigateAppendType, replace?: boolean): boo } return false } - const vp = getVisiblePath(ns) + const vp = NavTree.visiblePath(ns) const visible = vp.at(-1) if (visible) { if (routeName === visible.name && shallowEqual(visible.params, params)) { diff --git a/shared/fs/common/daemon.test.tsx b/shared/fs/common/daemon.test.tsx new file mode 100644 index 000000000000..2f1fecd8af64 --- /dev/null +++ b/shared/fs/common/daemon.test.tsx @@ -0,0 +1,40 @@ +/** @jest-environment jsdom */ +/// +import * as NavTree from '@/constants/nav-tree' +import * as Tabs from '@/constants/tabs' +import {act, cleanup, render} from '@testing-library/react' +import {navigationRef} from '@/constants/router' +import {useRouterState} from '@/stores/router' +import {resetAllStores} from '@/util/zustand' +import {FsDaemonProvider} from './daemon' +import {fsUserIn, fsUserOut} from './lifecycle' + +jest.mock('./lifecycle', () => ({ + afterKbfsDaemonRpcStatusChanged: jest.fn(), + fsUserIn: jest.fn(), + fsUserOut: jest.fn(), +})) + +const fsState = NavTree.tabState(Tabs.fsTab, [{name: 'fsRoot'}]) as unknown as NavTree.NavState + +afterEach(() => { + cleanup() + jest.clearAllMocks() + resetAllStores() +}) + +// The router store starts with no nav state, so the first state the provider sees has an +// undefined predecessor. That must read as "was on no screen", not as whatever the live +// navigator shows right now - which is already the new state. +test('landing on a files screen from the initial undefined nav state counts as entering files', () => { + NavTree.setModalRouteNames([]) + ;(navigationRef as unknown as Record)['getRootState'] = () => fsState + render({null}) + + act(() => { + useRouterState.setState({navState: fsState}) + }) + + expect(fsUserIn).toHaveBeenCalledTimes(1) + expect(fsUserOut).not.toHaveBeenCalled() +}) diff --git a/shared/fs/common/daemon.tsx b/shared/fs/common/daemon.tsx index 7630874b90b0..072c52413436 100644 --- a/shared/fs/common/daemon.tsx +++ b/shared/fs/common/daemon.tsx @@ -1,7 +1,7 @@ import * as C from '@/constants' import * as Constants from '@/constants/fs' import * as React from 'react' -import * as RouterConstants from '@/constants/router' +import * as NavTree from '@/constants/nav-tree' import * as T from '@/constants/types' import {useConfigState} from '@/stores/config' import {useDaemonState} from '@/stores/daemon' @@ -167,8 +167,8 @@ export const FsDaemonProvider = ({children}: {children: React.ReactNode}) => { return } - const wasScreen = fsRouteNames.includes(RouterConstants.getVisibleScreen(previousNavState)?.name ?? '') - const isScreen = fsRouteNames.includes(RouterConstants.getVisibleScreen(navState)?.name ?? '') + const wasScreen = fsRouteNames.includes(NavTree.visibleScreen(previousNavState)?.name ?? '') + const isScreen = fsRouteNames.includes(NavTree.visibleScreen(navState)?.name ?? '') if (wasScreen === isScreen) { return } From 89d04ece6ded5e4e9fb8806a6ae720e2fc32ee74 Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Mon, 28 Sep 2026 10:49:42 -0400 Subject: [PATCH 4/4] refactor(chat): drop getSelectedConversation's unused allowUnderModal parameter --- shared/constants/chat/common.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/shared/constants/chat/common.tsx b/shared/constants/chat/common.tsx index f43b1e2ac625..fab7bdfc09b1 100644 --- a/shared/constants/chat/common.tsx +++ b/shared/constants/chat/common.tsx @@ -5,8 +5,8 @@ import {isSplit, threadRouteName} from './layout' export const explodingModeGregorKeyPrefix = 'exploding:' -export const getSelectedConversation = (allowUnderModal: boolean = false): T.Chat.ConversationIDKey => { - const maybeVisibleScreen = getVisibleScreen(allowUnderModal) +export const getSelectedConversation = (): T.Chat.ConversationIDKey => { + const maybeVisibleScreen = getVisibleScreen(false) if (maybeVisibleScreen?.name === threadRouteName) { const mParams = maybeVisibleScreen.params as undefined | {conversationIDKey?: T.Chat.ConversationIDKey} return mParams?.conversationIDKey ?? T.Chat.noConversationIDKey