Race condition resolving queries with React's concurrent mode

[samkline]

Hi, I'm using urql's <Query> component with React's new concurrent mode (ReactDOM.unstable_createRoot as described here) and found that queries can be fulfilled by the GraphQL server but never re-rendered. That is, I've still got the { fetching: true, data: null } state because the component never re-rendered with data.

I originally thought this was #287 because the behavior seems exactly the same: sometimes it re-renders as expected, most times it doesn't, and throttling the network seems to improve odds of getting the expected behavior. I verified I'm using urql 1.1.3 so I have the fix for #287.

I can never reproduce this when using ReactDOM.render and I can consistently reproduce this with ReactDOM.unstable_createRoot, so it appears related to React's concurrent mode.

[kitten]

Hiya, can you check this with the example in this repo? Sorry, it's obviously a lot to ask but a reproduction always helps. It may be enough to try this with our 1-getting-started example.

So to summarise this:

• You're using urql 1.1.3
• You've enabled concurrent mode using unstable_createRoot (Have you tried using the unstable_ConcurrentMode element instead?)
• The initial state is not reflected correctly.

This seems suspicious (sorry in advance for the scepticism) in that we've introduced synchronous initial state and synchronous initial effects, so I don't even see where React would enter the hook without executing all our initial effects & state changes even in concurrent mode. Also the initial state is the following (note that data is not null)

{
    fetching: false,
    data: undefined,
    error: undefined,
}

So the only possibly state you're describing would be { fetching: true, data: undefined }, which is the normal fetching state and expected behaviour. So all in all, I'm pretty confused and that's why we'll need more details :)

[samkline]

Hey @kitten, thanks for the quick reply. That summary isn't quite right: the initial state is reflected correctly -- on first render, we haven't kicked off the network request yet so we get { fetching: true, data: undefined } (you're right that data was undefined, not null).

The issue is that when the network request finishes, in concurrent mode we don't always re-render the component with { fetching: false, data: ... }. Instead we get stuck in the { fetching: true } state. Disabling concurrent mode avoids the issue.

[kitten]

Interesting, I think it may be an issue with how React orders state updates in concurrent mode. Do you have a reproduction by any chance, or can you reproduce this simply by adding concurrent mode in one of our examples?

[kitten]

@samkline Hiya, I've tried to reproduce this on one of our examples without much luck. @iamstarkov You mentioned that you tried concurrent mode as well. Did you have any luck with reproducing the described behaviour here (outside of suspense mode) by any chance?

[iamstarkov]

@kitten there are few news. Published exchange eliminated errors but it barely works: sometimes it re-renders as expected, most times it doesn't. I described it and linked a PR and deployed demo in #293

[kitten]

@iamstarkov yea, just to clarify, you can reproduce this with suspense and concurrent mode, but not with either individually?

[iamstarkov]

I definitely tried it with Suspense, don’t remember about Concurrent mode

[kitten]

@iamstarkov I looked at your repository and as it turns out your issue is completely unrelated and I left a comment there. This PR is going to fix this for you: #369

Edit: v1.3.0 is now out which includes this PR

@samkline I'm still unsure whether your issue is related but I tried to replicate issues in concurrent mode and haven't been able to do so yet.

[kitten]

By chance I found a reference to this behaviour on Twitter, where someone is pointing out that React may be suspending after the initial mount and before the effect, then running the same component again with its previous state.

Hence setting any ref outside of effects is potentially unsafe if this is not taken into account.

https://twitter.com/ferdaber/status/1169706601542012928?s=21

[JoviDeCroock]

Hey @samkline

I've been trying to reproduce this issue, is there some condition that could help us reproduce it. We can assume a solution by just trying to move the refs inside effects but that doesn't guarantee solving the issue.

Maybe something like two fetches happening at the same instance or being children of each other.

[samkline]

Hey @kitten & @JoviDeCroock , apologies for the late response here. I went back and tried to reproduce the issue again with Urql 1.1.3 and wasn't able to. I didn't commit my work in the "broken" state, so it's possible something was different about my recent attempt to reproduce, or it's possible that this is just really non-deterministic.

Glad to hear there's a theory (and probable fix!) at least. I'm going to try concurrent mode in our application again and will report back if I notice any issues.