From 41fb3c7a9c494ee252c6e5e654b454c266d1cd9d Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Wed, 21 Oct 2026 16:41:13 -0700 Subject: [PATCH] fix(mobile): retire host edit navigation after leaving the route (#31692) Saving an endpoint after the user leaves can pop a second screen. Retire navigation ownership while allowing the endpoint save and client refresh to finish. --- mobile/app/h/[hostId]/edit.tsx | 87 +++++++++++++++++--------- mobile/src/host-edit-save-flow.test.ts | 54 +++++++++++----- mobile/tests-typecheck-baseline.txt | 1 - 3 files changed, 96 insertions(+), 46 deletions(-) diff --git a/mobile/app/h/[hostId]/edit.tsx b/mobile/app/h/[hostId]/edit.tsx index 44991156903..53dc48d3af2 100644 --- a/mobile/app/h/[hostId]/edit.tsx +++ b/mobile/app/h/[hostId]/edit.tsx @@ -37,33 +37,55 @@ export default function EditHostScreen() { // Why: setSaving is async, so a second trigger before the re-render could // still read stale state and re-enter handleSave; the ref closes that race. const savingRef = useRef(false) + const routeRevision = useRef(0) + const leave = () => { + routeRevision.current += 1 + router.back() + } - const load = useCallback(async () => { - if (!hostId) { - setLoadError('Missing host.') - return - } - try { - const hosts = await loadHosts() - const found = hosts.find((h) => h.id === hostId) ?? null - if (!found) { - setLoadError('This host was removed from this phone.') - setHost(null) + const load = useCallback( + async (revision: number) => { + if (!hostId) { + setLoadError('Missing host.') return } - setHost(found) - // The field edits the phone's override; an empty field means "use the desktop's name". - setName(found.personalName ?? '') - setAddress(displayHostEndpoint(found.endpoint)) - setLoadError(null) - } catch (err) { - setLoadError(err instanceof Error ? err.message : 'Failed to load host.') - setHost(null) - } - }, [hostId]) + try { + const hosts = await loadHosts() + if (revision !== routeRevision.current) { + return + } + const found = hosts.find((h) => h.id === hostId) ?? null + if (!found) { + setLoadError('This host was removed from this phone.') + setHost(null) + return + } + setHost(found) + // The field edits the phone's override; an empty field means "use the desktop's name". + setName(found.personalName ?? '') + setAddress(displayHostEndpoint(found.endpoint)) + setLoadError(null) + } catch (err) { + if (revision !== routeRevision.current) { + return + } + setLoadError(err instanceof Error ? err.message : 'Failed to load host.') + setHost(null) + } + }, + [hostId] + ) useEffect(() => { - void load() + const revision = ++routeRevision.current + setHost(null) + setLoadError(null) + savingRef.current = false + setSaving(false) + void load(revision) + return () => { + routeRevision.current += 1 + } }, [load]) const endpointEdit = useMemo( @@ -98,6 +120,7 @@ export default function EditHostScreen() { return } + const revision = routeRevision.current savingRef.current = true setSaving(true) setSaveError(null) @@ -110,9 +133,11 @@ export default function EditHostScreen() { ...(nextEndpoint !== undefined ? { endpoint: nextEndpoint } : {}) }) } catch (err) { - setSaveError(err instanceof Error ? err.message : 'Failed to save host.') - savingRef.current = false - setSaving(false) + if (revision === routeRevision.current) { + setSaveError(err instanceof Error ? err.message : 'Failed to save host.') + savingRef.current = false + setSaving(false) + } return } @@ -126,9 +151,11 @@ export default function EditHostScreen() { // best-effort re-prime; persisted data is unaffected } - savingRef.current = false - setSaving(false) - router.back() + if (revision === routeRevision.current) { + savingRef.current = false + setSaving(false) + leave() + } if (nextEndpoint !== undefined) { // Why: the live client, even one riding the relay, and its primed profile hold the old address. @@ -141,7 +168,7 @@ export default function EditHostScreen() { router.back()} + onPress={leave} accessibilityRole="button" accessibilityLabel="Back" > @@ -169,7 +196,7 @@ export default function EditHostScreen() { {loadError ? ( {loadError} - router.back()}> + Go back diff --git a/mobile/src/host-edit-save-flow.test.ts b/mobile/src/host-edit-save-flow.test.ts index 8a3aec82442..9f32d686dbd 100644 --- a/mobile/src/host-edit-save-flow.test.ts +++ b/mobile/src/host-edit-save-flow.test.ts @@ -74,7 +74,7 @@ function setFieldValue( value: string ): void { const input = renderer.root - .findAllByType('TextInput') + .findAll((node) => String(node.type) === 'TextInput') .find((node) => node.props.accessibilityLabel === accessibilityLabel) if (!input) { throw new Error(`${accessibilityLabel} input not found`) @@ -86,7 +86,7 @@ function setFieldValue( function findSaveButton(renderer: ReactTestRenderer) { const button = renderer.root - .findAllByType('Pressable') + .findAll((node) => String(node.type) === 'Pressable') .find((node) => node.props.accessibilityLabel === 'Save host') if (!button) { throw new Error('Save button not found') @@ -104,16 +104,18 @@ async function pressSave(renderer: ReactTestRenderer): Promise { } function findText(renderer: ReactTestRenderer, match: string): boolean { - return renderer.root.findAllByType('Text').some((node) => { - const children = node.props.children - if (typeof children === 'string') { - return children.includes(match) - } - if (Array.isArray(children)) { - return children.join('').includes(match) - } - return false - }) + return renderer.root + .findAll((node) => String(node.type) === 'Text') + .some((node) => { + const children = node.props.children + if (typeof children === 'string') { + return children.includes(match) + } + if (Array.isArray(children)) { + return children.join('').includes(match) + } + return false + }) } describe('edit host handleSave', () => { @@ -261,6 +263,28 @@ describe('edit host handleSave', () => { act(() => renderer.unmount()) }) + it('does not navigate again if the user leaves while the save is pending', async () => { + let resolveSave = () => {} + dependencies.updateHostNameAndEndpoint.mockImplementationOnce( + () => + new Promise((resolve) => { + resolveSave = resolve + }) + ) + const renderer = await renderEditHostRoute() + setFieldValue(renderer, 'Address', '192.168.1.20:6768') + await pressSave(renderer) + act(() => { + renderer.root.findByProps({ accessibilityLabel: 'Back' }).props.onPress() + renderer.unmount() + }) + expect(dependencies.back).toHaveBeenCalledTimes(1) + await act(async () => { + resolveSave() + }) + expect(dependencies.back).toHaveBeenCalledTimes(1) + expect(dependencies.refreshHostClient).toHaveBeenCalledWith('host-1') + }) }) describe('edit host load() error states', () => { @@ -282,7 +306,7 @@ describe('edit host load() error states', () => { const renderer = await renderEditHostRoute() expect(findText(renderer, 'Missing host.')).toBe(true) - expect(renderer.root.findAllByType('TextInput')).toHaveLength(0) + expect(renderer.root.findAll((node) => String(node.type) === 'TextInput')).toHaveLength(0) act(() => renderer.unmount()) }) @@ -292,7 +316,7 @@ describe('edit host load() error states', () => { const renderer = await renderEditHostRoute() expect(findText(renderer, 'This host was removed from this phone.')).toBe(true) - expect(renderer.root.findAllByType('TextInput')).toHaveLength(0) + expect(renderer.root.findAll((node) => String(node.type) === 'TextInput')).toHaveLength(0) act(() => renderer.unmount()) }) @@ -302,7 +326,7 @@ describe('edit host load() error states', () => { const renderer = await renderEditHostRoute() expect(findText(renderer, 'storage unreadable')).toBe(true) - expect(renderer.root.findAllByType('TextInput')).toHaveLength(0) + expect(renderer.root.findAll((node) => String(node.type) === 'TextInput')).toHaveLength(0) act(() => renderer.unmount()) }) diff --git a/mobile/tests-typecheck-baseline.txt b/mobile/tests-typecheck-baseline.txt index 2277479ae94..c77d8623c97 100644 --- a/mobile/tests-typecheck-baseline.txt +++ b/mobile/tests-typecheck-baseline.txt @@ -25,7 +25,6 @@ src/files/MobileFileExplorerPanel.test.ts src/files/MobileFileMarkdownPreview.test.ts src/hooks/mobile-dictation-desktop-start.test.ts src/host-edit-route-accessibility.test.ts -src/host-edit-save-flow.test.ts src/mock-server-key-pair.test.ts src/notifications/notification-consent-ownership.test.ts src/notifications/push-dismissal-native-races.test.ts