fix(mobile): load the repo list in GitHub Project mode (#12972)
This commit is contained in:
Jinwoo Hong
GitHub
parent
68b6e9f70d
commit
427f178a75
|
|
@ -80,6 +80,7 @@ import {
|
|||
import { shouldResolveHostedReviewStartPoint } from '../../../src/tasks/hosted-review-start-point'
|
||||
import { getLinkedWorkItemSuggestedName } from '../../../src/tasks/mobile-workspace-name'
|
||||
import {
|
||||
dropFailedGitHubRepoSlugEntries,
|
||||
filterGitHubProjectRowsForRepos,
|
||||
findRepoForGitHubProjectRepository,
|
||||
type GitHubRepoSlugCacheEntry
|
||||
|
|
@ -132,6 +133,9 @@ import {
|
|||
resolveVisibleTaskProvider,
|
||||
type TaskProvider
|
||||
} from '../../../src/tasks/mobile-task-providers'
|
||||
import { hasSettledHostRepoList } from '../../../src/tasks/host-repo-list'
|
||||
import { useHostRepoList } from '../../../src/tasks/use-host-repo-list'
|
||||
import { isHostedTaskRepo, reconcileRepoSelection } from '../../../src/tasks/hosted-repo-selection'
|
||||
import {
|
||||
extractLinearIssueReadItems,
|
||||
type LinearMobileIssue
|
||||
|
|
@ -1118,22 +1122,6 @@ async function mapWithConcurrency<T, R>(
|
|||
return results
|
||||
}
|
||||
|
||||
function isHostedTaskRepo(repo: RepoSummary): boolean {
|
||||
return repo.kind !== 'folder'
|
||||
}
|
||||
|
||||
function reconcileRepoSelection(
|
||||
repos: RepoSummary[],
|
||||
persisted: string[] | null | undefined
|
||||
): Set<string> {
|
||||
if (!persisted || persisted.length === 0) {
|
||||
return new Set()
|
||||
}
|
||||
const availableIds = new Set(repos.filter(isHostedTaskRepo).map((repo) => repo.id))
|
||||
const selected = persisted.filter((id) => availableIds.has(id))
|
||||
return selected.length === 0 ? new Set() : new Set(selected)
|
||||
}
|
||||
|
||||
function createLinearTask(issue: LinearIssue): TaskItem {
|
||||
return {
|
||||
key: `linear:${issue.workspaceId ?? 'workspace'}:${issue.id}`,
|
||||
|
|
@ -2084,10 +2072,22 @@ export default function MobileTasksScreen() {
|
|||
const reconnectAttempts = useReconnectAttempt(hostId)
|
||||
const lastConnectedAt = useLastConnectedAt(hostId)
|
||||
const clientRef = useRef<RpcClient | null>(null)
|
||||
const reposRef = useRef<RepoSummary[]>([])
|
||||
const loadGenerationRef = useRef(0)
|
||||
const taskResumeRef = useRef<TaskResumeState>({})
|
||||
const [repos, setRepos] = useState<RepoSummary[]>([])
|
||||
const repoList = useHostRepoList<RepoSummary>(
|
||||
client,
|
||||
client && connState === 'connected'
|
||||
? async () => {
|
||||
const response = await client.sendRequest('repo.list')
|
||||
if (!isSuccess(response)) {
|
||||
throw new Error(response.error.message)
|
||||
}
|
||||
return (response.result as { repos: RepoSummary[] }).repos
|
||||
}
|
||||
: null
|
||||
)
|
||||
const repos = repoList.state.repos
|
||||
const { ensureLoaded: repoListEnsureLoaded, reload: repoListReload } = repoList
|
||||
const [provider, setProvider] = useState<TaskProvider>('github')
|
||||
const [visibleProviders, setVisibleProviders] = useState<TaskProvider[]>(() =>
|
||||
normalizeVisibleTaskProviders(undefined)
|
||||
|
|
@ -2378,13 +2378,17 @@ export default function MobileTasksScreen() {
|
|||
) as RepoSummary | null,
|
||||
[activeGitHubProjectHost, githubRepoSlugCache, hostedRepos]
|
||||
)
|
||||
// Why: `every` is vacuously true on an empty repo list, so readiness has to ask
|
||||
// the resource whether that list is real yet. Otherwise the board renders
|
||||
// "No project items" for a board whose repos simply have not arrived.
|
||||
const githubProjectRepoSlugReady = useMemo(
|
||||
() =>
|
||||
hasSettledHostRepoList(repoList.state) &&
|
||||
hostedRepos.every((repo) => {
|
||||
const cached = githubRepoSlugCache[repo.id]
|
||||
return cached !== undefined && cached.path === repo.path
|
||||
}),
|
||||
[githubRepoSlugCache, hostedRepos]
|
||||
[githubRepoSlugCache, hostedRepos, repoList.state]
|
||||
)
|
||||
const visibleGitHubProjectRows = useMemo(
|
||||
() =>
|
||||
|
|
@ -2488,13 +2492,10 @@ export default function MobileTasksScreen() {
|
|||
throw new Error(response.error.message)
|
||||
}
|
||||
const result = response.result as GitHubOwnerRepo | null
|
||||
return {
|
||||
repoId: repo.id,
|
||||
path: repo.path,
|
||||
repository: result
|
||||
}
|
||||
return { repoId: repo.id, entry: { path: repo.path, repository: result } }
|
||||
} catch {
|
||||
return { repoId: repo.id, path: repo.path, repository: null }
|
||||
// Cached so readiness settles; `failed` marks it for retry on refresh.
|
||||
return { repoId: repo.id, entry: { path: repo.path, repository: null, failed: true } }
|
||||
}
|
||||
}).then((entries) => {
|
||||
if (cancelled) {
|
||||
|
|
@ -2503,7 +2504,7 @@ export default function MobileTasksScreen() {
|
|||
setGithubRepoSlugCache((current) => {
|
||||
const next = { ...current }
|
||||
for (const entry of entries) {
|
||||
next[entry.repoId] = { path: entry.path, repository: entry.repository }
|
||||
next[entry.repoId] = entry.entry
|
||||
}
|
||||
return next
|
||||
})
|
||||
|
|
@ -2621,8 +2622,31 @@ 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.
|
||||
const [boundClient, setBoundClient] = useState(client)
|
||||
if (boundClient !== client) {
|
||||
setBoundClient(client)
|
||||
setItems([])
|
||||
setGithubRepoSlugCache({})
|
||||
resetGitHubItemsState()
|
||||
}
|
||||
|
||||
useLayoutEffect(() => {
|
||||
clientRef.current = client
|
||||
// Why: ref writes belong in the commit phase. Doing this during render would
|
||||
// leak out of a concurrent render React later abandons.
|
||||
repoSelectionHydratedRef.current = false
|
||||
}, [client])
|
||||
|
||||
const persistTaskResumeState = useCallback(
|
||||
|
|
@ -2877,7 +2901,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)
|
||||
|
|
@ -3035,32 +3059,26 @@ export default function MobileTasksScreen() {
|
|||
setProvider(resolveVisibleTaskProvider(provider, visibleProviders))
|
||||
}, [provider, visibleProviders])
|
||||
|
||||
const loadRepos = useCallback(async (): Promise<RepoSummary[]> => {
|
||||
if (!client || connState !== 'connected') {
|
||||
return []
|
||||
// Selection follows the list rather than the fetch, so it reconciles the same
|
||||
// way no matter which caller triggered the load.
|
||||
useEffect(() => {
|
||||
if (repoList.state.status !== 'loaded') {
|
||||
return
|
||||
}
|
||||
const response = await client.sendRequest('repo.list')
|
||||
if (!isSuccess(response)) {
|
||||
throw new Error(response.error.message)
|
||||
}
|
||||
const result = response.result as { repos: RepoSummary[] }
|
||||
reposRef.current = result.repos
|
||||
setRepos(result.repos)
|
||||
if (!repoSelectionHydratedRef.current) {
|
||||
repoSelectionHydratedRef.current = true
|
||||
setSelectedRepoIds(reconcileRepoSelection(result.repos, defaultRepoSelectionRef.current))
|
||||
} else {
|
||||
setSelectedRepoIds((current) => {
|
||||
if (current.size === 0) {
|
||||
return current
|
||||
}
|
||||
const availableIds = new Set(result.repos.filter(isHostedTaskRepo).map((repo) => repo.id))
|
||||
const next = new Set([...current].filter((id) => availableIds.has(id)))
|
||||
return next.size === current.size ? current : next
|
||||
})
|
||||
setSelectedRepoIds(reconcileRepoSelection(repos, defaultRepoSelectionRef.current))
|
||||
return
|
||||
}
|
||||
return result.repos
|
||||
}, [client, connState])
|
||||
setSelectedRepoIds((current) => {
|
||||
if (current.size === 0) {
|
||||
return current
|
||||
}
|
||||
const availableIds = new Set(repos.filter(isHostedTaskRepo).map((repo) => repo.id))
|
||||
const next = new Set([...current].filter((id) => availableIds.has(id)))
|
||||
return next.size === current.size ? current : next
|
||||
})
|
||||
}, [repoList.state.status, repos])
|
||||
|
||||
const loadLinearContext = useCallback(async (): Promise<void> => {
|
||||
if (!client || connState !== 'connected' || !tasksSupported) {
|
||||
|
|
@ -3251,24 +3269,26 @@ export default function MobileTasksScreen() {
|
|||
}
|
||||
try {
|
||||
if (provider !== 'github' || githubMode !== 'items') {
|
||||
setGithubPages([])
|
||||
setGithubCurrentPage(0)
|
||||
setGithubTotalCount(null)
|
||||
setGithubSourceErrors([])
|
||||
setGithubSourceFallbacks([])
|
||||
}
|
||||
if (provider === 'github' && githubMode === 'project') {
|
||||
setItems([])
|
||||
return
|
||||
resetGitHubItemsState()
|
||||
}
|
||||
if (provider === 'linear' && !linearConnected) {
|
||||
setItems([])
|
||||
return
|
||||
}
|
||||
const currentRepos = reposRef.current.length > 0 ? reposRef.current : await loadRepos()
|
||||
// Why: Linear issues do not need the repo list, only the composer does, so
|
||||
// start the fetch either way but never make Linear wait on it.
|
||||
const repoListRequest = repoListEnsureLoaded()
|
||||
void repoListRequest.catch(() => {})
|
||||
const currentRepos = provider === 'linear' ? [] : await repoListRequest
|
||||
if (!isCurrent()) {
|
||||
return
|
||||
}
|
||||
// Why: project mode fetches no work items, but its rows are matched against
|
||||
// the repo list, so it must not return before loadRepos() has run.
|
||||
if (provider === 'github' && githubMode === 'project') {
|
||||
setItems([])
|
||||
return
|
||||
}
|
||||
if (provider === 'github' || provider === 'gitlab') {
|
||||
const supportedRepos = currentRepos.filter(isHostedTaskRepo)
|
||||
const queriedRepos =
|
||||
|
|
@ -3280,11 +3300,7 @@ export default function MobileTasksScreen() {
|
|||
return
|
||||
}
|
||||
setItems([])
|
||||
setGithubPages([])
|
||||
setGithubCurrentPage(0)
|
||||
setGithubTotalCount(null)
|
||||
setGithubSourceErrors([])
|
||||
setGithubSourceFallbacks([])
|
||||
resetGitHubItemsState()
|
||||
return
|
||||
}
|
||||
if (provider === 'github') {
|
||||
|
|
@ -3320,11 +3336,6 @@ export default function MobileTasksScreen() {
|
|||
return
|
||||
}
|
||||
if (provider === 'gitlab' && gitlabView === 'todos') {
|
||||
setGithubPages([])
|
||||
setGithubCurrentPage(0)
|
||||
setGithubTotalCount(null)
|
||||
setGithubSourceErrors([])
|
||||
setGithubSourceFallbacks([])
|
||||
const response = await requestClient.sendRequest('gitlab.todos', {
|
||||
repo: `id:${queriedRepos[0]!.id}`
|
||||
})
|
||||
|
|
@ -3341,11 +3352,6 @@ export default function MobileTasksScreen() {
|
|||
)
|
||||
return
|
||||
}
|
||||
setGithubPages([])
|
||||
setGithubCurrentPage(0)
|
||||
setGithubTotalCount(null)
|
||||
setGithubSourceErrors([])
|
||||
setGithubSourceFallbacks([])
|
||||
const results = await mapWithConcurrency(
|
||||
queriedRepos,
|
||||
GITHUB_REPO_CONCURRENCY,
|
||||
|
|
@ -3429,8 +3435,7 @@ export default function MobileTasksScreen() {
|
|||
return
|
||||
}
|
||||
setItems([])
|
||||
setGithubSourceErrors([])
|
||||
setGithubSourceFallbacks([])
|
||||
resetGitHubItemsState()
|
||||
setError(err instanceof Error ? err.message : 'Failed to load tasks')
|
||||
} finally {
|
||||
if (isCurrent()) {
|
||||
|
|
@ -3451,7 +3456,9 @@ export default function MobileTasksScreen() {
|
|||
linearConnected,
|
||||
linearFilter,
|
||||
linearOrderBy,
|
||||
loadRepos,
|
||||
// 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,
|
||||
selectedLinearWorkspaceId,
|
||||
|
|
@ -3890,6 +3897,19 @@ export default function MobileTasksScreen() {
|
|||
tasksSupported
|
||||
])
|
||||
|
||||
// Why: a refresh must re-read the host, not replay the cached list, or a repo
|
||||
// added since this screen mounted can never appear.
|
||||
const refreshTasks = useCallback(() => {
|
||||
void repoListReload().catch(() => {})
|
||||
void loadTasks({ silent: true })
|
||||
}, [loadTasks, repoListReload])
|
||||
|
||||
const refreshGitHubProject = useCallback(() => {
|
||||
setGithubRepoSlugCache(dropFailedGitHubRepoSlugEntries)
|
||||
refreshTasks()
|
||||
void loadGitHubProjectTable({ queryOverride: appliedGithubProjectSearch })
|
||||
}, [appliedGithubProjectSearch, loadGitHubProjectTable, refreshTasks])
|
||||
|
||||
useEffect(() => {
|
||||
if (!taskStateHydrated) {
|
||||
return
|
||||
|
|
@ -8120,22 +8140,15 @@ export default function MobileTasksScreen() {
|
|||
if (!isSuccess(response)) {
|
||||
throw new Error(response.error.message)
|
||||
}
|
||||
setRepos((current) =>
|
||||
current.map((candidate) =>
|
||||
candidate.id === repo.id
|
||||
? { ...candidate, issueSourcePreference: preference }
|
||||
: candidate
|
||||
)
|
||||
)
|
||||
reposRef.current = reposRef.current.map((candidate) =>
|
||||
candidate.id === repo.id ? { ...candidate, issueSourcePreference: preference } : candidate
|
||||
)
|
||||
// Why: the host owns issueSourcePreference, so re-read the list instead of
|
||||
// patching the cached copy and hoping the two stay in step.
|
||||
await repoListReload().catch(() => {})
|
||||
await loadTasks({ silent: true })
|
||||
} catch (err) {
|
||||
setError(err instanceof Error ? err.message : 'Failed to update issue source')
|
||||
}
|
||||
},
|
||||
[client, loadTasks, taskUiReady]
|
||||
[client, loadTasks, repoListReload, taskUiReady]
|
||||
)
|
||||
|
||||
const renderCommentComposer = (args: {
|
||||
|
|
@ -8691,10 +8704,10 @@ export default function MobileTasksScreen() {
|
|||
return
|
||||
}
|
||||
if (provider === 'github' && githubMode === 'project') {
|
||||
void loadGitHubProjectTable({ queryOverride: appliedGithubProjectSearch })
|
||||
refreshGitHubProject()
|
||||
return
|
||||
}
|
||||
void loadTasks({ silent: true })
|
||||
refreshTasks()
|
||||
}}
|
||||
>
|
||||
<RefreshCw size={16} color={taskUiReady ? colors.textSecondary : colors.textMuted} />
|
||||
|
|
@ -9278,9 +9291,7 @@ export default function MobileTasksScreen() {
|
|||
ItemSeparatorComponent={() => <View style={styles.separator} />}
|
||||
contentContainerStyle={[styles.list, { paddingBottom: spacing.lg + insets.bottom }]}
|
||||
refreshing={githubProjectLoading}
|
||||
onRefresh={() =>
|
||||
void loadGitHubProjectTable({ queryOverride: appliedGithubProjectSearch })
|
||||
}
|
||||
onRefresh={refreshGitHubProject}
|
||||
renderItem={({ item: entry }) => {
|
||||
if (entry.type === 'group') {
|
||||
return (
|
||||
|
|
@ -9485,7 +9496,7 @@ export default function MobileTasksScreen() {
|
|||
}
|
||||
contentContainerStyle={[styles.list, { paddingBottom: spacing.lg + insets.bottom }]}
|
||||
refreshing={refreshing}
|
||||
onRefresh={() => void loadTasks({ silent: true })}
|
||||
onRefresh={refreshTasks}
|
||||
renderItem={({ item: entry }) => {
|
||||
if (entry.type === 'section') {
|
||||
return (
|
||||
|
|
@ -9584,7 +9595,7 @@ export default function MobileTasksScreen() {
|
|||
}
|
||||
contentContainerStyle={[styles.list, { paddingBottom: spacing.lg + insets.bottom }]}
|
||||
refreshing={refreshing}
|
||||
onRefresh={() => void loadTasks({ silent: true })}
|
||||
onRefresh={refreshTasks}
|
||||
ListFooterComponent={
|
||||
provider === 'github' && githubMode === 'items' && githubCanShowPagination ? (
|
||||
<View style={styles.paginationFooter}>
|
||||
|
|
|
|||
|
|
@ -0,0 +1,118 @@
|
|||
import { readFileSync } from 'node:fs'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
|
||||
/** Wiring assertions only. The repo-list behaviour itself (caching, client
|
||||
* scoping, stale responses, retry) is covered behaviourally in
|
||||
* host-repo-list.test.ts and use-host-repo-list.test.tsx. What cannot be
|
||||
* reached from there is how this 15k-line route consumes the resource, so
|
||||
* these pin the consumption points the original bug lived in. */
|
||||
const source = readFileSync(new URL('../../app/h/[hostId]/tasks.tsx', import.meta.url), 'utf8')
|
||||
|
||||
/** 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.
|
||||
it('loads the repo list before project mode bails out', () => {
|
||||
const body = loadTasksBody()
|
||||
const repoLoad = body.indexOf('const repoListRequest = repoListEnsureLoaded()')
|
||||
const repoAwait = body.indexOf('await repoListRequest')
|
||||
const projectReturn = body.indexOf("provider === 'github' && githubMode === 'project'")
|
||||
expect(repoLoad, 'loadTasks must load the repo list').toBeGreaterThan(-1)
|
||||
expect(projectReturn, 'project mode must still short-circuit loadTasks').toBeGreaterThan(-1)
|
||||
expect(repoAwait, 'the repo list must be awaited, not just started').toBeGreaterThan(repoLoad)
|
||||
expect(projectReturn, 'project mode must bail out after the repo list load').toBeGreaterThan(
|
||||
repoAwait
|
||||
)
|
||||
})
|
||||
|
||||
it('still bails out before any work-item fetch in project mode', () => {
|
||||
const body = loadTasksBody()
|
||||
const projectReturn = body.indexOf("provider === 'github' && githubMode === 'project'")
|
||||
const workItemFetch = body.indexOf("provider === 'github' || provider === 'gitlab'")
|
||||
expect(workItemFetch, 'the hosted work-item fetch must still exist').toBeGreaterThan(-1)
|
||||
expect(workItemFetch, 'project mode must not reach the issue/PR fetch').toBeGreaterThan(
|
||||
projectReturn
|
||||
)
|
||||
})
|
||||
|
||||
it('keeps the repo list in the resource rather than local state', () => {
|
||||
expect(source).toContain('const repos = repoList.state.repos')
|
||||
expect(source, 'a second copy of the list would drift from the resource').not.toContain(
|
||||
'const [repos, setRepos]'
|
||||
)
|
||||
expect(source).not.toContain('reposRef')
|
||||
})
|
||||
})
|
||||
|
||||
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 body = block('const githubProjectRepoSlugReady = useMemo(', ' )')
|
||||
expect(body).toContain('hasSettledHostRepoList(repoList.state)')
|
||||
expect(body).toContain('[githubRepoSlugCache, hostedRepos, repoList.state]')
|
||||
})
|
||||
|
||||
// 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' ? (",
|
||||
'<Text style={styles.emptyText}>No project items</Text>'
|
||||
)
|
||||
expect(gate, 'the empty state must sit behind the readiness spinner').toContain(
|
||||
'githubProjectTable && !githubProjectRepoSlugReady ? ('
|
||||
)
|
||||
})
|
||||
|
||||
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(')
|
||||
})
|
||||
|
||||
it('routes every refresh control through those callbacks', () => {
|
||||
expect(source).toContain('onRefresh={refreshTasks}')
|
||||
expect(source).toContain('onRefresh={refreshGitHubProject}')
|
||||
expect(source, 'no refresh control may call loadTasks directly').not.toContain(
|
||||
'onRefresh={() => void loadTasks('
|
||||
)
|
||||
})
|
||||
|
||||
it('marks a failed slug lookup as retryable rather than resolved', () => {
|
||||
expect(source).toContain('repository: null, failed: true')
|
||||
})
|
||||
|
||||
// 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 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 =')
|
||||
})
|
||||
|
||||
// ...and the ref half belongs in the commit phase, for the same reason.
|
||||
it('resets the selection-hydration ref in the commit phase', () => {
|
||||
expect(block('clientRef.current = client', '}, [client])')).toContain(
|
||||
'repoSelectionHydratedRef.current = false'
|
||||
)
|
||||
})
|
||||
})
|
||||
|
|
@ -1,5 +1,6 @@
|
|||
import { describe, expect, it } from 'vitest'
|
||||
import {
|
||||
dropFailedGitHubRepoSlugEntries,
|
||||
filterGitHubProjectRowsForRepos,
|
||||
findRepoForGitHubProjectRepository,
|
||||
normalizeGitHubRepositorySlug
|
||||
|
|
@ -255,4 +256,38 @@ describe('GitHub project repo matching', () => {
|
|||
)
|
||||
).toBeNull()
|
||||
})
|
||||
|
||||
// Regression: a transient github.repoSlug error used to be cached as a
|
||||
// resolved "no repository", which filtered that repo's rows out forever.
|
||||
it('leaves a failed slug lookup matchable by the path fallback', () => {
|
||||
expect(
|
||||
findRepoForGitHubProjectRepository(
|
||||
'stablyai/orca',
|
||||
[{ id: 'repo-1', path: '/Users/me/stablyai/orca', displayName: 'orca' }],
|
||||
{ 'repo-1': { path: '/Users/me/stablyai/orca', repository: null, failed: true } }
|
||||
)
|
||||
).toEqual({ id: 'repo-1', path: '/Users/me/stablyai/orca', displayName: 'orca' })
|
||||
})
|
||||
})
|
||||
|
||||
describe('dropFailedGitHubRepoSlugEntries', () => {
|
||||
it('drops only the entries a retry could still resolve', () => {
|
||||
expect(
|
||||
dropFailedGitHubRepoSlugEntries({
|
||||
'repo-1': { path: '/a', repository: { owner: 'stablyai', repo: 'orca' } },
|
||||
'repo-2': { path: '/b', repository: null, failed: true },
|
||||
'repo-3': { path: '/c', repository: null }
|
||||
})
|
||||
).toEqual({
|
||||
'repo-1': { path: '/a', repository: { owner: 'stablyai', repo: 'orca' } },
|
||||
'repo-3': { path: '/c', repository: null }
|
||||
})
|
||||
})
|
||||
|
||||
// Why: the cache is a slug-effect dependency, so a fresh object on every
|
||||
// refresh would re-run the effect even when there is nothing to retry.
|
||||
it('returns the same object when nothing failed', () => {
|
||||
const cache = { 'repo-1': { path: '/a', repository: null } }
|
||||
expect(dropFailedGitHubRepoSlugEntries(cache)).toBe(cache)
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -15,6 +15,25 @@ export type GitHubProjectRepoMatch = {
|
|||
export type GitHubRepoSlugCacheEntry = {
|
||||
path: string
|
||||
repository: { owner: string; repo: string; host?: string } | null
|
||||
/** Resolution failed rather than resolving to "no repository". Cached so the
|
||||
* board stops waiting on it, but dropped on refresh so it is retried. */
|
||||
failed?: boolean
|
||||
}
|
||||
|
||||
/** Why: a transient `github.repoSlug` error would otherwise be cached forever as
|
||||
* an unresolved repo, filtering its rows out of every future board render. */
|
||||
export function dropFailedGitHubRepoSlugEntries(
|
||||
slugsByRepoId: Record<string, GitHubRepoSlugCacheEntry | undefined>
|
||||
): Record<string, GitHubRepoSlugCacheEntry | undefined> {
|
||||
const retryable = Object.entries(slugsByRepoId).filter(([, entry]) => entry?.failed === true)
|
||||
if (retryable.length === 0) {
|
||||
return slugsByRepoId
|
||||
}
|
||||
const next = { ...slugsByRepoId }
|
||||
for (const [repoId] of retryable) {
|
||||
delete next[repoId]
|
||||
}
|
||||
return next
|
||||
}
|
||||
|
||||
type CachedSlugState =
|
||||
|
|
|
|||
|
|
@ -0,0 +1,71 @@
|
|||
import { describe, expect, it } from 'vitest'
|
||||
import {
|
||||
hasSettledHostRepoList,
|
||||
hostRepoListReducer,
|
||||
initialHostRepoList,
|
||||
needsHostRepoListFetch,
|
||||
type HostRepoListState
|
||||
} from './host-repo-list'
|
||||
|
||||
type Repo = { id: string }
|
||||
|
||||
function reduce(
|
||||
state: HostRepoListState<Repo>,
|
||||
...actions: Parameters<typeof hostRepoListReducer<Repo>>[1][]
|
||||
): HostRepoListState<Repo> {
|
||||
return actions.reduce(hostRepoListReducer<Repo>, state)
|
||||
}
|
||||
|
||||
const idle = initialHostRepoList<Repo>()
|
||||
|
||||
describe('hostRepoListReducer', () => {
|
||||
it('keeps the last good list while a reload is in flight', () => {
|
||||
const loaded = reduce(idle, { type: 'requested' }, { type: 'resolved', repos: [{ id: 'a' }] })
|
||||
expect(reduce(loaded, { type: 'requested' })).toEqual({
|
||||
status: 'loading',
|
||||
repos: [{ id: 'a' }],
|
||||
error: ''
|
||||
})
|
||||
})
|
||||
|
||||
it('keeps the last good list when a reload fails, and records why', () => {
|
||||
const loaded = reduce(idle, { type: 'resolved', repos: [{ id: 'a' }] })
|
||||
expect(reduce(loaded, { type: 'requested' }, { type: 'failed', error: 'offline' })).toEqual({
|
||||
status: 'error',
|
||||
repos: [{ id: 'a' }],
|
||||
error: 'offline'
|
||||
})
|
||||
})
|
||||
|
||||
it('clears a previous host completely on reset', () => {
|
||||
const loaded = reduce(idle, { type: 'resolved', repos: [{ id: 'a' }] })
|
||||
expect(reduce(loaded, { type: 'reset' })).toEqual(idle)
|
||||
})
|
||||
|
||||
it('returns the identical state when a reset or repeat request changes nothing', () => {
|
||||
expect(hostRepoListReducer<Repo>(idle, { type: 'reset' })).toBe(idle)
|
||||
const loading = reduce(idle, { type: 'requested' })
|
||||
expect(hostRepoListReducer<Repo>(loading, { type: 'requested' })).toBe(loading)
|
||||
})
|
||||
})
|
||||
|
||||
describe('host repo list readiness', () => {
|
||||
// Regression (#12966): an empty list read as "this host has no repos", so the
|
||||
// Project board filtered every row against [] and rendered "No project items".
|
||||
it('does not treat an unfetched or in-flight list as settled', () => {
|
||||
expect(hasSettledHostRepoList(idle)).toBe(false)
|
||||
expect(hasSettledHostRepoList(reduce(idle, { type: 'requested' }))).toBe(false)
|
||||
})
|
||||
|
||||
it('settles on success and on failure, so the UI never waits forever', () => {
|
||||
expect(hasSettledHostRepoList(reduce(idle, { type: 'resolved', repos: [] }))).toBe(true)
|
||||
expect(hasSettledHostRepoList(reduce(idle, { type: 'failed', error: 'nope' }))).toBe(true)
|
||||
})
|
||||
|
||||
it('re-fetches after a failure but not after an empty success', () => {
|
||||
expect(needsHostRepoListFetch(idle)).toBe(true)
|
||||
expect(needsHostRepoListFetch(reduce(idle, { type: 'failed', error: 'nope' }))).toBe(true)
|
||||
// A host with no repos is a real answer; asking again on every load is waste.
|
||||
expect(needsHostRepoListFetch(reduce(idle, { type: 'resolved', repos: [] }))).toBe(false)
|
||||
})
|
||||
})
|
||||
|
|
@ -0,0 +1,63 @@
|
|||
/** The host repo list as a client-scoped resource.
|
||||
*
|
||||
* Why this is a state machine rather than a `repos` array plus a "did we fetch
|
||||
* it" flag: the Tasks screen is reused across hosts and `forceReconnect` swaps
|
||||
* the client underneath it, so a bare array cannot say *which* client answered,
|
||||
* and an empty array cannot say whether the host has no repos or was never
|
||||
* asked. Both ambiguities silently filtered every GitHub Project row (#12966).
|
||||
*/
|
||||
|
||||
export type HostRepoListStatus = 'idle' | 'loading' | 'loaded' | 'error'
|
||||
|
||||
export type HostRepoListState<Repo> = {
|
||||
status: HostRepoListStatus
|
||||
/** Last successful response. Retained across a reload so refreshing does not
|
||||
* blank the list, and cleared only by `reset`. */
|
||||
repos: Repo[]
|
||||
error: string
|
||||
}
|
||||
|
||||
export type HostRepoListAction<Repo> =
|
||||
| { type: 'reset' }
|
||||
| { type: 'requested' }
|
||||
| { type: 'resolved'; repos: Repo[] }
|
||||
| { type: 'failed'; error: string }
|
||||
|
||||
export const IDLE_HOST_REPO_LIST: HostRepoListState<never> = {
|
||||
status: 'idle',
|
||||
repos: [],
|
||||
error: ''
|
||||
}
|
||||
|
||||
export function initialHostRepoList<Repo>(): HostRepoListState<Repo> {
|
||||
return IDLE_HOST_REPO_LIST as HostRepoListState<Repo>
|
||||
}
|
||||
|
||||
export function hostRepoListReducer<Repo>(
|
||||
state: HostRepoListState<Repo>,
|
||||
action: HostRepoListAction<Repo>
|
||||
): HostRepoListState<Repo> {
|
||||
switch (action.type) {
|
||||
case 'reset':
|
||||
return state.status === 'idle' ? state : initialHostRepoList<Repo>()
|
||||
case 'requested':
|
||||
return state.status === 'loading' ? state : { ...state, status: 'loading', error: '' }
|
||||
case 'resolved':
|
||||
return { status: 'loaded', repos: action.repos, error: '' }
|
||||
case 'failed':
|
||||
return { ...state, status: 'error', error: action.error }
|
||||
}
|
||||
}
|
||||
|
||||
/** Whether a fetch has finished, successfully or not. Callers gate their empty
|
||||
* state on this: "no repos yet" must not render as "this host has no repos". */
|
||||
export function hasSettledHostRepoList(state: HostRepoListState<unknown>): boolean {
|
||||
return state.status === 'loaded' || state.status === 'error'
|
||||
}
|
||||
|
||||
/** Whether `loadTasks` still needs to fetch before it can trust `repos`. An
|
||||
* in-flight request does not count as fetched, so a caller that needs the list
|
||||
* now awaits its own request rather than reading a stale array. */
|
||||
export function needsHostRepoListFetch(state: HostRepoListState<unknown>): boolean {
|
||||
return state.status !== 'loaded'
|
||||
}
|
||||
|
|
@ -0,0 +1,36 @@
|
|||
import { describe, expect, it } from 'vitest'
|
||||
import { isHostedTaskRepo, reconcileRepoSelection } from './hosted-repo-selection'
|
||||
|
||||
const repos = [
|
||||
{ id: 'a', kind: 'worktree' },
|
||||
{ id: 'b', kind: 'worktree' },
|
||||
{ id: 'folder', kind: 'folder' }
|
||||
]
|
||||
|
||||
describe('isHostedTaskRepo', () => {
|
||||
it('excludes folder workspaces and keeps everything else', () => {
|
||||
expect(repos.filter(isHostedTaskRepo).map((repo) => repo.id)).toEqual(['a', 'b'])
|
||||
expect(isHostedTaskRepo({ id: 'no-kind' })).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
describe('reconcileRepoSelection', () => {
|
||||
it('treats an absent or empty persisted selection as "all repos"', () => {
|
||||
expect(reconcileRepoSelection(repos, null)).toEqual(new Set())
|
||||
expect(reconcileRepoSelection(repos, [])).toEqual(new Set())
|
||||
})
|
||||
|
||||
it('drops ids this host no longer has', () => {
|
||||
expect(reconcileRepoSelection(repos, ['a', 'gone'])).toEqual(new Set(['a']))
|
||||
})
|
||||
|
||||
// An empty set means "all repos", so a selection that survives nothing widens
|
||||
// back out rather than filtering every row away.
|
||||
it('widens back to all repos when nothing in the selection survives', () => {
|
||||
expect(reconcileRepoSelection(repos, ['gone'])).toEqual(new Set())
|
||||
})
|
||||
|
||||
it('never selects a folder workspace', () => {
|
||||
expect(reconcileRepoSelection(repos, ['folder'])).toEqual(new Set())
|
||||
})
|
||||
})
|
||||
|
|
@ -0,0 +1,27 @@
|
|||
/** Structural shape both helpers need. Kept minimal so the Tasks route's own
|
||||
* RepoSummary satisfies it without importing anything back. */
|
||||
export type HostedRepoCandidate = {
|
||||
id: string
|
||||
kind?: string | null
|
||||
}
|
||||
|
||||
/** Folder workspaces have no hosted provider behind them, so they cannot back a
|
||||
* GitHub/GitLab task or a Project board row. */
|
||||
export function isHostedTaskRepo(repo: HostedRepoCandidate): boolean {
|
||||
return repo.kind !== 'folder'
|
||||
}
|
||||
|
||||
/** Narrows a persisted repo-id selection to what this host actually has.
|
||||
* An empty result means "all repos", which is also what an empty persisted
|
||||
* selection means, so a selection whose repos have all disappeared widens back
|
||||
* out rather than silently matching nothing. */
|
||||
export function reconcileRepoSelection(
|
||||
repos: readonly HostedRepoCandidate[],
|
||||
persisted: readonly string[] | null | undefined
|
||||
): Set<string> {
|
||||
if (!persisted || persisted.length === 0) {
|
||||
return new Set()
|
||||
}
|
||||
const availableIds = new Set(repos.filter(isHostedTaskRepo).map((repo) => repo.id))
|
||||
return new Set(persisted.filter((id) => availableIds.has(id)))
|
||||
}
|
||||
|
|
@ -0,0 +1,253 @@
|
|||
import { createElement } from 'react'
|
||||
import { act, create } from 'react-test-renderer'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { useHostRepoList, type HostRepoListResource } from './use-host-repo-list'
|
||||
|
||||
type Repo = { id: string }
|
||||
|
||||
/** Renders the hook and exposes the latest resource plus a deferred fetcher so a
|
||||
* test can decide exactly when (and for which client) a response lands. */
|
||||
function mountResource() {
|
||||
const pending: { resolve: (repos: Repo[]) => void; reject: (err: Error) => void }[] = []
|
||||
let latest: HostRepoListResource<Repo> | null = null
|
||||
let calls = 0
|
||||
|
||||
function Probe({ clientKey, connected = true }: { clientKey: unknown; connected?: boolean }) {
|
||||
latest = useHostRepoList<Repo>(
|
||||
clientKey,
|
||||
connected
|
||||
? () => {
|
||||
calls += 1
|
||||
return new Promise<Repo[]>((resolve, reject) => pending.push({ resolve, reject }))
|
||||
}
|
||||
: null
|
||||
)
|
||||
return null
|
||||
}
|
||||
|
||||
let renderer!: ReturnType<typeof create>
|
||||
act(() => {
|
||||
renderer = create(createElement(Probe, { clientKey: 'client-a' }))
|
||||
})
|
||||
return {
|
||||
get resource(): HostRepoListResource<Repo> {
|
||||
if (!latest) {
|
||||
throw new Error('probe never rendered')
|
||||
}
|
||||
return latest
|
||||
},
|
||||
get callCount(): number {
|
||||
return calls
|
||||
},
|
||||
pending,
|
||||
rerender() {
|
||||
act(() => {
|
||||
renderer.update(createElement(Probe, { clientKey: 'client-a' }))
|
||||
})
|
||||
},
|
||||
rebind(clientKey: unknown, connected = true) {
|
||||
act(() => {
|
||||
renderer.update(createElement(Probe, { clientKey, connected }))
|
||||
})
|
||||
},
|
||||
async settle(index: number, repos: Repo[]) {
|
||||
await act(async () => {
|
||||
pending[index]!.resolve(repos)
|
||||
await Promise.resolve()
|
||||
})
|
||||
},
|
||||
async fail(index: number, message: string) {
|
||||
await act(async () => {
|
||||
pending[index]!.reject(new Error(message))
|
||||
await Promise.resolve()
|
||||
})
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
describe('useHostRepoList', () => {
|
||||
it('fetches once and then serves the cached list', async () => {
|
||||
const probe = mountResource()
|
||||
let first: Repo[] = []
|
||||
await act(async () => {
|
||||
void probe.resource.ensureLoaded().then((repos) => {
|
||||
first = repos
|
||||
})
|
||||
})
|
||||
await probe.settle(0, [{ id: 'a' }])
|
||||
expect(first).toEqual([{ id: 'a' }])
|
||||
expect(probe.resource.state.status).toBe('loaded')
|
||||
|
||||
await act(async () => {
|
||||
await probe.resource.ensureLoaded()
|
||||
})
|
||||
expect(probe.callCount).toBe(1)
|
||||
})
|
||||
|
||||
// Regression (#12966): a host with no repos must not be re-fetched forever.
|
||||
it('treats an empty response as a real answer', async () => {
|
||||
const probe = mountResource()
|
||||
await act(async () => {
|
||||
void probe.resource.ensureLoaded()
|
||||
})
|
||||
await probe.settle(0, [])
|
||||
await act(async () => {
|
||||
await probe.resource.ensureLoaded()
|
||||
})
|
||||
expect(probe.callCount).toBe(1)
|
||||
expect(probe.resource.state.status).toBe('loaded')
|
||||
})
|
||||
|
||||
it('collapses concurrent callers into one request', async () => {
|
||||
const probe = mountResource()
|
||||
await act(async () => {
|
||||
void probe.resource.ensureLoaded()
|
||||
void probe.resource.ensureLoaded()
|
||||
})
|
||||
expect(probe.callCount).toBe(1)
|
||||
})
|
||||
|
||||
it('retries after a failure instead of caching it', async () => {
|
||||
const probe = mountResource()
|
||||
await act(async () => {
|
||||
void probe.resource.ensureLoaded().catch(() => {})
|
||||
})
|
||||
await probe.fail(0, 'offline')
|
||||
expect(probe.resource.state.status).toBe('error')
|
||||
expect(probe.resource.state.error).toBe('offline')
|
||||
await act(async () => {
|
||||
void probe.resource.ensureLoaded().catch(() => {})
|
||||
})
|
||||
expect(probe.callCount).toBe(2)
|
||||
})
|
||||
|
||||
// Regression: the screen is reused across hosts, so the previous host's list
|
||||
// must never answer for the new one.
|
||||
it('drops the cached list as soon as the client changes', async () => {
|
||||
const probe = mountResource()
|
||||
await act(async () => {
|
||||
void probe.resource.ensureLoaded()
|
||||
})
|
||||
await probe.settle(0, [{ id: 'from-host-a' }])
|
||||
expect(probe.resource.state.repos).toEqual([{ id: 'from-host-a' }])
|
||||
|
||||
probe.rebind('client-b')
|
||||
expect(probe.resource.state.status).toBe('idle')
|
||||
expect(probe.resource.state.repos).toEqual([])
|
||||
|
||||
await act(async () => {
|
||||
void probe.resource.ensureLoaded()
|
||||
})
|
||||
expect(probe.callCount).toBe(2)
|
||||
})
|
||||
|
||||
// Regression: navigation keeps the old client alive, so its slow repo.list can
|
||||
// still resolve after the swap. It must not become the new host's answer.
|
||||
it('discards a response that arrives after the client changed', async () => {
|
||||
const probe = mountResource()
|
||||
await act(async () => {
|
||||
void probe.resource.ensureLoaded()
|
||||
})
|
||||
probe.rebind('client-b')
|
||||
await probe.settle(0, [{ id: 'from-host-a' }])
|
||||
|
||||
expect(probe.resource.state.status).toBe('idle')
|
||||
expect(probe.resource.state.repos).toEqual([])
|
||||
|
||||
await act(async () => {
|
||||
void probe.resource.ensureLoaded()
|
||||
})
|
||||
await probe.settle(1, [{ id: 'from-host-b' }])
|
||||
expect(probe.resource.state.repos).toEqual([{ id: 'from-host-b' }])
|
||||
})
|
||||
|
||||
// Regression: A -> B -> A reuses the same client object, so a client-key match
|
||||
// alone let a stale request for A overwrite a newer result for A.
|
||||
it('discards a stale response after returning to the original client', async () => {
|
||||
const probe = mountResource()
|
||||
await act(async () => {
|
||||
void probe.resource.ensureLoaded()
|
||||
})
|
||||
probe.rebind('client-b')
|
||||
probe.rebind('client-a')
|
||||
await act(async () => {
|
||||
void probe.resource.ensureLoaded()
|
||||
})
|
||||
await probe.settle(1, [{ id: 'new-a' }])
|
||||
expect(probe.resource.state.repos).toEqual([{ id: 'new-a' }])
|
||||
|
||||
await probe.settle(0, [{ id: 'stale-a' }])
|
||||
expect(probe.resource.state.repos).toEqual([{ id: 'new-a' }])
|
||||
})
|
||||
|
||||
// Regression: a refresh calls reload() and loadTasks() in the same event, and
|
||||
// React has not rendered the `requested` dispatch yet, so ensureLoaded saw
|
||||
// `loaded` and returned the very list the reload was replacing.
|
||||
it('joins an in-flight reload instead of serving the list it will replace', async () => {
|
||||
const probe = mountResource()
|
||||
await act(async () => {
|
||||
void probe.resource.ensureLoaded()
|
||||
})
|
||||
await probe.settle(0, [{ id: 'stale' }])
|
||||
|
||||
let joined: Repo[] = []
|
||||
await act(async () => {
|
||||
void probe.resource.reload()
|
||||
void probe.resource.ensureLoaded().then((repos) => {
|
||||
joined = repos
|
||||
})
|
||||
})
|
||||
expect(probe.callCount).toBe(2)
|
||||
await probe.settle(1, [{ id: 'fresh' }])
|
||||
expect(joined).toEqual([{ id: 'fresh' }])
|
||||
})
|
||||
|
||||
it('stays idle with no connection instead of caching an empty answer', async () => {
|
||||
const probe = mountResource()
|
||||
probe.rebind('client-a', false)
|
||||
await act(async () => {
|
||||
expect(await probe.resource.ensureLoaded()).toEqual([])
|
||||
})
|
||||
expect(probe.callCount).toBe(0)
|
||||
expect(probe.resource.state.status).toBe('idle')
|
||||
})
|
||||
|
||||
// Regression: the resource used to be a fresh object per render, so consumers
|
||||
// that held it in a dependency array re-created their callbacks every render
|
||||
// and their effects re-fired forever ("Maximum update depth exceeded").
|
||||
it('keeps a stable identity across renders that change nothing', async () => {
|
||||
const probe = mountResource()
|
||||
const first = probe.resource
|
||||
probe.rerender()
|
||||
expect(probe.resource).toBe(first)
|
||||
expect(probe.resource.ensureLoaded).toBe(first.ensureLoaded)
|
||||
expect(probe.resource.reload).toBe(first.reload)
|
||||
|
||||
await act(async () => {
|
||||
void probe.resource.ensureLoaded()
|
||||
})
|
||||
await probe.settle(0, [{ id: 'a' }])
|
||||
// A load changes state, so the object may differ, but the callbacks must not.
|
||||
expect(probe.resource.ensureLoaded).toBe(first.ensureLoaded)
|
||||
expect(probe.resource.reload).toBe(first.reload)
|
||||
const loaded = probe.resource
|
||||
probe.rerender()
|
||||
expect(probe.resource).toBe(loaded)
|
||||
})
|
||||
|
||||
it('re-reads the host on an explicit reload', async () => {
|
||||
const probe = mountResource()
|
||||
await act(async () => {
|
||||
void probe.resource.ensureLoaded()
|
||||
})
|
||||
await probe.settle(0, [{ id: 'a' }])
|
||||
await act(async () => {
|
||||
void probe.resource.reload()
|
||||
})
|
||||
expect(probe.callCount).toBe(2)
|
||||
// The previous list stays visible while the reload is in flight.
|
||||
expect(probe.resource.state.repos).toEqual([{ id: 'a' }])
|
||||
await probe.settle(1, [{ id: 'a' }, { id: 'b' }])
|
||||
expect(probe.resource.state.repos).toEqual([{ id: 'a' }, { id: 'b' }])
|
||||
})
|
||||
})
|
||||
|
|
@ -0,0 +1,129 @@
|
|||
import {
|
||||
useCallback,
|
||||
useEffect,
|
||||
useLayoutEffect,
|
||||
useMemo,
|
||||
useReducer,
|
||||
useRef,
|
||||
useState
|
||||
} from 'react'
|
||||
import {
|
||||
hostRepoListReducer,
|
||||
initialHostRepoList,
|
||||
needsHostRepoListFetch,
|
||||
type HostRepoListState
|
||||
} from './host-repo-list'
|
||||
|
||||
export type HostRepoListResource<Repo> = {
|
||||
state: HostRepoListState<Repo>
|
||||
/** Cached repos when this client already answered, otherwise one fetch. */
|
||||
ensureLoaded: () => Promise<Repo[]>
|
||||
/** Discards the cache so an explicit refresh re-reads the host. */
|
||||
reload: () => Promise<Repo[]>
|
||||
}
|
||||
|
||||
/** Binds the repo list to `clientKey`. Pass `fetchRepos: null` while there is no
|
||||
* usable connection; the resource then stays idle instead of caching an empty
|
||||
* answer that a later client would inherit. */
|
||||
export function useHostRepoList<Repo>(
|
||||
clientKey: unknown,
|
||||
fetchRepos: (() => Promise<Repo[]>) | null
|
||||
): HostRepoListResource<Repo> {
|
||||
const [state, dispatch] = useReducer(
|
||||
hostRepoListReducer<Repo>,
|
||||
undefined,
|
||||
initialHostRepoList<Repo>
|
||||
)
|
||||
const boundKeyRef = useRef(clientKey)
|
||||
const reposRef = useRef<Repo[]>([])
|
||||
const inFlightRef = useRef<Promise<Repo[]> | null>(null)
|
||||
const requestIdRef = useRef(0)
|
||||
const fetchRef = useRef(fetchRepos)
|
||||
const stateRef = useRef(state)
|
||||
|
||||
// Why: discarding the previous client's list has to happen before anything can
|
||||
// read it, and Expo reuses this screen for the next host. A state update during
|
||||
// render is the supported way to do that; a ref write here is not, because a
|
||||
// concurrent render React abandons would still have mutated it.
|
||||
const [boundKey, setBoundKey] = useState(clientKey)
|
||||
if (boundKey !== clientKey) {
|
||||
setBoundKey(clientKey)
|
||||
dispatch({ type: 'reset' })
|
||||
}
|
||||
|
||||
// Why: the async request guard reads these, so they are written in the commit
|
||||
// phase - the earliest point a render is known to have survived.
|
||||
useEffect(() => {
|
||||
fetchRef.current = fetchRepos
|
||||
stateRef.current = state
|
||||
})
|
||||
|
||||
useLayoutEffect(() => {
|
||||
if (boundKeyRef.current === clientKey) {
|
||||
return
|
||||
}
|
||||
boundKeyRef.current = clientKey
|
||||
reposRef.current = []
|
||||
inFlightRef.current = null
|
||||
// Retires every request issued for the previous client.
|
||||
requestIdRef.current += 1
|
||||
}, [clientKey])
|
||||
|
||||
// Why: consumers hold these in dependency arrays, so they must never change
|
||||
// identity. Everything they read lives in refs, so they never need to.
|
||||
const reload = useCallback(async (): Promise<Repo[]> => {
|
||||
const fetchNow = fetchRef.current
|
||||
if (!fetchNow) {
|
||||
return []
|
||||
}
|
||||
if (inFlightRef.current) {
|
||||
return inFlightRef.current
|
||||
}
|
||||
// Why: only the request issued for the currently bound client may commit.
|
||||
// A slow response from the previous host would otherwise land afterwards
|
||||
// and pin its repos as this host's authoritative list.
|
||||
const requestKey = boundKeyRef.current
|
||||
const requestId = requestIdRef.current + 1
|
||||
requestIdRef.current = requestId
|
||||
const request = (async (): Promise<Repo[]> => {
|
||||
dispatch({ type: 'requested' })
|
||||
try {
|
||||
const repos = await fetchNow()
|
||||
// Why: A -> B -> A reuses the same client, so matching the key alone
|
||||
// would let a stale request for A overwrite a newer result for A.
|
||||
if (boundKeyRef.current !== requestKey || requestIdRef.current !== requestId) {
|
||||
return []
|
||||
}
|
||||
reposRef.current = repos
|
||||
dispatch({ type: 'resolved', repos })
|
||||
return repos
|
||||
} catch (err) {
|
||||
if (boundKeyRef.current === requestKey && requestIdRef.current === requestId) {
|
||||
dispatch({
|
||||
type: 'failed',
|
||||
error: err instanceof Error ? err.message : 'Unknown error'
|
||||
})
|
||||
}
|
||||
throw err
|
||||
} finally {
|
||||
if (requestIdRef.current === requestId) {
|
||||
inFlightRef.current = null
|
||||
}
|
||||
}
|
||||
})()
|
||||
inFlightRef.current = request
|
||||
return request
|
||||
}, [])
|
||||
|
||||
// Why: within one event React has not rendered the `requested` dispatch yet, so
|
||||
// the status still reads `loaded`. Join the in-flight request instead of
|
||||
// handing back the list it is about to replace.
|
||||
const ensureLoaded = useCallback(
|
||||
(): Promise<Repo[]> =>
|
||||
inFlightRef.current ??
|
||||
(needsHostRepoListFetch(stateRef.current) ? reload() : Promise.resolve(reposRef.current)),
|
||||
[reload]
|
||||
)
|
||||
|
||||
return useMemo(() => ({ state, ensureLoaded, reload }), [ensureLoaded, reload, state])
|
||||
}
|
||||
Loading…
Reference in New Issue