diff --git a/mobile/app/h/[hostId]/tasks.tsx b/mobile/app/h/[hostId]/tasks.tsx index 71abd94e924..0f870c3e5d7 100644 --- a/mobile/app/h/[hostId]/tasks.tsx +++ b/mobile/app/h/[hostId]/tasks.tsx @@ -2637,6 +2637,15 @@ export default function MobileTasksScreen() { // Why: task-loading effects use this as a stale-client guard, so the ref // must be current before those passive effects can run after commit. + const resetGitHubItemsState = useCallback(() => { + setGithubRepoSources({}) + setGithubPages([]) + setGithubCurrentPage(0) + setGithubTotalCount(null) + setGithubSourceErrors([]) + setGithubSourceFallbacks([]) + }, []) + // Why: Expo reuses this screen for the next host, so an effect reset runs a // render too late and the previous host's rows show under the new one. The // repo list resets itself; these are the other client-scoped caches. @@ -2645,6 +2654,7 @@ export default function MobileTasksScreen() { setBoundClient(client) setItems([]) setGithubRepoSlugCache({}) + resetGitHubItemsState() } useLayoutEffect(() => { @@ -2906,7 +2916,7 @@ export default function MobileTasksScreen() { // pair but must not receive the newer task-specific method calls. setTasksSupportState({ kind: 'unsupported', client }) setItems([]) - setGithubPages([]) + resetGitHubItemsState() setGithubProjectTable(null) setShowLinearWorkspacePicker(false) setShowLinearTeamPicker(false) @@ -3274,11 +3284,7 @@ export default function MobileTasksScreen() { } try { if (provider !== 'github' || githubMode !== 'items') { - setGithubPages([]) - setGithubCurrentPage(0) - setGithubTotalCount(null) - setGithubSourceErrors([]) - setGithubSourceFallbacks([]) + resetGitHubItemsState() } if (provider === 'linear' && !linearConnected) { setItems([]) @@ -3309,11 +3315,7 @@ export default function MobileTasksScreen() { return } setItems([]) - setGithubPages([]) - setGithubCurrentPage(0) - setGithubTotalCount(null) - setGithubSourceErrors([]) - setGithubSourceFallbacks([]) + resetGitHubItemsState() return } if (provider === 'github') { @@ -3448,8 +3450,7 @@ export default function MobileTasksScreen() { return } setItems([]) - setGithubSourceErrors([]) - setGithubSourceFallbacks([]) + resetGitHubItemsState() setError(err instanceof Error ? err.message : 'Failed to load tasks') } finally { if (isCurrent()) { @@ -3470,6 +3471,8 @@ export default function MobileTasksScreen() { linearConnected, linearFilter, linearOrderBy, + // resetGitHubItemsState is useCallback([]), so its identity never changes + // and listing it here would only cost a line against the max-lines budget. repoListEnsureLoaded, provider, selectedLinearTeamIds, diff --git a/mobile/src/tasks/github-project-repo-list-load.test.ts b/mobile/src/tasks/github-project-repo-list-load.test.ts index 8cd4f99deb4..96658c3dfa9 100644 --- a/mobile/src/tasks/github-project-repo-list-load.test.ts +++ b/mobile/src/tasks/github-project-repo-list-load.test.ts @@ -8,14 +8,20 @@ import { describe, expect, it } from 'vitest' * these pin the consumption points the original bug lived in. */ const source = readFileSync(new URL('../../app/h/[hostId]/tasks.tsx', import.meta.url), 'utf8') -function loadTasksBody(): string { - const start = source.indexOf('const loadTasks = useCallback(') - expect(start, 'loadTasks must exist in the tasks route').toBeGreaterThan(-1) - const end = source.indexOf('const connectLinearAccount = useCallback(', start) - expect(end, 'loadTasks must be followed by connectLinearAccount').toBeGreaterThan(start) +/** Why: `slice(start, -1)` silently trims one byte instead of failing, so a + * missing end marker would let every assertion below pass vacuously. */ +function block(startMarker: string, endMarker: string): string { + const start = source.indexOf(startMarker) + expect(start, `tasks.tsx must contain ${startMarker}`).toBeGreaterThan(-1) + const end = source.indexOf(endMarker, start) + expect(end, `${startMarker} must be followed by ${endMarker}`).toBeGreaterThan(start) return source.slice(start, end) } +function loadTasksBody(): string { + return block('const loadTasks = useCallback(', 'const connectLinearAccount = useCallback(') +} + describe('mobile GitHub Project repo list loading', () => { // Regression (#12966): project mode returned before the repo list was ever // loaded, so hostedRepos stayed empty and every board row was filtered out. @@ -55,21 +61,28 @@ describe('mobile GitHub Project readiness and refresh', () => { // `every` is vacuously true on an empty list, so readiness must ask the // resource whether that list is real yet rather than infer it. it('derives slug readiness from the resource status', () => { - const start = source.indexOf('const githubProjectRepoSlugReady = useMemo(') - expect(start, 'slug readiness memo must exist').toBeGreaterThan(-1) - const body = source.slice(start, source.indexOf(' )', start)) + const body = block('const githubProjectRepoSlugReady = useMemo(', ' )') expect(body).toContain('hasSettledHostRepoList(repoList.state)') expect(body).toContain('[githubRepoSlugCache, hostedRepos, repoList.state]') }) - it('re-reads the host and retries failed slug lookups on refresh', () => { - const start = source.indexOf('const refreshTasks = useCallback(') - expect(start, 'refresh must be a single shared callback').toBeGreaterThan(-1) - expect(source.slice(start, source.indexOf('}, [', start))).toContain('repoListReload()') + // Regression: readiness is only useful if the renderer consults it. Without + // this the board renders "No project items" before the repo list arrives, and + // the memo assertion above stays green. + it('gates the empty state on readiness in the renderer', () => { + const gate = block( + "githubMode === 'project' ? (", + 'No project items' + ) + expect(gate, 'the empty state must sit behind the readiness spinner').toContain( + 'githubProjectTable && !githubProjectRepoSlugReady ? (' + ) + }) - const project = source.indexOf('const refreshGitHubProject = useCallback(') - expect(project, 'project refresh must be a single shared callback').toBeGreaterThan(-1) - const projectBody = source.slice(project, source.indexOf('}, [', project)) + it('re-reads the host and retries failed slug lookups on refresh', () => { + expect(block('const refreshTasks = useCallback(', '}, [')).toContain('repoListReload()') + + const projectBody = block('const refreshGitHubProject = useCallback(', '}, [') expect(projectBody).toContain('dropFailedGitHubRepoSlugEntries') expect(projectBody).toContain('refreshTasks()') expect(projectBody).toContain('loadGitHubProjectTable(') @@ -90,9 +103,7 @@ describe('mobile GitHub Project readiness and refresh', () => { // Regression: Expo reuses this screen for the next host, so an effect-based // reset runs a render too late and the previous host's rows show through. it('clears the other client-scoped caches during render', () => { - const start = source.indexOf('if (boundClient !== client) {') - expect(start, 'the client-scoped reset must run during render').toBeGreaterThan(-1) - const body = source.slice(start, source.indexOf('\n }', start)) + const body = block('if (boundClient !== client) {', '\n }') expect(body).toContain('setItems([])') expect(body).toContain('setGithubRepoSlugCache({})') expect(body, 'a ref write here would leak from an abandoned render').not.toContain('.current =') @@ -100,9 +111,7 @@ describe('mobile GitHub Project readiness and refresh', () => { // ...and the ref half belongs in the commit phase, for the same reason. it('resets the selection-hydration ref in the commit phase', () => { - const start = source.indexOf('clientRef.current = client') - expect(start, 'the client ref effect must exist').toBeGreaterThan(-1) - expect(source.slice(start, source.indexOf('}, [client])', start))).toContain( + expect(block('clientRef.current = client', '}, [client])')).toContain( 'repoSelectionHydratedRef.current = false' ) })