Use a single global infinite promise

[gaearon]

It's not super clear why the code is written this way, but I assume it's due to not being familiar with the pattern. If you want to wait until the freeze prop changes, it's enough to throw a Promise that never resolves.

This fixes an issue that would occur on initial mount with createRoot and/or Fabric. The issue is that React throws away uncommitted trees if they suspend during mount, and this includes throwing away useRef. So stashing Promises in useRef is wrong. Having one at the top level is both safe and simpler.

[kmagiera]

Thanks for sending the PR. I don't remember if we'd ever consider doing this in proposed way and as you suggested it is likely due to the fact we weren't familiar with this pattern.

As for the issue with initial mount that you mentioned, is it reported somewhere?

As for the actual change, one thing that I wonder is whether this could potentially result in memory leaks? My understanding is that what React does under the hood, is it calls .then on the promise that gets thrown. On the other hand, the promise object has to retain all the callbacks passed via .then. If there is just one global promise it'll keep all the callbacks forever therefore leaking memory. This problem may have limited impact depending on what objects these callbacks retain, but IMO it is possible that they may capture some React internal data structures (old fiber trees?) that may get very big in some apps.

[kmagiera]

I wonder if we could just override then to be a noop and if that'd suffice

[gaearon]

As for the issue with initial mount that you mentioned, is it reported somewhere?

No. I don't mean that it's an actual issue that shows up today. I meant that with a Concurrent Root (on the web, this corresponds to createRoot, on RN it should be enabled or at least opt-in with Fabric), useRef does not get preserved between suspended mount attempts. In general useRef or useState are never safe places to keep a Promise cache. Promise cache should always be outside the thing that may suspend.

As for the actual change, one thing that I wonder is whether this could potentially result in memory leaks? My understanding is that what React does under the hood, is it calls .then on the promise that gets thrown. On the other hand, the promise object has to retain all the callbacks passed via .then. If there is just one global promise it'll keep all the callbacks forever therefore leaking memory.

Hm, interesting! Next.js does use this pattern, but I'm not sure if this is a concern.

I wonder if we could just override then to be a noop and if that'd suffice

Yes, that should work fine. (On the React side, it's how a built-in version of this will be implemented.)

[gaearon]

Pushed a fix.