diff --git a/src/screens/ContactDetailScreen.tsx b/src/screens/ContactDetailScreen.tsx index 3c835af9..5d905da2 100644 --- a/src/screens/ContactDetailScreen.tsx +++ b/src/screens/ContactDetailScreen.tsx @@ -146,6 +146,17 @@ export default function ContactDetailScreen() { const [moreOpen, setMoreOpen] = React.useState(false); const [moveOpen, setMoveOpen] = React.useState(false); const [groupPickerOpen, setGroupPickerOpen] = React.useState(false); + // Every hook runs before the not-found return: deleting the contact (or a + // sync removing it) re-renders this screen without it, and a hook that only + // ran with a contact crashed React with "fewer hooks than expected". + const members = contact?.members; + const memberContacts = React.useMemo(() => { + if (!members) return []; + const memberIds = Object.keys(members).filter((k) => members[k]); + return memberIds + .map((mid) => allContacts.find((c) => c.id === mid || c.uid === mid)) + .filter(Boolean) as ContactCard[]; + }, [members, allContacts]); if (!contact) { return ( @@ -214,14 +225,6 @@ export default function ContactDetailScreen() { return 0; })(); - const memberContacts = React.useMemo(() => { - if (!contact.members) return []; - const memberIds = Object.keys(contact.members).filter((k) => contact.members![k]); - return memberIds - .map((mid) => allContacts.find((c) => c.id === mid || c.uid === mid)) - .filter(Boolean) as ContactCard[]; - }, [contact.members, allContacts]); - const openMail = (addr: string) => { navigation.navigate('Compose', { prefillTo: [{ name, email: addr }], diff --git a/src/screens/GroupDetailScreen.tsx b/src/screens/GroupDetailScreen.tsx index f1f23741..f0e36b83 100644 --- a/src/screens/GroupDetailScreen.tsx +++ b/src/screens/GroupDetailScreen.tsx @@ -54,6 +54,9 @@ export default function GroupDetailScreen() { const [pickerOpen, setPickerOpen] = React.useState(false); const [confirmDelete, setConfirmDelete] = React.useState(false); const [removingMember, setRemovingMember] = React.useState(null); + // Before the not-found return, so deleting the group doesn't change the + // number of hooks (React throws on that). + const excludedIds = React.useMemo(() => new Set(members.map((m) => m.id)), [members]); if (!group) { return ( @@ -127,8 +130,6 @@ export default function GroupDetailScreen() { } }; - const excludedIds = React.useMemo(() => new Set(members.map((m) => m.id)), [members]); - return ( diff --git a/src/screens/__tests__/hooks-before-not-found.test.ts b/src/screens/__tests__/hooks-before-not-found.test.ts new file mode 100644 index 00000000..9c770d6a --- /dev/null +++ b/src/screens/__tests__/hooks-before-not-found.test.ts @@ -0,0 +1,32 @@ +// A detail screen whose record disappears (deleted here, or removed by a +// sync) re-renders down its not-found branch. Any hook placed after that +// early return then runs one time fewer and React throws "Rendered fewer +// hooks than expected", which took the whole screen down on delete. This +// scans the screen components for hook calls after their not-found return. +import { describe, it, expect } from 'vitest'; +import { readFileSync } from 'node:fs'; +import { join } from 'node:path'; + +const SCREENS = join(__dirname, '..'); + +/** Hook calls in `component` after the line `marker`, up to its closing brace. */ +function hooksAfter(file: string, component: string, marker: string): string[] { + const src = readFileSync(join(SCREENS, file), 'utf8'); + const start = src.indexOf(`export default function ${component}(`); + expect(start).toBeGreaterThanOrEqual(0); + const rest = src.slice(start); + const body = rest.slice(0, rest.search(/\r?\n\}\r?\n/)); + const at = body.indexOf(marker); + expect(at).toBeGreaterThanOrEqual(0); + return body.slice(at).match(/\b(?:React\.)?use[A-Z]\w*\(/g) ?? []; +} + +describe('hooks run before the not-found return', () => { + it('ContactDetailScreen', () => { + expect(hooksAfter('ContactDetailScreen.tsx', 'ContactDetailScreen', 'if (!contact) {')).toEqual([]); + }); + + it('GroupDetailScreen', () => { + expect(hooksAfter('GroupDetailScreen.tsx', 'GroupDetailScreen', 'if (!group) {')).toEqual([]); + }); +});