Extract GitHub task search commit debouncing into a hook (#13112)
* Extract GitHub task search commit debouncing into a hook Move debounce logic from TaskPage into useGitHubTaskSearchCommit to prevent excessive GitHub API calls on every keystroke. Uses a 750ms idle window before committing search values. Add tests for the new hook. * Keep task rows visible while typing search query Removed premature row-hiding logic from the search input handler that was triggering before debounced queries fire. The handler now only updates the input state; debouncing and query timing are handled by a dedicated hook. Added e2e test verifying search idles before fetching and Enter doesn't double-fetch. * Test that GitHub task search commits cancel on disable and unmount Verify the useGitHubTaskSearchCommit hook properly cleans up pending commits when disabled or when the component unmounts. This prevents unnecessary GitHub API calls during normal user interaction. Also fix e2e test instrumentation to find the active repo through the worktree relationship rather than assuming the first repo with a path.
This commit is contained in:
parent
597c84f36c
commit
4c26ef626c
|
|
@ -431,6 +431,7 @@ import {
|
|||
type LinearOrderBy,
|
||||
type LinearViewMode
|
||||
} from '@/components/task-page-localized-options'
|
||||
import { useGitHubTaskSearchCommit } from '@/components/use-github-task-search-commit'
|
||||
|
||||
function isGitLabMRFilter(value: GitLabTaskFilter | GitLabIssueFilter): value is GitLabTaskFilter {
|
||||
return value === 'opened' || value === 'merged' || value === 'closed' || value === 'all'
|
||||
|
|
@ -6533,19 +6534,21 @@ export default function TaskPage(): React.JSX.Element {
|
|||
]
|
||||
)
|
||||
|
||||
useEffect(() => {
|
||||
if (!taskResumeApplied) {
|
||||
return
|
||||
}
|
||||
const timeout = window.setTimeout(() => {
|
||||
const scoped = scopeGitHubTaskSearch(taskSearchInput, activeGithubTaskKind)
|
||||
const commitTaskSearch = useCallback(
|
||||
(value: string): void => {
|
||||
const scoped = scopeGitHubTaskSearch(value, activeGithubTaskKind)
|
||||
if (scoped !== appliedTaskSearch) {
|
||||
setTasksFiltering(true)
|
||||
}
|
||||
setAppliedTaskSearch(scoped)
|
||||
}, TASK_SEARCH_DEBOUNCE_MS)
|
||||
return () => window.clearTimeout(timeout)
|
||||
}, [activeGithubTaskKind, appliedTaskSearch, taskSearchInput, taskResumeApplied])
|
||||
},
|
||||
[activeGithubTaskKind, appliedTaskSearch]
|
||||
)
|
||||
useGitHubTaskSearchCommit({
|
||||
enabled: taskResumeApplied,
|
||||
onCommit: commitTaskSearch,
|
||||
value: taskSearchInput
|
||||
})
|
||||
|
||||
useEffect(() => {
|
||||
if (!taskResumeApplied) {
|
||||
|
|
@ -7140,17 +7143,11 @@ export default function TaskPage(): React.JSX.Element {
|
|||
setTaskRefreshNonce((current) => current + 1)
|
||||
}, [activeGithubTaskKind, setTaskResumeState, taskSearchInput])
|
||||
|
||||
const handleTaskSearchChange = useCallback(
|
||||
(event: React.ChangeEvent<HTMLInputElement>): void => {
|
||||
const next = event.target.value
|
||||
const scoped = scopeGitHubTaskSearch(next, activeGithubTaskKind)
|
||||
setTaskSearchInput(next)
|
||||
setActiveTaskPreset(null)
|
||||
// Why: visible rows are keyed by appliedTaskSearch, not the draft input; hide stale rows once the draft changes the query.
|
||||
setTasksFiltering(scoped !== appliedTaskSearch)
|
||||
},
|
||||
[activeGithubTaskKind, appliedTaskSearch]
|
||||
)
|
||||
const handleTaskSearchChange = useCallback((event: React.ChangeEvent<HTMLInputElement>): void => {
|
||||
const next = event.target.value
|
||||
setTaskSearchInput(next)
|
||||
setActiveTaskPreset(null)
|
||||
}, [])
|
||||
|
||||
const handleSetDefaultTaskPreset = useCallback(
|
||||
(presetId: TaskViewPresetId): void => {
|
||||
|
|
|
|||
|
|
@ -0,0 +1,59 @@
|
|||
// @vitest-environment happy-dom
|
||||
|
||||
import { act, renderHook } from '@testing-library/react'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import {
|
||||
GITHUB_TASK_SEARCH_IDLE_MS,
|
||||
useGitHubTaskSearchCommit
|
||||
} from './use-github-task-search-commit'
|
||||
|
||||
describe('useGitHubTaskSearchCommit', () => {
|
||||
beforeEach(() => vi.useFakeTimers())
|
||||
afterEach(() => vi.useRealTimers())
|
||||
|
||||
it('commits only the final value after a full idle window', () => {
|
||||
const onCommit = vi.fn()
|
||||
const view = renderHook(
|
||||
({ value }) => useGitHubTaskSearchCommit({ enabled: true, onCommit, value }),
|
||||
{ initialProps: { value: 'r' } }
|
||||
)
|
||||
|
||||
act(() => vi.advanceTimersByTime(400))
|
||||
view.rerender({ value: 'ra' })
|
||||
act(() => vi.advanceTimersByTime(400))
|
||||
view.rerender({ value: 'rate' })
|
||||
act(() => vi.advanceTimersByTime(GITHUB_TASK_SEARCH_IDLE_MS - 1))
|
||||
|
||||
expect(onCommit).not.toHaveBeenCalled()
|
||||
|
||||
act(() => vi.advanceTimersByTime(1))
|
||||
expect(onCommit).toHaveBeenCalledOnce()
|
||||
expect(onCommit).toHaveBeenCalledWith('rate')
|
||||
})
|
||||
|
||||
it('cancels a pending commit when disabled', () => {
|
||||
const onCommit = vi.fn()
|
||||
const view = renderHook(
|
||||
({ enabled }) => useGitHubTaskSearchCommit({ enabled, onCommit, value: 'rate' }),
|
||||
{ initialProps: { enabled: true } }
|
||||
)
|
||||
|
||||
act(() => vi.advanceTimersByTime(400))
|
||||
view.rerender({ enabled: false })
|
||||
|
||||
act(() => vi.runAllTimers())
|
||||
expect(onCommit).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('cancels a pending commit on unmount', () => {
|
||||
const onCommit = vi.fn()
|
||||
const view = renderHook(() =>
|
||||
useGitHubTaskSearchCommit({ enabled: true, onCommit, value: 'rate' })
|
||||
)
|
||||
|
||||
view.unmount()
|
||||
act(() => vi.runAllTimers())
|
||||
|
||||
expect(onCommit).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
|
@ -0,0 +1,30 @@
|
|||
import { useEffect, useRef } from 'react'
|
||||
|
||||
// Remote GitHub fan-out needs a longer idle window; Enter still commits immediately.
|
||||
export const GITHUB_TASK_SEARCH_IDLE_MS = 750
|
||||
|
||||
type GitHubTaskSearchCommitOptions = {
|
||||
enabled: boolean
|
||||
onCommit: (value: string) => void
|
||||
value: string
|
||||
}
|
||||
|
||||
export function useGitHubTaskSearchCommit({
|
||||
enabled,
|
||||
onCommit,
|
||||
value
|
||||
}: GitHubTaskSearchCommitOptions): void {
|
||||
const onCommitRef = useRef(onCommit)
|
||||
// Keep latest callback without restarting the idle timer when identity changes.
|
||||
useEffect(() => {
|
||||
onCommitRef.current = onCommit
|
||||
}, [onCommit])
|
||||
|
||||
useEffect(() => {
|
||||
if (!enabled) {
|
||||
return
|
||||
}
|
||||
const timeout = window.setTimeout(() => onCommitRef.current(value), GITHUB_TASK_SEARCH_IDLE_MS)
|
||||
return () => window.clearTimeout(timeout)
|
||||
}, [enabled, value])
|
||||
}
|
||||
|
|
@ -13,6 +13,11 @@ type RenderedTaskSource = {
|
|||
active: boolean
|
||||
}
|
||||
|
||||
type TaskSearchRequestProbe = {
|
||||
countQueries: string[]
|
||||
fetchQueries: string[]
|
||||
}
|
||||
|
||||
const TASK_SOURCE_BY_LABEL: Record<string, string> = {
|
||||
GitHub: 'github',
|
||||
GitLab: 'gitlab',
|
||||
|
|
@ -49,6 +54,105 @@ async function getRenderedTaskSources(
|
|||
}, TASK_SOURCE_BY_LABEL)
|
||||
}
|
||||
|
||||
async function openInstrumentedGitHubTasksPage(
|
||||
page: Parameters<typeof getStoreState>[0]
|
||||
): Promise<void> {
|
||||
await page.evaluate(() => {
|
||||
const store = window.__store
|
||||
if (!store) {
|
||||
throw new Error('window.__store is not available')
|
||||
}
|
||||
const state = store.getState()
|
||||
const activeWorktree = Object.values(state.worktreesByRepo)
|
||||
.flat()
|
||||
.find((worktree) => worktree.id === state.activeWorktreeId)
|
||||
const repo = state.repos.find((candidate) => candidate.id === activeWorktree?.repoId)
|
||||
if (!repo || !state.settings) {
|
||||
throw new Error('GitHub Tasks probe requires a ready repository and settings')
|
||||
}
|
||||
const probe: TaskSearchRequestProbe = { countQueries: [], fetchQueries: [] }
|
||||
;(
|
||||
window as typeof window & { __taskSearchRequestProbe?: TaskSearchRequestProbe }
|
||||
).__taskSearchRequestProbe = probe
|
||||
const existingIssue = {
|
||||
id: 'issue:999',
|
||||
type: 'issue' as const,
|
||||
number: 999,
|
||||
title: 'Existing GitHub issue',
|
||||
state: 'open' as const,
|
||||
url: 'https://github.com/orca/e2e/issues/999',
|
||||
labels: [],
|
||||
updatedAt: '2026-08-08T00:00:00Z',
|
||||
author: 'orca-e2e',
|
||||
repoId: repo.id,
|
||||
assignees: [],
|
||||
reviewRequests: []
|
||||
}
|
||||
|
||||
store.setState({
|
||||
repos: state.repos.map((candidate) =>
|
||||
candidate.id === repo.id
|
||||
? { ...candidate, upstream: { owner: 'orca', repo: 'e2e' } }
|
||||
: candidate
|
||||
),
|
||||
settings: {
|
||||
...state.settings,
|
||||
defaultTaskSource: 'github',
|
||||
defaultTaskViewPreset: 'issues',
|
||||
visibleTaskProviders: ['github']
|
||||
},
|
||||
taskResumeState: {
|
||||
...state.taskResumeState,
|
||||
githubItemsPreset: 'issues',
|
||||
githubItemsQuery: 'is:issue is:open',
|
||||
githubMode: 'items'
|
||||
},
|
||||
prefetchWorkItems: () => undefined,
|
||||
fetchWorkItemsAcrossRepos: async (_repos, _perRepoLimit, _displayLimit, query) => {
|
||||
probe.fetchQueries.push(query)
|
||||
return { items: [existingIssue], failedCount: 0, githubUnavailable: false }
|
||||
},
|
||||
countWorkItemsAcrossRepos: async (_repos, query) => {
|
||||
probe.countQueries.push(query)
|
||||
return { totalCount: 1, totalPages: 1 }
|
||||
}
|
||||
})
|
||||
store
|
||||
.getState()
|
||||
.openTaskPage(
|
||||
{ taskSource: 'github', preselectedRepoId: repo.id },
|
||||
{ recordTasksInteraction: false }
|
||||
)
|
||||
})
|
||||
}
|
||||
|
||||
async function readTaskSearchRequestProbe(
|
||||
page: Parameters<typeof getStoreState>[0]
|
||||
): Promise<TaskSearchRequestProbe> {
|
||||
return page.evaluate(() => {
|
||||
const probe = (window as typeof window & { __taskSearchRequestProbe?: TaskSearchRequestProbe })
|
||||
.__taskSearchRequestProbe
|
||||
if (!probe) {
|
||||
throw new Error('Task search request probe is not installed')
|
||||
}
|
||||
return { countQueries: [...probe.countQueries], fetchQueries: [...probe.fetchQueries] }
|
||||
})
|
||||
}
|
||||
|
||||
async function resetTaskSearchRequestProbe(
|
||||
page: Parameters<typeof getStoreState>[0]
|
||||
): Promise<void> {
|
||||
await page.evaluate(() => {
|
||||
const probe = (window as typeof window & { __taskSearchRequestProbe?: TaskSearchRequestProbe })
|
||||
.__taskSearchRequestProbe
|
||||
if (!probe) {
|
||||
throw new Error('Task search request probe is not installed')
|
||||
}
|
||||
probe.countQueries.length = 0
|
||||
probe.fetchQueries.length = 0
|
||||
})
|
||||
}
|
||||
|
||||
test.describe('Tasks page', () => {
|
||||
test.beforeEach(async ({ orcaPage }) => {
|
||||
await waitForSessionReady(orcaPage)
|
||||
|
|
@ -129,4 +233,46 @@ test.describe('Tasks page', () => {
|
|||
await expect(orcaPage.locator('.xterm').first()).toBeVisible({ timeout: 5_000 })
|
||||
}
|
||||
})
|
||||
|
||||
test('GitHub search waits for idle, keeps rows visible, and Enter does not double-fetch', async ({
|
||||
orcaPage
|
||||
}) => {
|
||||
await openInstrumentedGitHubTasksPage(orcaPage)
|
||||
|
||||
const input = orcaPage.getByPlaceholder('Search GitHub issues...')
|
||||
const existingIssue = orcaPage.getByText('Existing GitHub issue', { exact: true })
|
||||
await expect(input).toBeVisible()
|
||||
await expect(existingIssue).toBeVisible()
|
||||
|
||||
await input.fill('')
|
||||
await orcaPage.waitForTimeout(800)
|
||||
await resetTaskSearchRequestProbe(orcaPage)
|
||||
|
||||
await input.pressSequentially('rate', { delay: 400 })
|
||||
|
||||
expect(await readTaskSearchRequestProbe(orcaPage)).toEqual({
|
||||
countQueries: [],
|
||||
fetchQueries: []
|
||||
})
|
||||
await expect(existingIssue).toBeVisible()
|
||||
|
||||
await expect
|
||||
.poll(async () => readTaskSearchRequestProbe(orcaPage), { timeout: 2_000 })
|
||||
.toEqual({ countQueries: ['is:issue rate'], fetchQueries: ['is:issue rate'] })
|
||||
|
||||
await resetTaskSearchRequestProbe(orcaPage)
|
||||
await input.pressSequentially('x')
|
||||
await input.press('Enter')
|
||||
|
||||
await expect
|
||||
.poll(async () => readTaskSearchRequestProbe(orcaPage), { timeout: 2_000 })
|
||||
.toEqual({ countQueries: ['is:issue ratex'], fetchQueries: ['is:issue ratex'] })
|
||||
await orcaPage.waitForTimeout(800)
|
||||
expect(await readTaskSearchRequestProbe(orcaPage)).toEqual({
|
||||
countQueries: ['is:issue ratex'],
|
||||
fetchQueries: ['is:issue ratex']
|
||||
})
|
||||
await expect(input).toHaveValue('is:issue ratex')
|
||||
await expect(existingIssue).toBeVisible()
|
||||
})
|
||||
})
|
||||
|
|
|
|||
Loading…
Reference in New Issue