fix: Add default debounce & staleTime to file/folder prefetching #346
Conversation
WalkthroughThe hooks Changes
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
⏰ Context from checks skipped due to timeout of 90000ms (1)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
packages/web/src/hooks/usePrefetchFolderContents.ts (1)
21-33: 🛠️ Refactor suggestionSame dependency oversight as in file-source hook
useDebounceCallbackis missing its dependency array, causing a new debounced instance every render.Apply the same pattern as suggested for
usePrefetchFileSource:-const prefetchFolderContents = useDebounceCallback((repoName: string, revisionName: string, path: string) => { +const prefetchFolderContents = useDebounceCallback( + (repoName: string, revisionName: string, path: string) => { queryClient.prefetchQuery({ queryKey: ['tree', repoName, revisionName, path, domain], queryFn: () => unwrapServiceError( getFolderContents({ repoName, revisionName, path }, domain) ), staleTime, }); -}, debounceDelay); + }, + debounceDelay, + [queryClient, domain, staleTime] +);
🧹 Nitpick comments (2)
packages/web/src/hooks/usePrefetchFileSource.ts (1)
14-17: Duplicate default values scattered across hooksBoth file & folder hooks hard-code
200and5 * 60 * 1000.
Extracting them into a shared constant (e.g.PREFETCH_DEFAULTS) avoids drift and conveys intent.Nit but worth considering.
packages/web/src/hooks/usePrefetchFolderContents.ts (1)
14-17: Consolidate duplicated configuration constantsSame comment as in the file-source hook: consider centralising
debounceDelayandstaleTimedefaults to avoid magic numbers in multiple files.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/web/src/hooks/usePrefetchFileSource.ts(1 hunks)packages/web/src/hooks/usePrefetchFolderContents.ts(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/web/src/hooks/usePrefetchFileSource.ts (1)
packages/web/src/lib/utils.ts (1)
unwrapServiceError(392-399)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: build
- GitHub Check: build
- GitHub Check: build
brendan-kellam
force-pushed
the
bkellam/prefetch_debounce
branch
from
9729170 to
29352cf
Compare
We noticed that if you move your cursor across many files in the file tree and then click on one of the files, occasionally the file would never load. This was especially apparent in deployed environments like the demo instance.
My theory is that we were doing too many prefetches too quickly, resulting in some issue when loading the file after the user clicked on it. This PR adds:
staleTime(docs) of 5 minutes, meaning that if a user hovers over a file that has already been fetched, it won't be fetched again (until 5 minutes has elapsed).debounceDelaytime of 200ms, meaning we will only prefetch when the user's cursor has "settled" on a file.Summary by CodeRabbit
New Features
Refactor
Bug Fixes