fix: improve ChannelList performance by improving threshold calls and rerenders of Header #1919
Conversation
package/src/components/ChannelList/hooks/usePaginatedChannels.ts
Outdated
Show resolved
Hide resolved
| const [staticChannelsActive, setStaticChannelsActive] = useState<boolean>(false); | ||
| const [activeQueryType, setActiveQueryType] = useState<QueryType | null>(null); | ||
| const [hasNextPage, setHasNextPage] = useState<boolean>(false); |
There was a problem hiding this comment.
also here, initally hasnextPage is true right as before? we dont know if its false.. but we can assume that there is always a nextPage and if not the api would return empty
There was a problem hiding this comment.
It makes more sense to be false initially. If there are more data, it will be true. It is almost the same thing just making it more readable
There was a problem hiding this comment.
Usually these kind of things are set to true initially.. also explained by vishal in the call
There was a problem hiding this comment.
But why can't it be false, this will never be true for the first fetch and can be true only if there are more data to load, right? Just asking to check if I am wrong here.
Eg: See loadingExtraData here - https://alexb72.medium.com/how-to-add-pagination-to-a-react-native-flatlist-ce2425d02f1a
There was a problem hiding this comment.
Aren't we receiving that info from our BE after the first fetch? I thought we had a "page_count" or such?
There was a problem hiding this comment.
Technically it makes more sense to keep it false so that no logic will load more/extra data initially no matter if there's any data or not; if there's more data after first call it will be true, and the query will be called automatically
There was a problem hiding this comment.
if its false initially.. why does it query? I thought we query only if hasNextPage is true
There was a problem hiding this comment.
If you go through the code. queryType is reload for the first time, which means it should call queryChannels to get the first 30 sets of data, then if there's more data or equal data we set the hasNextPage to true and next call is done to get more data. Initially, if it's false it will be true after the first call if there's more data. What's the point in keeping it true since the beginning?
My point is regardless of what it is it will be true eventually if there's more data left to receive so why should we keep it true?
There was a problem hiding this comment.
Check the useEffect for !enableOfflineSupport case and you will get what is happening.
There was a problem hiding this comment.
@santhoshvai we set the hasNextPage only if after the first queryChannels call is made and when there is an extra page/data (we do it by calculating channelQueryResponse.length >= newOptions.limit).
Wit this PR we call queryChannels only when needed (hasNextPage is true) and use of resources has been limited
package/src/components/ChannelList/hooks/usePaginatedChannels.ts
Outdated
Show resolved
Hide resolved
...s/docs/reactnative/common-content/core-components/channel-list/props/load_more_threshold.mdx
Outdated
Show resolved
Hide resolved
| const [staticChannelsActive, setStaticChannelsActive] = useState<boolean>(false); | ||
| const [activeQueryType, setActiveQueryType] = useState<QueryType | null>(null); | ||
| const [hasNextPage, setHasNextPage] = useState<boolean>(false); |
There was a problem hiding this comment.
Aren't we receiving that info from our BE after the first fetch? I thought we had a "page_count" or such?
|
This PR get's my approval after we settle the discussion about the initial state of |
Co-authored-by: Steve Galili <galiliziv@gmail.com>
|
Kudos, SonarCloud Quality Gate passed! |
|
After Beta release let's share with the relevant integrators! |
|
🎉 This PR is included in version 5.9.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
🎯 Goal
This PR focuses on fixing the Header rerender issue in ChannelList if provided, and multiple
queryChannelscall when the threshold is reached and more data is loaded.🛠 Implementation details
To fix the Header rerender the null check and
channels.lengthcheck is removed as it wasn't really needed.To fix the threshold API hits facebook/react-native#14015 (comment) and https://stackoverflow.com/a/60666252/10826415 was followed.
🎨 UI Changes
iOS
Android
🧪 Testing
☑️ Checklist
developbranch