React-query: Fails to rerender component once data is loaded

[kyle-mccarthy]

I've noticed an odd issue while using react-query with next js where components sometimes do not rerender when data finishes loading (loading to success). From what I can tell, it only happens on the first render of the app and when multiple components are using the same query. In addition to the multiple components, one of them needs to have a side effect that causes a rerender on the first render (ex useEffect that sets some state).

Code sample: https://codesandbox.io/s/stoic-currying-n4h7x?file=/pages/_app.js

In the sample there are two pages, index and works, where the index page demonstrates the conditions that trigger the bug.

I've added a gif to help explain what I am talking about. Note the issue only happens on the first app render on the main index page where it calls set state from the effect but not on the first app render for the 'works' page. On the index page, making the sidebar rerender by incrementing the count fixes/syncs the state.

[hitbear518]

Same issue

[jcmnunes]

I started getting a similar issue on the first app render as well after upgrading react-query's version. After some research, I think the issue was introduced in v1.3.5.

In the codesandbox below, we can see that:

1. The issue goes way if you choose a version before v1.3.5 (e.g. v1.3.4).
2. Commenting the useEffect in Main.jsx fixes the issue also.

https://codesandbox.io/s/long-mountain-73425?file=/src/components/Main.jsx

[thebuilder]

Also started seeing this. It doesn't trigger an update in the hook consumer.
Downgrading to 1.3.4 fixed the issue for me.

[kyle-mccarthy]

@thebuilder I think that the bug was actually introduced in 1.3.0, I went through and tested the different versions in codesandbox and they started to break with 1.3.0...

• 1.3.0 has the bug
• 1.2.9 works correctly

[kyle-mccarthy]

I've tracked down the commit that introduced the bug.

[jcmnunes]

@thebuilder I think that the bug was actually introduced in 1.3.0, I went through and tested the different versions in codesandbox and they started to break with 1.3.0...

• 1.3.0 has the bug
• 1.2.9 works correctly

Interesting 🤔So, we might be dealing with two different issues here.

@kyle-mccarthy , just for the record, as I mentioned, you can easily see in the codesandbox below, which is essentially a simplified version of yours, in CRA with asyn/await, that the faulty version is v1.3.5.

Btw, thank you for the sandbox 🙏

https://codesandbox.io/s/long-mountain-73425?file=/src/components/Main.jsx

[kyle-mccarthy]

@jcmnunes oh yeah that is interesting... I wonder if they are the same bug but different conditions on which it occurs.

~~I tracked down the line of code that is actually causing the problem (well mine at least). I think that it may be a race condition or something of the sort. Commenting the line out fixes the problem but would break suspense from what I can tell.

https://github.com/tannerlinsley/react-query/blob/v1.4.1/src/useBaseQuery.js#L79~~

Actually that is just part of the problem. I think that I fixed it locally, but want to verify something first...

@tannerlinsley why are instances that are used for suspense filtered out in the query.updateInstance method? https://github.com/tannerlinsley/react-query/blob/master/src/queryCache.js#L293

[jcmnunes]

@jcmnunes oh yeah that is interesting... I wonder if they are the same bug but different conditions on which it occurs.

I tracked down the line of code that is actually causing the problem (well mine at least). I think that it may be a race condition or something of the sort. Commenting the line out fixes the problem but would break suspense from what I can tell.

https://github.com/tannerlinsley/react-query/blob/v1.4.1/src/useBaseQuery.js#L79

Nicely done @kyle-mccarthy 👍But that line is added precisely in v1.3.5. That checks out with what I found. Am I missing something? It seems we agree on the faulty version after all.

[kyle-mccarthy]

@jcmnunes So that line of code is part of the problem, but there was also a change to the queryCache introduced in 1.3.0 when the subscribe method was changed. https://github.com/tannerlinsley/react-query/compare/v1.2.9...v1.3.0#diff-2c6b0e709da79e736512dd96001d9716

I forked the repo and added my fix, it looks like it resolves both of our problems. https://github.com/kyle-mccarthy/react-query/commit/623a3dcc0af528eb5e1c5fb5707cdeadd9b48bea

I can make PR with the fix, however, I am not sure what the repercussions are of not filtering out instances that are used for suspense.

[jcmnunes]

I can make PR with the fix, however, I am not sure what the repercussions are of not filtering out instances that are used for suspense.

Yes, I totally understand that. We might need help from @tannerlinsley on this one ;)

[tannerlinsley]

I'm pushing a potential fix (pending the tests actually pass in CI this time... 🙄 )

[tannerlinsley]

I think it's gonna work!

[tannerlinsley]

[kyle-mccarthy]

@tannerlinsley awesome! BTW I just checked and the code on master does fix both issues. Thank you!