fix: add debounce to channel file search #37973 #37980
Conversation
|
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
WalkthroughReplaced immediate search-query updates with a debounced search value (400ms) so the component queries using the stabilized Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/meteor/client/views/room/contextualBar/RoomFiles/RoomFilesWithData.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/client/views/room/contextualBar/RoomFiles/RoomFilesWithData.tsx
🧠 Learnings (1)
📚 Learning: 2025-12-02T22:23:49.593Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37654
File: apps/meteor/client/hooks/useAppSlashCommands.ts:32-38
Timestamp: 2025-12-02T22:23:49.593Z
Learning: In apps/meteor/client/hooks/useAppSlashCommands.ts, the `data?.forEach((command) => slashCommands.add(command))` call during render is intentional. The query is configured with `structuralSharing: false` to prevent React Query from keeping stable data references, and `slashCommands.add` is idempotent, so executing on every render is acceptable and ensures the command registry stays current.
Applied to files:
apps/meteor/client/views/room/contextualBar/RoomFiles/RoomFilesWithData.tsx
🔇 Additional comments (3)
apps/meteor/client/views/room/contextualBar/RoomFiles/RoomFilesWithData.tsx (3)
4-4: LGTM: Import and state initialization are correct.The
useEffectimport anddebouncedTextstate are properly added to support the debouncing mechanism.Also applies to: 17-17
33-40: LGTM: Query debouncing correctly implemented.The query construction properly uses
debouncedTextinstead oftext, and the dependency array is correctly updated. This ensures API calls only trigger after the user stops typing for 400ms, preventing rate limit errors while keeping the input responsive.
47-60: LGTM: Input remains responsive.Correctly passes the immediate
textvalue (line 51) to keep the input responsive while the API query uses the debounced value. This provides optimal UX by showing user input immediately while preventing rate limit errors.
|
Hey @MartinSchoeler i have made the changes that you have requested tell me if anything or any other issues that i can work on in this organisation. |
|
hey @MartinSchoeler can i get any update on this PR that i raised |
|
hey @MartinSchoeler can i get any update on this PR that i raised when will merge this ? |
|
hey @MartinSchoeler please review and merge the PR as it is long time |
fix: Add debouncing to file search to prevent rate limit errors #37973
Proposed changes (including videos or screenshots)
This PR fixes the rate limiting issue in the file search functionality by implementing debouncing. Previously, every keystroke triggered an immediate API call, causing rapid successive requests that exceeded the server's rate limit (HTTP 429 errors).
Changes made:
debouncedTextstate that updates with a 400ms delay after the user stops typinguseEffectto prevent excessive API callsBefore: Typing "Clipboard" (9 characters) = 9 API requests in rapid succession → Rate limit errors (HTTP 429)
After: Typing "Clipboard" = 1 API request after 400ms of typing inactivity → No rate limit errors
The UI remains responsive - the input field shows typed text immediately, but the actual API call is delayed until the user stops typing for 400ms.
Issue(s)
Closes #37973
Steps to test or reproduce
Further comments
I chose a 400ms debounce delay as it provides a good balance between responsiveness and preventing excessive API calls. This is a common debounce interval used in search implementations.
Alternative approaches considered:
The 400ms delay ensures that even rapid typing won't trigger rate limits while maintaining a smooth user experience.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.