Skip to content

refactor(router): a Navigator adapter as the seam - #29611

Draft
chrisnojima wants to merge 1 commit into
nojima/HOTPOT-arch-06-nav-treefrom
nojima/HOTPOT-arch-07-navigator-adapter
Draft

refactor(router): a Navigator adapter as the seam#29611
chrisnojima wants to merge 1 commit into
nojima/HOTPOT-arch-06-nav-treefrom
nojima/HOTPOT-arch-07-navigator-adapter

Conversation

@chrisnojima

@chrisnojima chrisnojima commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Problem

The navigation seam was "a module of 25 functions reaching a process-global navigationRef". Because there was no injectable adapter, the only way to substitute navigation was to mock the module — so 14 test files hand-rolled jest.mock('@/constants/router', ...), each declaring a different partial shape of it.

navigateAppend also carried hidden temporal state: a 1000ms in-flight dedupe window that no test could reach.

util/safe-navigation.tsx (12 lines) was the only hook-shaped adapter, covering 2 of ~25 operations — ~60 call sites against ~479 going straight to the module global.

Change

New constants/navigator.tsx:

NavigatorRef = {isReady, getRootState, dispatch, addListener}
makeNavigator(ref)   // adds the seven tree-aware ops on top

Exactly two adapters — the real container-ref binding and test/fake-navigator.ts — which is what makes the seam real rather than hypothetical.

constants/router.tsx's free functions become one-line bindings to getNavigator(), so all ~479 call sites are untouched.

Mocks eliminated

15 of 15. The 14 jest.mock factories, plus constants/navigate-to-thread.test.ts, which stubbed the container ref by hand and carried a 10-line comment working around the module-global _pendingAppend — that cache is now per-navigator, so the workaround is gone.

Previously untestable, now tested

The in-flight dedupe window, the visible-route dupe check, navUpToScreen's three branches (popTo / in-place reset / replaceIfMissing), clearModals' filter, switchTab, setChatRootParams, navigateUp/popStack. constants/tests/navigator.test.ts is 29 tests.

14 mutations, each caught — including dropping the in-flight dedupe, widening the window to 100s, neutering clearModals' filter, and making the fake ignore ready.

Fixed along the way

test/mocks/react-navigation-core.js's CommonActions.navigate and setParams built payloads no version of React Navigation produces (the bare name; params without their {params} wrapper). Tests asserting on those actions were asserting a shape production never dispatches.

Behaviour change, flagged not hidden

switchTab/setChatRootParams now bail explicitly when routes[0] isn't loggedIn. Previously switchTab dispatched TabActions.jumpTo at the logged-out stack router, which has no JUMP_TO handler — same outcome, now intentional.

Out of scope

Chat logic stays in constants/router.tsx. The dispatches in router-v2/common.tsx, tab-bar.desktop.tsx and teams/confirm-modals/ target a specific navigator handed to a component, not the root — not this seam, noted in the module header.

Validation

lint:all clean — 0 bailed out, 0 whole-props deps, tsc clean both projects.
jest --runInBand231 suites / 2281 tests (baseline 230 / 2236).

@chrisnojima
chrisnojima added this pull request to stack #29615 September 9, 2026 19:40
Navigation was "a module of functions reaching a process-global
NavigationContainerRef", so the only way to substitute it in a test was to mock
the module: fourteen test files hand-rolled jest.mock('@/constants/router'),
each declaring a different partial shape, and none of them exercised the code
they were replacing.

constants/navigator defines one Navigator interface - dispatch, getRootState,
isReady, addListener - and makeNavigator builds the tree-shape-aware operations
(navigateUp, popStack, clearModals, navigateAppend, navUpToScreen, switchTab,
setChatRootParams) on top of it. There are exactly two adapters: the real
NavigationContainerRef binding, and test/fake-navigator's in-memory one, which
records dispatches and serves whatever root state a test sets. They agree on
readiness - both drop dispatches and report no root state until the container
has mounted - because callers rely on that instead of guarding themselves.

The free functions in constants/router are now a thin binding to the default
adapter, so no call site changed.

navigateAppend's in-flight dedupe cache moves from module scope onto the
navigator instance. That is what made it untestable: it outlived every test in
a file, and jest.resetModules() would have handed the code under test a
different navigationRef. constants/tests/navigator.test covers it now, together
with the 1000ms backstop, navUpToScreen's three branches, clearModals' filter
and setChatRootParams' in-place/reset split.

All fourteen mock factories are converted, plus navigate-to-thread.test, which
was stubbing the container ref by hand and working around the module-level
cache with per-test conversation ids.

test/mocks/react-navigation-core's CommonActions.navigate and setParams built
payloads that no version of React Navigation produces (the bare name, and the
params without their {params} wrapper), so a test asserting on one of those
actions was asserting a shape production never dispatches. They now match
@react-navigation/routers.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015rccpV5nLxxC5opF5xzrz7
@chrisnojima
chrisnojima force-pushed the nojima/HOTPOT-arch-07-navigator-adapter branch from 98b64c4 to 99dddad Compare September 11, 2026 01:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant