React 18 - infinite loop and function as child issue with transition and suspense with useMemo

[joshribakoff-sm]

I've turned off strict mode, and tried to create a simple example that breaks in v18 rc3.

I'm trying to use useMemo to detect when some state changes and create a new memoized promise. Another useMemo call detects when the promise changes and wraps it in a "resource" object. I am passing the resource down to a child. There are multiple suspense boundaries between where I am creating the "resource" and where I'm calling .read(). I expect this to load once with no errors, and when I click the button I expect a single transition. Instead, although the app loads I get this error react-dom.development.js:86 Warning: Functions are not valid as a React child. This may happen if you return a Component instead of <Component /> from render. Or maybe you meant to call this function. When I click the button, it then goes into an infinite loop. This is unexpected because <App> should not be suspending, only <Child>. I'm also not returning any components without calling them from what I can tell.

If I set the "resource" into state it works fine. This only seems to happen if I try to create the "resource" with useMemo.

Minimal reproduction case

import {
  startTransition,
  Suspense,
  useRef,
  useEffect,
  useState,
  useMemo,
} from "react";
import logo from "./logo.svg";
import "./App.css";

function App() {
  const ref = useRef(0);
  const [validQueryParams, setValidQueryParams] = useState();

  const promise = useMemo(() => {
    console.log("making promise that resolves in 3s", ref.current);
    ref.current++;
    return new Promise((res) => {
      setTimeout(() => {
        console.log(validQueryParams);
        return res();
      }, 3000);
    });
  }, [validQueryParams]);

  const query = useMemo(() => {
    console.log("wrapping promise in resource object");
    return wrapPromise(promise);
  }, [promise]);

  useEffect(() => {
    console.log("mount");
    return () => console.log("unmount");
  }, []);

  console.log({ query });
  return (
    <>
      <Suspense fallback={() => <div>fallback</div>}>
        <Child query={query} />
        <button
          onClick={() => {
            startTransition(() => {
              setValidQueryParams(Math.random());
            });
          }}
        >
          Start transition
        </button>
      </Suspense>
    </>
  );
}

function Child({ query }) {
  if (!query) return null;
  return (
    <Suspense fallback={() => <div>fallback</div>}>
      <div>{JSON.stringify(query.read())}</div>
    </Suspense>
  );
}

export default App;

export function wrapPromise(promise) {
  let status = "pending";
  let result;
  const suspender = promise.then(
    (r) => {
      status = "success";
      result = r;
    },
    (e) => {
      status = "error";
      result = e;
    }
  );
  return {
    read() {
      if (status === "pending") {
        throw suspender;
      } else if (status === "error") {
        throw result;
      } else if (status === "success") {
        return result;
      }
    },
  };
}

[gaearon]

This is your original example:

https://codesandbox.io/s/dazzling-cherry-5bsiv6?file=/src/App.js

I can reproduce this error:

Warning: Functions are not valid as a React child. This may happen if you return a Component instead of <Component /> from render. Or maybe you meant to call this function rather than return it.

Like the error says, the issue is that you are trying to render a function:

    <Suspense fallback={() => <div>fallback</div>}>

This line should be instead:

    <Suspense fallback={<div>fallback</div>}>

This is the version where this mistake is fixed (in both places where the original code had it):

https://codesandbox.io/s/vigorous-cloud-clwtvt?file=/src/App.js

I can observe that after clicking "Start transition", there is a log that repeats every three seconds. I assume this is the issue you are referring to as the loop. I'll need to look a bit more to understand why this doesn't work. However, in general, Suspense relies on having an external cache. If you're creating "resources" during render (whether with useMemo or useState), you will likely run into issues.

[joshribakoff-sm]

Thanks Dan.

Yes, fetch as you render is working great by following the code sandbox examples in the React docs.

We wanted to explore "fetch on render" which was where we got the infinite loop. We think there are sometimes use cases for both patterns, although we can only get one of them to work. Having an example of "fetch on render" would probably be very helpful for library authors, or if its a pattern that won't work, clarifying this would be great!

Thank you again!

[gaearon]

Even with "fetch on render", the intention is that both the pending Promise and the eventual result (after its completion) are stored outside of your components. Even if that Map is lazily filled instead of being created preemptively. This could be a manual sideways cache for now. (Of course, this poses tricky questions about invalidation.) That's how Relay Suspense integration works today. In the future, we'd like to build primitives to make this easy: reactwg/react-18#25.

[joshribakoff-sm]

I assumed that to satisfy the requirement of storing the Promise "externally", it would suffice to do so above the suspense boundary. I was expecting only the child to suspend and not the parent, and if the parent does not suspend I am confused why it would lose it's state. In either case, working examples would be very helpful! I'll review the linked discussion, thank you for that context as well.

[gaearon]

Here is a smaller example of the same loop:

https://codesandbox.io/s/serverless-wave-469uso?file=/src/App.js

The issue here is that when you suspend and React doesn't have anything useful to commit (because you're in a transition, so there is no fallback loading state to show), the whole result of that render gets thrown away. This includes anything that happened during render, no matter how high in the tree. This includes any new memoized results. So useMemo — even in the parent component — doesn't get a chance to actually "get saved". It's like this useMemo never ran.

I assumed that to satisfy the requirement of storing the Promise "externally", it would suffice to do so above the suspense boundary

This can work in a limited way, but any information you rely on has to be calculated outside of render. This is why storing a resource in state works (it won't get dropped) but creating a resource during render doesn't (it will get dropped).

[gaearon]

So, to sum up the recommendation:

• Generally, this is why Suspense is meant to work with a cache that lives outside components. Either hand-rolled like Relay or, in the future, using something like Built-in Suspense Cache reactwg/react-18#25.
• As a compromise, it's possible to keep resources in state, but they have to be (1) separated with a Suspense from all components reading them, (2) created outside of rendering and stored in state instead of derived during render.

[joshribakoff-sm]

Thank you, it feels great to get such a concise and quick response. I was very keen to understand the "rules" here, and now I understand perfectly. This will be very useful context to disseminate once the release is stable and the docs are updated on the React site!

[gaearon]

We think it’s still pretty hard to do a hand-rolled cache correctly. It’s not clear what’s the right place to document it until the built-in Cache is available. We don’t want to give an impression that now is the best time to try to write a Suspense integration for a data fetching library.

[joshribakoff-sm]

Totally makes sense. For now we decided to use the "compromise" solution you mentioned, so we're setting the "resource" into state above the suspense boundary and this works perfectly 👨‍🍳

We're only using it one place, rendering a bunch of expensive SVG, so we're using an escape hatch since it's so impactful for our UX :)