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-query): fix missed updates between creation and subscription #5474

Merged
merged 6 commits into from
May 29, 2023

Conversation

TkDodo
Copy link
Collaborator

@TkDodo TkDodo commented May 26, 2023

closes #5443

@vercel
Copy link

vercel bot commented May 26, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
query ⬜️ Ignored (Inspect) May 29, 2023 11:13am

@nx-cloud
Copy link

nx-cloud bot commented May 26, 2023

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 5044427. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch


✅ Successfully ran 2 targets

Sent with 💌 from NxCloud.

@codesandbox-ci
Copy link

codesandbox-ci bot commented May 26, 2023

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.


// Update result to make sure we did not miss any query updates
// between creating the observer and subscribing to it.
observer.updateResult()
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks interesting, I'll try to test it in my project

Copy link
Contributor

Choose a reason for hiding this comment

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

So, I can not do it, cause of workspace: * in a build(

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

codesandbox preview seems to be having issues right now. You could patch-package the fix in your project, or we just ship it and trust the test coverage.

Copy link
Contributor

Choose a reason for hiding this comment

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

The main problem, that I cannot patch) It's to long story to describe)

Let's ship it, cause looks like this fix might help.

@codecov-commenter
Copy link

codecov-commenter commented May 27, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.44 🎉

Comparison is base (00319a1) 91.79% compared to head (908f4f6) 92.24%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5474      +/-   ##
==========================================
+ Coverage   91.79%   92.24%   +0.44%     
==========================================
  Files         101       39      -62     
  Lines        3890     1264    -2626     
  Branches      974      358     -616     
==========================================
- Hits         3571     1166    -2405     
+ Misses        298       92     -206     
+ Partials       21        6      -15     
Impacted Files Coverage Δ
packages/react-query/src/useBaseQuery.ts 95.74% <100.00%> (+0.29%) ⬆️

... and 62 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

},
})

// this simulates a synchronous update between the time the query is created
Copy link
Contributor

Choose a reason for hiding this comment

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

Please, correct me if I'm wrong, It's not important that the update was synchronous or asynchronous? You've used the synchronous update just because it's easer to test?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, exactly

@TkDodo TkDodo merged commit c290906 into main May 29, 2023
11 checks passed
@TkDodo TkDodo deleted the feature/fix-race-condition branch May 29, 2023 11:30
@artem-malko
Copy link
Contributor

@TkDodo good news, we've tested the latest version of react-query and everything is ok right now! THX a lot)

@ericbiewener I can recommend you to update react-query to 4.29.12

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