-
Notifications
You must be signed in to change notification settings - Fork 787
Fix useQuery keeps polling bug from #3272 #3273
Conversation
- update add link prop to the MockedProvider - add test that fails when component unmounted
@dqunbp: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Meteor Contributor Agreement here: https://contribute.meteor.com/ |
is the agreemetn signed? this causes apps to completelyt stop working in Chrome as it continues to fire off polling requests for components that no longer existing. |
@damiangreen It's of course signed |
@dqunbp nice work i hope it gets merged soon |
I am really excited for this PR to be merged. Could you help me understand why
Why is the |
I think the idea is taking this execution from the main thread and let the browser queue and do that when it's best for it. maybe?! 🤔 |
Maybe, but React defers running useEffect (and cleanup) until after the browser has painted, so doing extra work is less of a problem. So if that is the reason, is it necessary? |
Because inside setTimeout(() => {
this.currentObservable.query!.resetQueryStoreErrors();
}); This code breaks without it |
👍 makes sense! Thanks for taking the time to explain!
On Sun, Aug 4, 2019 at 11:44 AM dqunbp ***@***.***> wrote:
Because inside QueryData, setTimeout is used:
setTimeout(() => {
this.currentObservable.query!.resetQueryStoreErrors();
});
This code breaks without it
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#3273?email_source=notifications&email_token=AANWGKYBGK6PMXH5A6XSDR3QC32PNA5CNFSM4IGQJN3KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3QEIII#issuecomment-518013985>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AANWGK3QPBAFGNWN7OQI42LQC32PNANCNFSM4IGQJN3A>
.
--
*Tommy Groshong*
[email protected]
LinkedIn <http://www.linkedin.com/profile/view?id=96523046&trk=tab_pro> |
Google+ <https://plus.google.com/u/0/+ThomasGroshong/> | github
<https://github.com/tgroshon>
|
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.
Thanks very much for tackling this @dqunbp!
These changes will let us avoid the extra `setTimeout` call in the cleanup `useEffect`.
Fixes #3272