feat(search): add debounced GitHub organization autocomplete - #160
feat(search): add debounced GitHub organization autocomplete#160AdityaOP007 wants to merge 1 commit into
Conversation
github-actions
Bot
added
enhancement
WalkthroughAdds a debounced GitHub organization autocomplete to the HomePage. The component supports cached searches, request cancellation, keyboard and pointer selection, accessible listbox states, outside-click dismissal, and integration with the existing organization workflow. ChangesOrganization autocomplete
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant OrganizationAutocomplete
participant useDebounce
participant searchOrganizations
participant GitHubAPI
User->>OrganizationAutocomplete: Enter organization query
OrganizationAutocomplete->>useDebounce: Schedule query
useDebounce-->>OrganizationAutocomplete: Return debounced query
OrganizationAutocomplete->>searchOrganizations: Search with abort signal
searchOrganizations->>GitHubAPI: Request organization results
GitHubAPI-->>searchOrganizations: Return search items
searchOrganizations-->>OrganizationAutocomplete: Return organizations
OrganizationAutocomplete-->>User: Render suggestions
Possibly related issues
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
first-time-contributor
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/OrganizationAutocomplete.jsx`:
- Around line 122-127: Update OrganizationAutocomplete’s highlightedIndex
navigation to track rendered suggestion option nodes with refs, then scroll the
newly highlighted option into view using scrollIntoView({ block: 'nearest' })
whenever highlightedIndex changes. Preserve the existing ArrowDown and ArrowUp
wrapping behavior.
- Around line 8-13: Update the cache access logic in OrganizationAutocomplete so
cache hits refresh the entry’s recency, making eviction from the Map-based cache
truly LRU rather than FIFO; preserve the existing single-entry eviction
behavior. Also expose a reset function for clearing the module-level cache so
tests can isolate their state.
- Around line 42-47: Clarify the intent of the useEffect watching value by
adding a comment that it immediately closes the dropdown and clears suggestions
before the debounced query effect runs. Keep its MIN_QUERY_LENGTH check aligned
with the later debounced-value effect, without changing the existing behavior.
- Around line 155-169: Update the OrganizationAutocomplete component signature
to accept an ariaLabel prop mapped from the “aria-label” attribute, defaulting
to an appropriate organization autocomplete name, then pass it to the combobox
input alongside the existing ARIA attributes.
- Around line 62-104: Guard all post-await success and error state updates in
the organization search flow with the same current-controller check used by the
finally block. In the request logic around searchOrganizations, only update
cache and suggestions, isOpen, highlightedIndex, or error when
abortControllerRef.current still equals controller; ensure stale responses
cannot overwrite the active query’s state.
- Around line 171-191: Update the suggestions <ul> container in
OrganizationAutocomplete so mousedown events on the dropdown, including its
scrollbar, prevent the input blur from committing the partial query. Preserve
the existing li selection behavior and only apply this change to the suggestions
list.
In `@src/hooks/useClickOutside.js`:
- Around line 4-19: Update the useClickOutside hook to store the latest handler
in a ref and invoke that ref from the document listener, keeping the listener
stable across renders. Remove handler from the effect’s subscription
dependencies while preserving ref-based outside-click detection and cleanup for
both mousedown and touchstart.
In `@src/services/github.js`:
- Around line 147-167: Extract shared helpers for GitHub request header
construction and response handling, then reuse them in fetchWithCache,
fetchRateLimit, and searchOrganizations. Move the existing Authorization/Accept
setup, rate-limit-update event dispatch, and 403/non-OK status mapping into
those helpers while preserving current behavior and each function’s
request-specific URL or cache logic.
- Around line 155-164: Prevent the `/search/users` response in the GitHub
request flow from dispatching its headers as the shared `rate-limit-update`
event, so it cannot overwrite core quota state used by `AppContext`, `Navbar`,
`RateLimitBanner`, and `SettingsPage`. Remove the dispatch around the rate-limit
payload, or consistently add a search resource distinction across state,
storage, and consumers.
In `@src/test/OrganizationAutocomplete.test.jsx`:
- Around line 23-31: Reset OrganizationAutocomplete’s module-level suggestion
cache before each test so tests are isolated and can reuse query strings safely.
Export a dedicated cache-reset function from the OrganizationAutocomplete
module, invoke it alongside vi.clearAllMocks() in beforeEach, and update the
cache test to use an explicit query rather than relying on prior test state.
- Around line 90-119: Add accessibility assertions to the keyboard-navigation
test around the combobox input: verify aria-expanded reflects the open and
closed states, and aria-activedescendant matches the highlighted option’s id
after ArrowDown and ArrowUp navigation. Add equivalent state assertions to the
Escape and outside-click test cases, ensuring focus and active-descendant values
are cleared when the list closes.
- Around line 67-76: Update the three OrganizationAutocomplete tests that wait
600 ms to use fake timers instead: enable fake timers for each test or shared
setup, advance past the 400 ms debounce within act, and restore real timers in
afterEach. Remove the real-time Promise delays while preserving each test’s
existing assertions.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3dfc5862-c442-42da-93af-d7ec869c3940
📒 Files selected for processing (6)
src/components/OrganizationAutocomplete.jsxsrc/hooks/useClickOutside.jssrc/hooks/useDebounce.jssrc/pages/HomePage.jsxsrc/services/github.jssrc/test/OrganizationAutocomplete.test.jsx
| // We use a small in-memory LRU cache specifically for autocomplete to prevent | ||
| // duplicating API requests during rapid typing and to avoid unnecessarily polluting | ||
| // the global IndexedDB cache with partially-typed, short-lived queries. | ||
| const cache = new Map(); | ||
| const MAX_SUGGESTIONS = 8; | ||
| const MIN_QUERY_LENGTH = 2; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
The cache is FIFO, not LRU, and the eviction removes only one entry.
The comment claims an LRU cache, but a read at line 70 does not reorder the key, so line 86 evicts the oldest inserted key. Either reinsert the key on a cache hit to make it a true LRU, or correct the comment. Also consider exporting a reset function so tests do not depend on module state that persists between cases.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/OrganizationAutocomplete.jsx` around lines 8 - 13, Update the
cache access logic in OrganizationAutocomplete so cache hits refresh the entry’s
recency, making eviction from the Map-based cache truly LRU rather than FIFO;
preserve the existing single-entry eviction behavior. Also expose a reset
function for clearing the module-level cache so tests can isolate their state.
| useEffect(() => { | ||
| if (value.trim().length < MIN_QUERY_LENGTH) { | ||
| setIsOpen(false); | ||
| setSuggestions([]); | ||
| } | ||
| }, [value]); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
The min-length gate is duplicated.
This effect repeats the gate in the effect at lines 49-56. The second effect already clears the suggestions and closes the dropdown once the debounced value falls under MIN_QUERY_LENGTH; this effect exists only to close it before the debounce elapses. Keep it, but state that intent in the comment, or merge both gates into one helper so the two thresholds cannot diverge.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/OrganizationAutocomplete.jsx` around lines 42 - 47, Clarify
the intent of the useEffect watching value by adding a comment that it
immediately closes the dropdown and clears suggestions before the debounced
query effect runs. Keep its MIN_QUERY_LENGTH check aligned with the later
debounced-value effect, without changing the existing behavior.
| if (abortControllerRef.current) { | ||
| abortControllerRef.current.abort(); | ||
| } | ||
|
|
||
| const controller = new AbortController(); | ||
| abortControllerRef.current = controller; | ||
|
|
||
| const cacheKey = trimmed.toLowerCase(); | ||
| if (cache.has(cacheKey)) { | ||
| setSuggestions(cache.get(cacheKey)); | ||
| setIsOpen(true); | ||
| setLoading(false); | ||
| setHighlightedIndex(-1); | ||
| return; | ||
| } | ||
|
|
||
| try { | ||
| const results = await searchOrganizations(trimmed, pat, controller.signal); | ||
|
|
||
| const deduplicated = results.filter((item, index, self) => | ||
| index === self.findIndex((t) => t.login.toLowerCase() === item.login.toLowerCase()) | ||
| ).slice(0, MAX_SUGGESTIONS); | ||
|
|
||
| if (cache.size > 100) { | ||
| const firstKey = cache.keys().next().value; | ||
| cache.delete(firstKey); | ||
| } | ||
| cache.set(cacheKey, deduplicated); | ||
|
|
||
| setSuggestions(deduplicated); | ||
| setIsOpen(true); | ||
| setHighlightedIndex(-1); | ||
| } catch (err) { | ||
| if (err.name !== 'AbortError') { | ||
| setError(true); | ||
| setSuggestions([]); | ||
| setIsOpen(true); | ||
| } | ||
| } finally { | ||
| if (abortControllerRef.current === controller) { | ||
| setLoading(false); | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Guard the post-await state writes with the current-controller check.
Lines 91-99 write state without verifying that controller is still the active request. abort() does not cancel a response that already settled, so a request for an earlier query can resolve after a newer request started and then overwrite suggestions, isOpen, and error for a query the user has left. The finally block at line 101 already applies the correct guard; apply it to the success and error paths too.
🐛 Proposed fix
try {
const results = await searchOrganizations(trimmed, pat, controller.signal);
+ if (abortControllerRef.current !== controller) return;
const deduplicated = results.filter((item, index, self) =>
index === self.findIndex((t) => t.login.toLowerCase() === item.login.toLowerCase())
).slice(0, MAX_SUGGESTIONS);
@@
setSuggestions(deduplicated);
setIsOpen(true);
setHighlightedIndex(-1);
} catch (err) {
- if (err.name !== 'AbortError') {
+ if (err.name !== 'AbortError' && abortControllerRef.current === controller) {
setError(true);
setSuggestions([]);
setIsOpen(true);
}
} finally {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (abortControllerRef.current) { | |
| abortControllerRef.current.abort(); | |
| } | |
| const controller = new AbortController(); | |
| abortControllerRef.current = controller; | |
| const cacheKey = trimmed.toLowerCase(); | |
| if (cache.has(cacheKey)) { | |
| setSuggestions(cache.get(cacheKey)); | |
| setIsOpen(true); | |
| setLoading(false); | |
| setHighlightedIndex(-1); | |
| return; | |
| } | |
| try { | |
| const results = await searchOrganizations(trimmed, pat, controller.signal); | |
| const deduplicated = results.filter((item, index, self) => | |
| index === self.findIndex((t) => t.login.toLowerCase() === item.login.toLowerCase()) | |
| ).slice(0, MAX_SUGGESTIONS); | |
| if (cache.size > 100) { | |
| const firstKey = cache.keys().next().value; | |
| cache.delete(firstKey); | |
| } | |
| cache.set(cacheKey, deduplicated); | |
| setSuggestions(deduplicated); | |
| setIsOpen(true); | |
| setHighlightedIndex(-1); | |
| } catch (err) { | |
| if (err.name !== 'AbortError') { | |
| setError(true); | |
| setSuggestions([]); | |
| setIsOpen(true); | |
| } | |
| } finally { | |
| if (abortControllerRef.current === controller) { | |
| setLoading(false); | |
| } | |
| } | |
| if (abortControllerRef.current) { | |
| abortControllerRef.current.abort(); | |
| } | |
| const controller = new AbortController(); | |
| abortControllerRef.current = controller; | |
| const cacheKey = trimmed.toLowerCase(); | |
| if (cache.has(cacheKey)) { | |
| setSuggestions(cache.get(cacheKey)); | |
| setIsOpen(true); | |
| setLoading(false); | |
| setHighlightedIndex(-1); | |
| return; | |
| } | |
| try { | |
| const results = await searchOrganizations(trimmed, pat, controller.signal); | |
| if (abortControllerRef.current !== controller) return; | |
| const deduplicated = results.filter((item, index, self) => | |
| index === self.findIndex((t) => t.login.toLowerCase() === item.login.toLowerCase()) | |
| ).slice(0, MAX_SUGGESTIONS); | |
| if (cache.size > 100) { | |
| const firstKey = cache.keys().next().value; | |
| cache.delete(firstKey); | |
| } | |
| cache.set(cacheKey, deduplicated); | |
| setSuggestions(deduplicated); | |
| setIsOpen(true); | |
| setHighlightedIndex(-1); | |
| } catch (err) { | |
| if (err.name !== 'AbortError' && abortControllerRef.current === controller) { | |
| setError(true); | |
| setSuggestions([]); | |
| setIsOpen(true); | |
| } | |
| } finally { | |
| if (abortControllerRef.current === controller) { | |
| setLoading(false); | |
| } | |
| } |
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] 90-90: Avoid using the initial state variable in setState
Context: setSuggestions(deduplicated)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(setstate-same-var)
🪛 React Doctor (0.9.3)
[error] 102-102: This resets a loading/busy flag only on the success path: if the awaited call rejects the reset never runs and the flag stays stuck truthy (a spinner that never stops, a button disabled forever). Move the reset into a finally block, or mirror it on every catch, so it clears on rejection too.
A trailing setLoading(false) after an await never runs if the awaited call rejects, so the flag stays stuck truthy; reset it in a finally block (or mirror the reset on every catch) so it clears on both paths.
(no-loading-flag-reset-outside-finally)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/OrganizationAutocomplete.jsx` around lines 62 - 104, Guard all
post-await success and error state updates in the organization search flow with
the same current-controller check used by the finally block. In the request
logic around searchOrganizations, only update cache and suggestions, isOpen,
highlightedIndex, or error when abortControllerRef.current still equals
controller; ensure stale responses cannot overwrite the active query’s state.
Source: Linters/SAST tools
| if (e.key === 'ArrowDown') { | ||
| e.preventDefault(); | ||
| setHighlightedIndex(prev => (prev < suggestions.length - 1 ? prev + 1 : 0)); | ||
| } else if (e.key === 'ArrowUp') { | ||
| e.preventDefault(); | ||
| setHighlightedIndex(prev => (prev > 0 ? prev - 1 : suggestions.length - 1)); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
Scroll the highlighted option into view.
Arrow navigation moves highlightedIndex through up to eight options inside a 250 px scroll container. The active option can stay outside the visible area, so keyboard users cannot see the current selection. Track the option nodes with a ref and call scrollIntoView({ block: 'nearest' }) when highlightedIndex changes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/OrganizationAutocomplete.jsx` around lines 122 - 127, Update
OrganizationAutocomplete’s highlightedIndex navigation to track rendered
suggestion option nodes with refs, then scroll the newly highlighted option into
view using scrollIntoView({ block: 'nearest' }) whenever highlightedIndex
changes. Preserve the existing ArrowDown and ArrowUp wrapping behavior.
| return ( | ||
| <div ref={containerRef} style={{ position: 'relative', flex: 1, minWidth: 160 }}> | ||
| <input | ||
| value={value} | ||
| onChange={onChange} | ||
| onKeyDown={handleKeyDown} | ||
| onBlur={handleBlur} | ||
| placeholder={placeholder} | ||
| style={{ ...style, width: '100%', boxSizing: 'border-box' }} | ||
| role="combobox" | ||
| aria-expanded={isOpen} | ||
| aria-controls="organization-suggestions" | ||
| aria-autocomplete="list" | ||
| aria-activedescendant={highlightedIndex >= 0 ? `suggestion-${highlightedIndex}` : undefined} | ||
| /> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Give the combobox an accessible name.
The input exposes role="combobox" but has only a placeholder. Screen readers do not treat a placeholder as a reliable accessible name, so the control is announced without a purpose. Add an explicit aria-label prop, and default it for the organization use case.
♿ Proposed fix
<input
value={value}
onChange={onChange}
onKeyDown={handleKeyDown}
onBlur={handleBlur}
placeholder={placeholder}
style={{ ...style, width: '100%', boxSizing: 'border-box' }}
role="combobox"
+ aria-label={ariaLabel || 'Search GitHub organizations'}
aria-expanded={isOpen}
aria-controls="organization-suggestions"
aria-autocomplete="list"
+ autoComplete="off"
aria-activedescendant={highlightedIndex >= 0 ? `suggestion-${highlightedIndex}` : undefined}
/>Add the prop to the signature at lines 15-23 as 'aria-label': ariaLabel.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/OrganizationAutocomplete.jsx` around lines 155 - 169, Update
the OrganizationAutocomplete component signature to accept an ariaLabel prop
mapped from the “aria-label” attribute, defaulting to an appropriate
organization autocomplete name, then pass it to the combobox input alongside the
existing ARIA attributes.
| export async function searchOrganizations(query, pat, signal) { | ||
| try { | ||
| const headers = { Accept: 'application/vnd.github.v3+json' } | ||
| if (pat) headers.Authorization = `token ${pat}` | ||
|
|
||
| const url = `https://api.github.com/search/users?q=${encodeURIComponent(query)}+type:org&per_page=8` | ||
| const res = await fetch(url, { headers, signal }) | ||
|
|
||
| window.dispatchEvent( | ||
| new CustomEvent('rate-limit-update', { | ||
| detail: { | ||
| limit: Number(res.headers.get('x-ratelimit-limit')), | ||
| remaining: Number(res.headers.get('x-ratelimit-remaining')), | ||
| used: Number(res.headers.get('x-ratelimit-used')), | ||
| reset: Number(res.headers.get('x-ratelimit-reset')) | ||
| } | ||
| }) | ||
| ) | ||
|
|
||
| if (res.status === 403) throw new Error('RATE_LIMIT') | ||
| if (!res.ok) throw new Error(`HTTP_${res.status}`) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Extract the shared header and response-handling logic.
Lines 149-150 and 155-167 repeat fetchWithCache (lines 60-78) verbatim. Duplicated header construction, event dispatch, and status mapping will diverge when one copy changes. Extract one helper that builds headers and one that handles the response, then call both from fetchWithCache, fetchRateLimit, and searchOrganizations.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/services/github.js` around lines 147 - 167, Extract shared helpers for
GitHub request header construction and response handling, then reuse them in
fetchWithCache, fetchRateLimit, and searchOrganizations. Move the existing
Authorization/Accept setup, rate-limit-update event dispatch, and 403/non-OK
status mapping into those helpers while preserving current behavior and each
function’s request-specific URL or cache logic.
| window.dispatchEvent( | ||
| new CustomEvent('rate-limit-update', { | ||
| detail: { | ||
| limit: Number(res.headers.get('x-ratelimit-limit')), | ||
| remaining: Number(res.headers.get('x-ratelimit-remaining')), | ||
| used: Number(res.headers.get('x-ratelimit-used')), | ||
| reset: Number(res.headers.get('x-ratelimit-reset')) | ||
| } | ||
| }) | ||
| ) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all listeners and producers of the rate-limit-update event.
rg -n -C 8 "rate-limit-update" --glob '*.{js,jsx,ts,tsx}'
# Find rate-limit rendering surfaces.
rg -n -C 5 -i "ratelimit|rate_limit|remaining" --glob 'src/**/*.{js,jsx}' -g '!src/services/github.js'Repository: AOSSIE-Org/OrgExplorer
Length of output: 160
🏁 Script executed:
#!/bin/bash
set -u
printf '%s\n' '--- candidate files ---'
git ls-files | rg '(^|/)(github|.*rate.*limit.*|.*github.*)\.(js|jsx|ts|tsx)$' || true
printf '%s\n' '--- service symbols and event references ---'
rg -n -C 12 --glob '*.{js,jsx,ts,tsx}' \
'rate-limit-update|fetchWithCache|searchOrganizations|x-ratelimit' . || true
printf '%s\n' '--- github.js structure ---'
if [ -f src/services/github.js ]; then
ast-grep outline src/services/github.js || true
printf '%s\n' '--- github.js relevant source ---'
sed -n '1,220p' src/services/github.js
fiRepository: AOSSIE-Org/OrgExplorer
Length of output: 38028
🏁 Script executed:
#!/bin/bash
set -u
printf '%s\n' '--- AppContext rate-limit state and consumers ---'
sed -n '1,130p' src/context/AppContext.jsx
printf '%s\n' '--- all rate-limit state usages ---'
rg -n -C 10 --glob '*.{js,jsx,ts,tsx}' \
'rateLimit|oe_rate_limit|x-ratelimit|rate-limit-update' src || trueRepository: AOSSIE-Org/OrgExplorer
Length of output: 25491
Keep search quota separate from core rate-limit state.
AppContext.jsx stores each rate-limit-update payload in the shared rateLimit state and oe_rate_limit storage. A /search/users response can therefore replace the core quota displayed by Navbar, RateLimitBanner, and SettingsPage.
Remove this dispatch, or add a resource: 'search' field and update the state, storage, and consumers to keep search and core quotas separate.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/services/github.js` around lines 155 - 164, Prevent the `/search/users`
response in the GitHub request flow from dispatching its headers as the shared
`rate-limit-update` event, so it cannot overwrite core quota state used by
`AppContext`, `Navbar`, `RateLimitBanner`, and `SettingsPage`. Remove the
dispatch around the rate-limit payload, or consistently add a search resource
distinction across state, storage, and consumers.
| beforeEach(() => { | ||
| vi.clearAllMocks(); | ||
| appContext.useApp.mockReturnValue({ pat: 'fake-token' }); | ||
| githubService.searchOrganizations.mockResolvedValue([ | ||
| { id: 1, login: 'reactjs', avatar_url: 'img1.png' }, | ||
| { id: 2, login: 'react', avatar_url: 'img2.png' }, | ||
| { id: 3, login: 'react', avatar_url: 'img2.png' } // duplicate | ||
| ]); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Reset the component cache between tests.
OrganizationAutocomplete holds its suggestion cache in a module-level Map. vi.clearAllMocks() does not clear it, so every test here uses a distinct query string ("debounce", "react", "keyboard", "select", "stale1", "cache_test", ...) to avoid cross-test hits. That makes the suite order-dependent and it hides regressions in the cache key logic.
Export a reset function from the component module and call it in beforeEach, or use vi.resetModules() with a dynamic import per test. The cache test at lines 227-248 can then use an explicit query without relying on leftover state.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/test/OrganizationAutocomplete.test.jsx` around lines 23 - 31, Reset
OrganizationAutocomplete’s module-level suggestion cache before each test so
tests are isolated and can reuse query strings safely. Export a dedicated
cache-reset function from the OrganizationAutocomplete module, invoke it
alongside vi.clearAllMocks() in beforeEach, and update the cache test to use an
explicit query rather than relying on prior test state.
| it('does not trigger for less than 2 characters', async () => { | ||
| const { rerender } = renderComponent({ value: '' }); | ||
|
|
||
| rerender(<OrganizationAutocomplete value="r" onChange={mockOnChange} />); | ||
|
|
||
| // Wait for the debounce time to pass to ensure it is not called | ||
| await new Promise(r => setTimeout(r, 600)); | ||
|
|
||
| expect(githubService.searchOrganizations).not.toHaveBeenCalled(); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Vitest fake timers with React Testing Library act and userEvent advanceTimers
💡 Result:
When using Vitest fake timers with React Testing Library's userEvent, you must ensure that userEvent can interact with the fake timer system to prevent test timeouts during simulated user interactions [1][2]. To configure this, pass the Vitest advance timer function to the advanceTimers option in userEvent.setup [1][3][4]. Configuration Example: const user = userEvent.setup({ advanceTimers: vi.advanceTimersByTime, }); Key Considerations: 1. Automatic Detection: Recent versions of user-event include internal logic designed to automatically detect Vitest's timer environment if vi or jest globals are present [5]. If your tests still timeout, explicitly providing the advanceTimers option as shown above is the standard and recommended solution [6][3][4]. 2. Handling Asynchronous Work: While passing vi.advanceTimersByTime is generally sufficient for user-event's internal delays, if your component logic or subsequent assertions involve promises triggered by those timers, you may need to use vi.advanceTimersByTimeAsync [3]. 3. Wrapping in act: When advancing time manually (e.g., to trigger debounced functions or delays in your own application code), always wrap the advancement in act to ensure React processes state updates correctly [3]: await act(async => { await vi.advanceTimersByTimeAsync(300); }); Do not use delay: null as a workaround for timeouts, as this can cause unpredictable behavior in how events are processed [1][7]. Instead, rely on the explicit advanceTimers configuration [1].
Citations:
- 1: https://testing-library.com/docs/user-event/options
- 2: https://testing-library.com/docs/using-fake-timers/
- 3: https://qaskills.sh/blog/vitest-testing-library-user-event-fake-timers
- 4: Bug Report: userEvent click causing test timeout in Vitest but works in Jest vitest-dev/vitest#6179
- 5: feat: automatically advance fake timers in Vitest testing-library/user-event#1304
- 6: Update to v14 breaks @testing-library/user-event on Vitest testing-library/react-testing-library#1197
- 7: https://testing-library.com/docs/user-event/options/
🏁 Script executed:
# Inspect the test setup, timer usage, and component debounce implementation.
printf '%s\n' '--- test file ---'
sed -n '1,320p' src/test/OrganizationAutocomplete.test.jsx
printf '%s\n' '--- package/config references ---'
rg -n "vitest|jest|fakeTimers|afterEach|OrganizationAutocomplete|setTimeout|debounce" \
package.json vitest.config.* vite.config.* src . --glob '!node_modules' --glob '!dist' --glob '!build' 2>/dev/null | head -240Repository: AOSSIE-Org/OrgExplorer
Length of output: 25892
🏁 Script executed:
# Inspect the debounce hook, component cache lifecycle, and Vitest setup to determine
# whether fake timers and a test-level afterEach are safe and sufficient.
printf '%s\n' '--- debounce hook ---'
cat -n src/hooks/useDebounce.js
printf '%s\n' '--- autocomplete component ---'
sed -n '1,180p' src/components/OrganizationAutocomplete.jsx
printf '%s\n' '--- Vitest config ---'
cat -n vite.config.js
printf '%s\n' '--- test setup ---'
cat -n src/test/setup.js
printf '%s\n' '--- package scripts and test dependencies ---'
sed -n '1,75p' package.jsonRepository: AOSSIE-Org/OrgExplorer
Length of output: 7562
Replace real-time debounce waits with fake timers.
Use fake timers in the three tests that wait 600 ms. Advance past the 400 ms debounce inside act, then restore real timers in afterEach. This removes about 1.8 seconds of test time and avoids wall-clock delays in CI.
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] 72-72: Avoid using the initial state variable in setState
Context: setTimeout(r, 600)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(setstate-same-var)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/test/OrganizationAutocomplete.test.jsx` around lines 67 - 76, Update the
three OrganizationAutocomplete tests that wait 600 ms to use fake timers
instead: enable fake timers for each test or shared setup, advance past the 400
ms debounce within act, and restore real timers in afterEach. Remove the
real-time Promise delays while preserving each test’s existing assertions.
| it('supports keyboard navigation', async () => { | ||
| const { rerender } = renderComponent({ value: '' }); | ||
|
|
||
| rerender( | ||
| <OrganizationAutocomplete | ||
| value="keyboard" | ||
| onChange={mockOnChange} | ||
| onKeyDown={mockOnKeyDown} | ||
| onSelectOrg={mockOnSelectOrg} | ||
| /> | ||
| ); | ||
|
|
||
| await waitFor(() => { | ||
| expect(screen.getByText('reactjs')).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| const input = screen.getByRole('combobox'); | ||
|
|
||
| fireEvent.keyDown(input, { key: 'ArrowDown' }); | ||
| expect(screen.getByRole('listbox').children[0]).toHaveAttribute('aria-selected', 'true'); | ||
|
|
||
| fireEvent.keyDown(input, { key: 'ArrowDown' }); | ||
| expect(screen.getByRole('listbox').children[1]).toHaveAttribute('aria-selected', 'true'); | ||
|
|
||
| fireEvent.keyDown(input, { key: 'ArrowUp' }); | ||
| expect(screen.getByRole('listbox').children[0]).toHaveAttribute('aria-selected', 'true'); | ||
|
|
||
| fireEvent.keyDown(input, { key: 'Enter' }); | ||
| expect(mockOnSelectOrg).toHaveBeenCalledWith('reactjs'); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
Assert the ARIA state during keyboard navigation.
The test checks aria-selected on the options, but no test covers the combobox state that assistive technology reads: aria-expanded on the input and aria-activedescendant pointing at the highlighted option id. Add those assertions here and after the Escape and outside-click cases.
💚 Proposed additions
const input = screen.getByRole('combobox');
+ expect(input).toHaveAttribute('aria-expanded', 'true');
fireEvent.keyDown(input, { key: 'ArrowDown' });
expect(screen.getByRole('listbox').children[0]).toHaveAttribute('aria-selected', 'true');
+ expect(input).toHaveAttribute('aria-activedescendant', 'suggestion-0');Based on path instructions for test files: "Accessibility testing is included".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/test/OrganizationAutocomplete.test.jsx` around lines 90 - 119, Add
accessibility assertions to the keyboard-navigation test around the combobox
input: verify aria-expanded reflects the open and closed states, and
aria-activedescendant matches the highlighted option’s id after ArrowDown and
ArrowUp navigation. Add equivalent state assertions to the Escape and
outside-click test cases, ensuring focus and active-descendant values are
cleared when the list closes.
Source: Path instructions
Link your account with GitcordThanks for opening this PR, @AdityaOP007! To receive Discord notifications and contributor tracking for this organization:
Once linked, Gitcord can notify you about reviews, merges, and more. — Posted by Gitcord |
1 participant
Summary
Implements debounced GitHub Organization Autocomplete for the Home page as requested in #141.
Users can now search for GitHub organizations while typing and quickly select an organization using either the mouse or keyboard, without affecting the existing chip-based organization workflow.
What's Included
AbortControllersupport to cancel stale requests and prevent outdated results from overwriting newer searches.Rate-Limit Protection
The implementation is designed to minimize GitHub API usage through:
This ensures that keyboard navigation and repeated searches do not unnecessarily generate additional GitHub API requests.
Validation
Targeted tests
Full test suite
Production build
No new dependencies were added, and the changes are limited to the implementation and tests required for #141.
Closes #141
Summary by CodeRabbit