urql icon indicating copy to clipboard operation
urql copied to clipboard

fix(react-urql): Upgrade muted warning code for React 19 internals

Open kitten opened this issue 9 months ago • 3 comments

Resolves #3764

Summary

Same as before: the warning is irrelevant, and we're looking to mute it. We can make the deferred dispatching permanent, if we don't want to rely on internals, but this PR currently keeps it the same and just updates how we access internals.

These lines show how this internal has changed: https://github.com/facebook/react/blob/b286430c8a585dc2e2e3cc023e7c455ec2b34ab7/packages/react/src/jsx/ReactJSXElement.js#L52-L61

As before, just because the warning is issued that doesn't mean anything is wrong. We're only doing this to mute this warning.

[!WARNING] I haven't tested this yet, at all, but it should work fine.

Set of changes

  • Update how ReactCurrentOwner is accessed using new internals

kitten avatar Apr 01 '25 09:04 kitten

🦋 Changeset detected

Latest commit: 6f964aed89fc709ced560d98d9e05725f4bc01fd

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
urql Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

changeset-bot[bot] avatar Apr 01 '25 09:04 changeset-bot[bot]

@kitten any timeline for when this PR will get merged and released? With Expo 53 being released this week there will be a lot of projects now on React 19 and getting this error.

ericpoulinnz avatar May 03 '25 11:05 ericpoulinnz

We use this pnpm patch to appy this PR before it merged

diff --git a/dist/urql.es.js b/dist/urql.es.js
index c2ef1c7e614e49bad3002c98893ccbc5ed2abc5b..0b5feec9a4631e3827389803d687b573b21c26a5 100644
--- a/dist/urql.es.js
+++ b/dist/urql.es.js
@@ -70,11 +70,21 @@ var hasDepsChanged = (e, r) => {
   return !1;
 };
 
-var p = r.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED;
+
+var p = r.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED || r.__CLIENT_INTERNALS_DO_NOT_USE_OR_WARN_USERS_THEY_CANNOT_UPGRADE;
 
 function deferDispatch(e, r) {
-  if ("production" !== process.env.NODE_ENV && p && p.ReactCurrentOwner && p.ReactCurrentOwner.current) {
-    Promise.resolve(r).then(e);
+  if (!!p && process.env.NODE_ENV !== 'production') {
+    const currentOwner = p.ReactCurrentOwner
+      ? p.ReactCurrentOwner.current
+      : p.A &&
+      p.A.getOwner &&
+      p.A.getOwner();
+    if (currentOwner) {
+      Promise.resolve(r).then(e);
+    } else {
+      e(r);
+    }
   } else {
     e(r);
   }

sep2 avatar May 14 '25 13:05 sep2

@kitten I've also just encountered this warning in a project.

I've created a failing test in https://github.com/urql-graphql/urql/pull/3817, maybe this can help to verify the change in this PR?

amannn avatar Aug 27 '25 13:08 amannn

@amannn Cheers! The problem isn't writing a test in this PR but that concurrent behaviour is hard to predict and unreliable in many tests. But I'm not very concerned about the changes in this PR, but what I'd like to merge is a different change instead; for context: https://github.com/urql-graphql/urql/issues/3764#issuecomment-2895936993

kitten avatar Aug 27 '25 13:08 kitten

@kitten Ok, I see! I'll keep an eye out for your other fix then.

FWIW when I copy your fix from this PR to my branch, then the test passes.

amannn avatar Aug 27 '25 13:08 amannn

@kitten I've patched urql with this change in my production app (a little over 10k MAU) for the last two months without encountering any issues.

I honestly don't understand how anyone using urql with the latest react doesn't do this. Unless I've implemented it incorrectly? The constant warnings made debugging an absolute nightmare.

ericpoulinnz avatar Aug 27 '25 18:08 ericpoulinnz