Addressing a race condition when using React Query with Loadable Components

[ento]

I'm working with a project that has started using React Query in conjunction with Loadable Components.

We have a RQ query that gets run in both a component that gets immediately rendered and a component that gets loaded as a Loadable Component. The latter component sometimes doesn't get re-rendered with the query's latest data.

I've added debug logs to various places and the issue seems to happen when the order of events is like

Initial components get rendered
---> Query starts fetching data
Loadable component gets rendered
<--- Query finishes fetching data
useSyncExternalStore invokes the subscribe callback (for the useQuery call invoked by the loadable component)

One workaround we found is to add an useEffect hook that forces the loaded component to re-render once.

Here is a simplified version of the project, which is on React 16.9.0:

16.9-repro | codesandbox (You may need to adjust how long it takes to 'fetch' the remote data in queries.js. One number that worked in codesandbox for me is 700ms, although I needed to reload a few times to see it repro. The repro rate is closer to 100% when I run it locally)
16.9-workaround | codesandbox

The issue actually seems to go away when I upgrade to React 16.10.0. Bisecting the react repo's commits pointed to a commit that enables MessageLoop implementation of the scheduler by default as the one that fixed the issue.

However, when I modified the scheduler to always run out of time per loop, the issue surfaced again. The same workaround of forcing a re-render still works.

16.10-repro | codesandbox
16.10-workaround | codesandbox

The same thing happens with React v18: modifying the scheduler to always run out of time per loop reproduces the issue, and forcing a re-render fixes the issue.

18.2-repro | codesandbox
18.2-workaround | codesandbox

Having no real understanding of how these libraries should work in this situation, I'm not sure how to assess the issue.

1. Is the solution / workaround of forcing a re-render a reasonable one? Is there some other approach that React Query affords?
2. Is React switching to the MessageLoop implementation something that actually addresses the root cause of the race condition (or a deep enough cause of the issue) such that we can assume the race condition won't happen in practice, as long as I don't mess with the scheduler's internals? (Maybe this is a question for React folks rather than React Query, but I figured the community here would have expertise on both)

[TkDodo]

thanks for the detailed reproductions.

modifying the scheduler to always run out of time per loop reproduces the issue

I'm not sure what that means exactly? Which scheduler have you modified?

I've tried the react18 version on a new sandbox with no modifications to react-query or react and it runs fine: https://codesandbox.io/s/festive-chaum-gonecu?file=/src/index.jsx

so maybe there used to be an issue in react that has been fixed?

[ento]

I'm not sure what that means exactly? Which scheduler have you modified?

It's this scheduler NPM package that's managed in the react monorepo.

The modification I did is in this patch file:
https://github.com/ento/react-query-loadable-components-race-condition-mcve/blob/v18.2-repro/patches/scheduler%2B0.23.0.patch

diff --git a/node_modules/scheduler/cjs/scheduler.development.js b/node_modules/scheduler/cjs/scheduler.development.js
index 21bfb25..de80387 100644
--- a/node_modules/scheduler/cjs/scheduler.development.js
+++ b/node_modules/scheduler/cjs/scheduler.development.js
@@ -520,7 +520,7 @@ var performWorkUntilDeadline = function () {
     // has been blocked.
 
     startTime = currentTime;
-    var hasTimeRemaining = true; // If a scheduler task throws, exit the current browser task so the
+    var hasTimeRemaining = false; // If a scheduler task throws, exit the current browser task so the
     // error can be observed.
     //
     // Intentionally not using a try-catch, since that makes some debugging

What I think this does is make React's rendering loop yield to the browser on every loop, making it more likely for the browser to insert a unit of work into React's internal work queue (like when the Promise that fetches the lazy-loaded component gets resolved) before React can process all pending hook callbacks. If my understanding is correct, this helps reproduce the issue without changing how React works internally, as it only changes the timing of when each rendering loop reaches the deadline. (But my understanding could be off.)

so maybe there used to be an issue in react that has been fixed?

In the original post, I wrote "The issue actually seems to go away when I upgrade to React 16.10.0", but later we found out that the issue still persists with the actual project is upgraded to v16.10. So the switch to use the MessageLoop implementation didn't fix the issue, at least.

I have not had the chance to test the actual project in v18, but the issue reproduces when I run the v18.2-repro branch locally.

Though running it in codesandbox doesn't seem to reproduce it.. Having to load the lazily imported component over the internet might change the order of events significantly enough that it doesn't happen in codesandbox.