-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(table): resolve double fetch issue in useInfiniteScroll hook #3332
base: canary
Are you sure you want to change the base?
fix(table): resolve double fetch issue in useInfiniteScroll hook #3332
Conversation
🦋 Changeset detectedLatest commit: f8c7b6e The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@abhisektomar1 is attempting to deploy a commit to the NextUI Inc Team on Vercel. A member of the Team first needs to authorize it. |
Warning Rate limit exceeded@abhisektomar1 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 35 minutes and 45 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe Changes
Sequence Diagram(s)sequenceDiagram
participant Component as Component
participant useInfiniteScroll as useInfiniteScroll
participant loadMoreCallback as loadMoreCallback
participant onLoadMore as onLoadMore
Component ->> useInfiniteScroll: invoke()
useInfiniteScroll ->> loadMoreCallback: create with useCallback
loadMoreCallback ->> onLoadMore: invoke if conditions met
onLoadMore ->> Component: load more data
Note over loadMoreCallback: Added debounce to prevent multiple calls
Assessment against linked issues
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- packages/hooks/use-infinite-scroll/src/index.ts (3 hunks)
Additional context used
Biome
packages/hooks/use-infinite-scroll/src/index.ts
[error] 85-100: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
Unsafe fix: Omit the else clause.
Additional comments not posted (6)
packages/hooks/use-infinite-scroll/src/index.ts (6)
2-2
: Imports look good.The addition of
useCallback
supports the new debouncedloadMore
function, aligning with the PR objectives.
43-51
: Effective use ofuseCallback
and concurrency control inloadMore
.This implementation effectively prevents double fetching and ensures that
loadMore
is not recreated unnecessarily, which aligns with the PR objectives to optimize infinite scroll behavior.
66-66
: OptimizedIntersectionObserver
setup.The new
IntersectionObserver
setup uses improved options, including a root margin based on thedistance
prop and a lower threshold, which better handles the trigger for loading more content.Also applies to: 69-75, 78-78
80-84
: Proper cleanup forIntersectionObserver
.The cleanup logic properly disconnects the observer when the component unmounts or is no longer needed, preventing potential memory leaks and ensuring resource efficiency.
86-98
: Optimized scroll-based loading logic with debounce.The debounced event listener for scroll events is a good optimization, preventing excessive loading triggers and enhancing performance during rapid scrolling.
103-103
: Appropriate return values for external interaction.Returning
loaderRef
andscrollContainerRef
allows for external manipulation and interaction, which is essential for the hook's functionality in various scenarios.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since you've modified code inside packages, please add a changeset file. The content should be like this.
---
"@nextui-org/use-infinite-scroll": patch
---
fix(table): resolve double fetch issue in useInfiniteScroll hook (#3251)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- packages/hooks/use-infinite-scroll/src/index.ts (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- packages/hooks/use-infinite-scroll/src/index.ts
I noticed that there's a test failure related to the NextUI.Input component inside an Autocomplete component. However, my changes were only to the useInfiniteScroll hook and didn't touch these components. This seems to be a pre-existing issue in the test suite. Could someone from the maintenance team please advise on how to proceed? |
Summary
Resolved the issue where the useInfiniteScroll hook was fetching data twice on initial render. This fix ensures that data is fetched only once during initial render and additional data is fetched upon scrolling.
Issue
Changes
Closes #
📝 Description
I use isLoadingRef to prevent concurrent load calls, implements a debounced loadMore function with useCallback, and simplifies the IntersectionObserver logic.
The hook now handles both loader-based and scroll-based detection more efficiently, using debounce for scroll events. I also improved cleanup for the IntersectionObserver and better TypeScript typing. These changes collectively make the hook more robust, reducing issues like multiple simultaneous loads and excessive re-renders, while providing a smoother infinite scrolling experience
⛳️ Current behavior (updates)
Table useInfiniteScroll hook fetches twice
🚀 New behavior
💣 Is this a breaking change (Yes/No):
No
📝 Additional Information
Summary by CodeRabbit
loadMore
callback mechanism.