fix infinite loading #9354
fix infinite loading #9354
Conversation
WalkthroughThe pull request modifies the Changes
Assessment against linked issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
src/components/Form/AutoCompleteAsync.tsx (3)
Line range hint
91-94: Consider adding loading state cleanup.To prevent any edge cases where the loading state might get stuck, consider adding a cleanup function to reset the loading state when the component unmounts or when the query changes.
useEffect(() => { fetchDataDebounced(query); + return () => { + setLoading(false); + }; }, [query]);
Line range hint
63-78: Consider implementing error state handling.While the error is logged to console, it might be helpful to show a user-friendly error message in the UI when the fetch fails. This would improve the user experience by providing clear feedback.
const fetchDataDebounced = useDebounce(async (searchQuery: string) => { setLoading(true); + setError(null); try { const fetchedData = (await fetchData(searchQuery)) || []; const filteredData = filter ? fetchedData.filter((item: any) => filter(item)) : fetchedData; setData( showNOptions !== undefined ? filteredData.slice(0, showNOptions) : filteredData, ); } catch (error) { console.error("Error fetching data:", error); + setError("Failed to fetch results. Please try again."); } finally { setLoading(false); } }, debounceTime);
Line range hint
116-127: Consider debouncing the onChange handler for the input.While we're debouncing the fetch, the
setQueryis still called on every keystroke. Consider debouncing the input change handler to reduce unnecessary state updates.+const setQueryDebounced = useDebounce((value: string) => { + setQuery(value); +}, 100); <ComboboxInput id={id} name={name} className={classNames( "cui-input-base truncate pr-16", error && "border-danger-500", )} placeholder={/* ... */} displayValue={/* ... */} - onChange={({ target }) => setQuery(target.value)} + onChange={({ target }) => setQueryDebounced(target.value)} onFocus={props.onFocus} onBlur={props.onBlur} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
src/components/Form/AutoCompleteAsync.tsx(1 hunks)
🔇 Additional comments (1)
src/components/Form/AutoCompleteAsync.tsx (1)
95-95: LGTM! This change correctly fixes the infinite loading issue.
Removing fetchDataDebounced from the dependency array is the right approach as it prevents the infinite loop caused by the function being recreated on each render. The debounced function maintains a stable reference through useDebounce, so it doesn't need to be in the deps array.
Let's verify the loading state behavior:
✅ Verification successful
The search results show consistent usage of useDebounce across the codebase. Let's verify the dependency arrays in these components to ensure they follow the same pattern.
Let's verify one more aspect - the usage of loading state in other components to ensure our pattern is consistent.
The dependency array change is correct and follows consistent patterns across the codebase
The verification confirms that:
- The loading state management in
AutoCompleteAsyncfollows the standard pattern used across the codebase, with propersetLoading(true)before async operations andsetLoading(false)in the finally block - Other components using
useDebounce(PatientRegister, DiagnosesFilter) also maintain stable function references and don't include the debounced function in dependency arrays - The loading state implementation is robust with proper error handling and cleanup
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other similar patterns in the codebase to ensure consistent implementation
ast-grep --pattern 'useDebounce($_, $_)'
Length of output: 4028
Script:
#!/bin/bash
# Search for useEffect patterns with query dependencies in these files
ast-grep --pattern 'useEffect(() => {
$$$
}, [$_])' src/components/Patient/PatientRegister.tsx src/components/Patient/DiagnosesFilter.tsx
Length of output: 2075
Script:
#!/bin/bash
# Search for loading state patterns in components using useDebounce
rg "setLoading\(" -A 2 -B 2
Length of output: 2788
|
LGTM @khavinshankar the cypress is re-run , verify it before merging |
|
@manmeetnagii Your efforts have helped advance digital healthcare and TeleICU systems. 🚀 Thank you for taking the time out to make CARE better. We hope you continue to innovate and contribute; your impact is immense! 🙌 |
Proposed Changes
Summary by CodeRabbit
Bug Fixes
Refactor