-
Notifications
You must be signed in to change notification settings - Fork 523
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
Replace custom useQuery util with TanStack useQuery #9360
Conversation
Important Review skippedMore than 25% of the files skipped due to max files limit. The review is being skipped to prevent a low-quality review. 44 files out of 126 files are above the max files limit of 75. Please upgrade to Pro plan to get higher limits. You can disable this status message by setting the 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
This approach LGTM.
Just something to note, our request
function does perform query retries by itself. And in this implementation Tanstack's Query Client has retry configured to 1.
We could modify our request
to not perform retries.
care_fe/src/Utils/request/request.ts
Line 53 in b25ecd0
for (let i = 0; i < reattempts + 1; i++) { |
👋 Hi, @amjithtitus09, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
Looks like there's an issue with refetching when component remounts. Example: when we go to asset list page, and open an asset and go back to asset list page using the back button, it shows no assets found. |
…mports in new usages
…oute to ensure behaviour remains same as of previous impleemntation of useQuery
Opted out of caching between usages of the useQuery that has the same query key. This was causing so many weird state management issues with our existing implementation. Cache for the same useQuery invocations is still present as per defaults. |
@coderabbitai full review |
✅ Actions performedFull review triggered. |
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 we merged Users redesign FE yesterday, there's a few more instances of useQuery that ought to be replaced.
Other than that, LGTM.
Done, thanks for finding that miss! |
@rithviknishad @amjithtitus09 Your efforts have helped advance digital healthcare and TeleICU systems. 🚀 Thank you for taking the time out to make CARE better. We hope you continue to innovate and contribute; your impact is immense! 🙌 |
Proposed Changes
@ohcnetwork/care-fe-code-reviewers
Merge Checklist