React 18 - waterfall suspensions re-trigger the Suspense boundary

[arackaf]

I'm seeing some baffling behavior with React 18 in concurrent mode. The minimal repro is below. The tl;dr is this:

I have a single Suspense boundary at the root of the application. The fallback turns the screen pink (so you know it's there).

Problem 1

I load some data with a Suspense hook. When the data come back, and render, subsequent Suspensions will happen, since I have a SuspenseImg component which suspends while each image preloads.

Expected Behavior

The fallback should show until the data come in, and all of the subsequent image preloads are done.

Actual Behavior

The fallback shows, then the page renders briefly, without data, and then re-suspends while the images preload.

---

Problem 2

There's a button which loads more data, using startTransition. The data load re-triggers the same suspension, and the new data cause the same suspensions when rendered, via the same SuspenseImg (I force it to suspend even though I'm loading the same 5 images over and over).

Expected Behavior

The loading boolean from useTransition should show until the data are returned, and all of the images have pre-loaded

Actual Behavior

The loading boolean shows while the data are loading, and then the main fallback shows while the images preload.

---

I'll assume this isn't a bug, and that I instead have misunderstood how this works. I had thought Suspense was supposed to make this stuff fire and forget, with React keeping the appropriate fallback / loading boolean set until the entire state has been set, and everything rendered without anything suspending. But it seems I've misunderstood something?

---

https://codesandbox.io/s/suspense-debug-pz5mz?file=/src/useQuery.js

[arackaf]

Update: the incomparable, peerless, magnificent Rich Harris actually figured out what was "wrong."

It seems setting state inside of the useEffect effectively broke / interrupted / completed the initial suspension, and when the image preloads happened, things re-suspended. So on initial load, the pink screen showed twice, and on incremental data loading, the pink screen flashed after the loading boolean went back to false.

This sandbox shows the updated code, but of course I had to remove the infinite list (or whatever it's called). I'm just showing new data as it comes in, discarding the old. I'm guessing the correct way to implement what I had originally is with some clever use of useMemo, to piece together the growing list from new data?

I hope this behavior isn't final, and that state changes from within useEffect will eventually combine with the initial suspension, otherwise there will be a lot of surprised and disappointed users out there.

https://codesandbox.io/s/suspense-debug-2-swqgd?file=/src/DataList.js

[sebmarkbage]

Yes this behavior is final. In fact delaying the effects is a huge part of this feature. Please reserve judgement about how confusing this will be until you have read the docs for the feature that will be available when it’s actually released.

[arackaf]

Thanks for the reply @sebmarkbage - will there be any new api's for setting state after some piece of state changes? Ie, I'm honestly stumped on how to implement an infinite list, that grows when new data come in (without setting state in an effect, or writing to a ref in render) - ie a suspense-friendly version of the first sandbox.

[sebmarkbage]

Hint: You don’t store data in state. You store what pages are currently visible and fetch the data in a loop. That way you are also able to refresh them automatically.

[arackaf]

@sebmarkbage ah well that's less a hint vs the entire solution :)

[arackaf]

@sebmarkbage ohhhh I think I see now why (iirc) Relay has an imperative api, ie something like

relayAPI.read()

vs a hook like useQuery();

Here, as you said, you'd loop over your pages, and call .read() (which wouldn't work for a hook).

Lastly, am I correct in assuming you could put this behind a useMemo call? Ie, is it valid to Suspend / throw a promise inside of the useMemo callback? I'd assume so but I'm hesitant to assume anything here :)

[gaearon]

You might want to have a look at this: reactwg/react-18#25. Having a backing cache (not state/memo) is pretty central to the design of this feature. The data that you suspend on is meant to be stored there, not in the state. That’s also why effects aren’t relevant to the picture — the refetching happens due to reads and explicit refetch calls (which clear the cache). Relay doesn’t work like this yet, so it approximates this with its own cache, but the goal is to make this a primitive that library developers can use and rely on.

[arackaf]

@gaearon thanks Dan. I did my best to understand that post. It sounds like there's two caches? The underlying cache coming from your data provider, ie your GraphQL client, or similar. And then another, higher level cache on top of that, which you all are in the early stages of providing?

I'm having a bit of trouble seeing how that would fit in, here, to this use case, ie an infinite list, where the user may have clicked "show more" 3 times?

Sebastian clarified quite a bit in mentioning that only you'd only store the current "lastKey" values, which are used for each of those queries.

So on render, I could easily just call dataProvider.read() N times, get my cached data, and put the results together. But those array concatenations are certainly not free. It might be premature optimization, but I had wondered if I could put that stuff into a useMemo call. I had assumed the read() calls in the useMemo callback were allowed to suspend, but now it seems like the proper solution is to use a secondary cache, which sits atop my GraphQL cache?

Am I understanding this correctly at all?

[gaearon]

But those array concatenations are certainly not free

Hmm. This sounds like a very minor concern unless we’re talking about thousands of items on every render. Whenever you map() JSX you also create arrays. Is there some particular perf issue you are worried about here? I think I’m missing something.

[gaearon]

The underlying cache coming from your data provider, ie your GraphQL client, or similar. And then another, higher level cache on top of that, which you all are in the early stages of providing?

Yeah.

I'm having a bit of trouble seeing how that would fit in

The key part i was referring to is just that you wouldn’t rely on anything component-level (state/memo) for “stability”. When we suspend before mount we throw away the tree. Storing data (and Promises) in a cache is what makes your code resilient to that. Because it’s ok to recreate the tree later and that won’t cause a refetch or a cache miss.

[gaearon]

but now it seems like the proper solution is to use a secondary cache, which sits atop my GraphQL cache?

I think this depends on what you’re trying to cache. The “stitch together a few pages” use case doesn’t seem like it requires any sort of caching to me. If you’re concerned about concatenation itself you can read all pages that you need first and then useMemo the expression that concats them — but again — I’d be very surprised to see a bottleneck of a perf issue here for any realistic pagination use case. In general, I think that yes — we would provide an extra Cache level on top of yours to model consistency guarantees and invalidation within the tree. But that’s not ready yet and I don’t think that’s what you’re asking about.

[arackaf]

@gaearon thanks Dan I really appreciate the thorough explanation. I suppose you're right about the array allocations, and in any event, it's good to see useMemo would still be a suitable escape hatch. Reading the cached data for N pages is all but constant, and then concat'ing them all up should be trivial, as you said.

I've been doing some C++ at work lately and sometimes I forget to switch off the paranoia about allocations and such.

I still don't quite see how or where that secondary cache is supposed to live, but I'll look forward to some docs when it's done.

[arackaf]

@gaearon oh wait. I might understand where that secondary cache fits in. So with this use case, where we’re pulling more and more data from Dynamo, each time by passing a new lastPageKey or whatever. I’d store that array of keys in state, like Seb said, and read each cached page on render (or suspend if not in cache)

But I could also store that array of keys in this new cache you’re working on, this way if the user clicks to a new page, and then hits the back button, I could load the same data they were just viewing, right? Is that roughly how it’s supposed to fit in?