Notice: Recent issues with urql@^2.1 React Bindings

[kitten]

I'll keep this one brief, the tl;dr is, if you're using the React bindings, please either upgrade to [email protected] or revert to [email protected]. Ensure that @urql/core remains up-to-date for the latest patches on 2.4.3.

What happened?

In a recent version, we've decided to implement use-sync-external-store and use the useSyncExternalStore hook to align with what may become a future norm in React 18.

This wasn't strictly speaking necessary. A lot of this goes into React's tearing research. However, with each hook being rather independent with a GraphQL client, it was already a small concern that hooks being out-of-sync with updates could be of concern to us.

This didn't pan out. We forgot to port over some of the implementation details we needed to preserve. We encountered or re-encountered issues that we need workarounds for. etc etc.

Additionally, while we thought that with our same support of Concurrent, Suspense, and other "modes of operation" in React, like with SSR and react-ssr-prepass we could reduce our maintained code, this has proven to be false. The resulting code remained approximately the same.

In summary, any benefit we thought we'd get, we didn't. Meanwhile, we were observing regressions in our implementation.

How did we respond?

To address an influx of issues that people encountered, we reverted the PR that implemented this change, #2227, and are back on our old implementation of hooks like useQuery.

This is an easy compromise for us to make. As users, you'll notice no improvement in the (broken) 2.1.3 release. In fact, the intention was for both implementations to work identically.

Hence, we decided to revert and 2.2.0 is the new safe version.

Why did this happen in the first place?

For two reasons.

We thought that useSyncExternalStore may actually reduce the amount of issues we'll encounter in React 18. While this wasn't the case (and we would not be alone with keeping a store-like implementation that isn't using it), we still maintained that it would reduce what we'd have to maintain. This leads into the other reason.

Testing. The testing frameworks we use and the tests we maintain are old. They kept us from regressing on basic API behaviour. However, as we encountered more edge cases relating to Strict Mode, Suspense, and Concurrent Mode, we found that they're impossible to test for in the test setup we have. In fact, many of these issues couldn't even happen in preact/compat for example, which shows that they only happen in certain conditions and special modes in React.

A lot of issues that we've regressed on and that are associated with edge cases are: only observable in certain conditions, only trigger warnings, or are easily spotted with specific, custom-made test apps.

In short, we don't have browser-based tests and even if we did, those tests are hard to write and haven't been written. They'd even take us a while to write as we struggle to keep a list of everything we've encountered so far 😅

This made us rethink and ultimately revert.

What does this mean to me?

Please upgrade to [email protected] if you're affected and on [email protected]. Otherwise, skip the affected minor releases.

If you aren't using the React bindings, you'll be fine. If you are affected, also make sure that any downgrade, if you choose to downgrade that is, doesn't downgrade @urql/core so you're not missing out on patches.

We'll have to think how to go from here, possibly, with more tests and starting our testing approach for Reac from scratch. This is a bit of a shame, and we can't exactly tell whether we'll be successful in finding a test setup that works.
However, we're pretty confident in the stability of our previous implementation. It's proven stable, we've iterated on it, and there are no known issues in regards to its Concurrent, Strict Mode, Suspense, or React 17/18 compatibility.

Thank you for reporting the regressions to us! ❤️

[nandorojo]

Should this still be happening with [email protected] ([email protected])? I have two components calling the same query which appears to still lead to this warning. Wish it were easier to repro, but I can try to if this is unexpected...

The cause is from:

<Sidebar />:

const [me] = useMeQuery({
  pause: !user
})
const [artistPublic] = useArtistPublicQuery({
  variables: { slug },
  pause: me.data?.approved
})
const [artist] = useArtistQuery({
  variables: { slug },
  pause: !me.data?.approved
})

const artistData = artist.data || artistPublic.data

<SellersInviteOnlyScreen />:

const [me] = useMeQuery({
  pause: !user
})

[Svarto]

@nandorojo did you find a solution? I have the same issue in my app, switching between two pages and have a cursor based pagination query that updates the same local cache list of user posts - when switching between pages the cursor updates and thus "updates" the local cache on the previous page.

@kitten Should we just silence this Warning or do something about it? It started popping up recently, updated to the latest urql/core 3.0.1 and urql 3.0.1. Or do I need to manually pause the useQuery hooks when navigating away from a screen?

[nandorojo]

I'm still seeing the warning. React Native seems prone to it since previous screens stay mounted when you navigate around. I've come to ignore it.

[kitten]

@Svarto It's been a couple of versions since this, so to basically summarise, in 2.2.3 we found a separate issue with mounting updates that were scheduled incorrectly.
However, that means that if we're still seeing these warnings in React Native by now and you're not seeing any other issues, that it's perfectly safe to ignore them.

The reason I'm pretty sure about that is that the issue we resolved in that version was a removal of a workaround (which was a little leaky in even catching the edge case) which actually introduced faulty behaviour.
However, since that's been fixed we're pretty sure that in React, no other edge cases remain. So while I can't say when this warning is issued in RN, in theory our update timing should be correct in 2.2.3 || ^3.0.0

[Svarto]

Thanks a lot both for the quick replies, it's now clear! @kitten @nandorojo

[DanConway]

@kitten, @nandorojo, @Svarto we were having this issue in React Native too.

The workaround we came up with was to add a client-side parameter to the query (e.g. pageName) that doesn't do anything other making URQL treat them as different queries so it won't update the state of the query result on page1 when executing it gets the result of the query on page2.

export type CustomVariables = QueryVariables & { pageName: string };
export function useCustomQuery(vars: CustomVariables) {
  return useQuery<QueryResult, CustomVariables>({ query, variables });
}

[nandorojo]

That doesn't sound like a bad idea, albeit a hack. However, it looks like I'm now getting this error for a single component, so I'm a bit lost.

[nandorojo]

From what I've seen, this is just a warning and is apparently safe to ignore. So I may do that with YellowBox. I get it all the time in my native app (but basically never on Web) when using the same query in many places, such as useQuery(myAccountDocument).

[jacobjove]

So it seems like there is still no good resolution for the Cannot update a component ... while rendering a different component ... error? Making urql handle the queries separately (by adding different params) does not seem like a great solution, unless I'm misunderstanding something.

FWIW, I believe the React team does not consider this to just be a warning. It is intended to point out buggy code that should be fixed, which is why it is an error-level log rather than a warning-level log.

[r614]

We've been getting this error quite a bit when reusing the same query in multiple components, is there a way to de-dup these queries, perhaps by passing in an optional queryKey rather than creating different query objects? I don't think its a good pattern and we have like 3+ duplicated queries that fetch basically the same fragments in certain cases

[nandorojo]

you might want to consider just having a provider at the root of the app for the queries that passes them down. the downside of this is that it won’t auto-trigger refetches if that’s what you want.

[r614]

One workaround we are doing atm in a particular usecase is wrapping query refetch calls in a forceRefetch function in a top level Provider, which is then called in some downstream component. However, we want try and move away from this to as many standalone components as possible, since that pattern only works in extremely specific cases and we do want to auto-trigger refetches