Skip to content
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

Profiles to React Queries #633

Closed
wants to merge 4 commits into from
Closed

Profiles to React Queries #633

wants to merge 4 commits into from

Conversation

alexrisch
Copy link
Contributor

@alexrisch alexrisch commented Aug 30, 2024

Why?
Right now zustand is being used to manage the network state of profiles We refresh profiles whenever a new group is added/new conversation/group is refreshed

Theres some potential issues:
Groups can get large
Conversation lists can get large
New and interesting ways of using profiles in the app may be added (group invites for example) These require adding additional checks if we should update the profiles etc. What happens when a request fails? What happens if members change? As we move towards open source it can be helpful to devs to have more consumable hooks, rather than tracking down exactly where the data is coming from and why it's refreshing/cached/missing

With React Query we get better out of the box retries, and can use a batching and deduplicating library to manage this for us The main goal of this is that incase anything gets missed, we can have a foolproof fallback to refresh just in case

We can further improve the usage of profiles by:

  1. Having MMKV entries for each profile rather than the whole state
  2. Remove fetching in group syncs and have a full fetch for each groups members later on
  3. Completely removing the Zustand profile store to avoid storing network state in Zustand

Closes #583

Alex Risch and others added 2 commits October 11, 2024 10:01
Why?
Right now zustand is being used to manage the network state of profiles
We refresh profiles whenever a new group is added/new conversation/group is refreshed

Theres some potential issues:
Groups can get large
Conversation lists can get large
New and interesting ways of using profiles in the app may be added (group invites for example)
These require adding additional checks if we should update the profiles etc.
What happens when a request fails? What happens if members change?
As we move towards open source it can be helpful to devs to have more consumable hooks, rather than tracking down exactly where the data is coming from and why it's refreshing/cached/missing

With React Query we get better out of the box retries, and can use a batching and deduplicating library to manage this for us
The main goal of this is that incase anything gets missed, we can have a foolproof fallback to refresh just in case
@alexrisch alexrisch force-pushed the ar/refactor-profiles branch from 425b6db to e0dadc6 Compare October 11, 2024 14:57
@alexrisch alexrisch marked this pull request as ready for review October 11, 2024 14:59
@alexrisch alexrisch requested a review from a team as a code owner October 11, 2024 14:59
@alexrisch alexrisch changed the title WIP: Refactor of Profiles to React Query Refactor of Profiles to React Query Oct 11, 2024
@alexrisch alexrisch changed the title Refactor of Profiles to React Query Profiles to React Queries Oct 11, 2024
Comment on lines +13 to +23
const profileSocials = create({
fetcher: async (addresses: string[]) => {
const data = await getProfilesForAddresses(addresses);
return data;
},
resolver: indexedResolver(),
scheduler: windowedFiniteBatchScheduler({
windowMs: 10,
maxBatchSize: 150,
}),
});
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think react-query handle request deduplication

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does, but this helps with batching all requests
Example I am fetching profile A, B, C it will do all of those requests
Docs: https://tanstack.com/query/v4/docs/framework/react/community/batching-requests-using-bathshit#implementation

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OH very nice yeah makes sense didn't know about this

Comment on lines +133 to +137

export const profileSocialsQueryKey = (
account: string,
peerAddress: string
) => [QueryKeys.PROFILE_SOCIALS, account, peerAddress];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious, why did you choose to add all keys into 1 file? Just really curious. I'm not doing that but maybe it's the best practice. What I do is put each keys into it's own file

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just kinda did it one day, but I think how we are structuring folders by feature it probably makes more sense to split them into the separate files

Copy link
Collaborator

@thierryskoda thierryskoda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love that! Big fan of react-query. Would just make sure that it's aligned with how @technoplato planned to integrate them so that when we go with XState we don't have to refactor a ton.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature request: Move Profile Fetching to React Query
2 participants