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

fix: react hooks observers #1126

Closed
wants to merge 3 commits into from
Closed

fix: react hooks observers #1126

wants to merge 3 commits into from

Conversation

helciofranco
Copy link
Member

This is exactly the same implementation of FuelLabs/fuels-npm-packs#55
Applying here just for compatibility purposes (temporarily since this will be removed soon).


This PR fixes and reduces a lot of re-renders across our query observers.

This issue was caused due to the way which @tanstack/react-query handles the tracked properties to re-renders.
Internally, they implemented a Proxy to intercept and track which properties a component is listening from a useQuery hook.

It means that when we're calling a useQuery with spread operators, it is forwarding to that Proxy that we want to listen to EVERY existent property.

const query = useQuery();

return { custom: query.data, ...query }; // <- "...query" is the issue here

So, what I did? I created the useNamedQuery that is also a Proxy and it'll also rename the data property to anything you want to name, and keeping the performance everywhere.

This ensures we're going to re-render ONLY when tracked properties get updates.

⚠️ Before ✅ Now
Screenshot 2024-03-08 at 11 13 38 Only 1 property will trigger re-renders Screenshot 2024-03-08 at 11 12 59

Copy link
Contributor

github-actions bot commented Mar 8, 2024

Coverage report for ./packages/app

St.
Category Percentage Covered / Total
🟡 Statements 64.9% 3258/5020
🔴 Branches 41.35% 586/1417
🔴 Functions 48.36% 680/1406
🟡 Lines 65.49% 3142/4798

Test suite run success

236 tests passing in 72 suites.

Report generated by 🧪jest coverage report action from 682f61c

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.

1 participant