Fetcher stuck in "loading" state when using `Await` with `Promise.all`

[jmezzacappa]

What version of Remix are you using?

1.19.3

Are all your remix dependencies & dev-dependencies using the same version?

• [X] Yes

Steps to Reproduce

Here's a reproduction on CodeSandbox: https://codesandbox.io/p/sandbox/remix-await-fetcher-bug-xg58yq?file=/app/routes/test-defer.tsx

• Use <Await> with Promise.all to await multiple promises from deferred loader(s).
• Submit a form with useFetcher.

import { defer } from "@remix-run/node";
import { Await, useFetcher, useLoaderData } from "@remix-run/react";
import { Suspense, useMemo } from "react";

function wait(timeMs: number) {
  return new Promise<void>((resolve) => setTimeout(resolve, timeMs));
}

export async function loader() {
  return defer({
    instant: "hi",
    deferred1: wait(1000).then(() => ({ foo: "bar" })),
    deferred2: wait(2000).then(() => ({ baz: "quux" })),
  });
}

export async function action() {
  await wait(1000);
  return null;
}

export default function TestDeferPage() {
  const data = useLoaderData<typeof loader>();
  const fetcher = useFetcher();

  const { deferred1, deferred2 } = data;
  const promises = useMemo(
    () => Promise.all([deferred1, deferred2]),
    [deferred1, deferred2]
  );

  return (
    <div>
      <p>Instant data: {data.instant}</p>

      <Suspense fallback="loading...">
        <Await resolve={promises}>
          {([data1, data2]) => (
            <div>
              <p>foo: {data1.foo}</p>
              <p>baz: {data2.baz}</p>
            </div>
          )}
        </Await>
      </Suspense>

      <p>Fetcher state: {fetcher.state}</p>

      <fetcher.Form method="post">
        <button type="submit">submit</button>
      </fetcher.Form>
    </div>
  );
}

Expected Behavior

The fetcher should go through the normal submitting/loading states, then eventually return to idle.

Actual Behavior

• The fetcher gets stuck in the "loading" state.
• The browser tab starts using lots of CPU.

[brophdawg11]

Hm, I'm not sure what's going on here. It's getting into a loop on a new Promise.all creation each render. However, deferred1/deferred2 should be stable across re-renders so the useMemo should work as expected and stabilize the Promise.all.

This seems to check out as well if compare them across re-renders, but the useMemo still runs even though they are the same instance, and this produces a new Promise.all causing the loop.

if (typeof window !== "undefined") {
  if (window.promises == null) { window.promises = []; }
  if (window.count == null) { window.count = 0; }
  if (window.promises?.length > 1) {
    console.log(
      "calling useMemo, deps equal?",
      deferred1 === window.promises[window.promises.length - 1][0],
      deferred2 === window.promises[window.promises.length - 1][1]
    );
  }
}

const promises = useMemo(() => {
  console.log("inside useMemo - creating new promise via Promise.all()");
  if (typeof window !== "undefined") {
    window.promises.push([deferred1, deferred2]);
    if (window.count++ > 50) { throw new Error("Max iterations reached"); }
  }
  return Promise.all([deferred1, deferred2]);
}, [deferred1, deferred2]);

Output:

I'll dig into this a bit deeper, but in the meantime I would either move the Promise.all into the loader if you want them to display together, or you can avoid the Promise.all and put both Await's inside the same Suspense to achieve the same UI.

<Suspense fallback="loading...">
  <Await resolve={deferred1}>{(data1) => <p>foo: {data1.foo}</p>}</Await>
  <Await resolve={deferred2}>{(data2) => <p>baz: {data2.baz}</p>}</Await>
</Suspense>

[jmezzacappa]

@brophdawg11 thanks for looking into this!

We usually do use Promise.all only in loaders, when possible. In this case, we have three promises for deferred data in multiple loaders in nested routes, and a few of our pages need to combine this data to render a view.

I forgot to mention an important detail: I just encountered this bug when updating from Remix 1.16.1 to 1.19.3. Everything worked fine in 1.16.1.

[wmdmomo]

@jmezzacappa When I upgraded to remix1.19.3, I also encountered the same problem. Finally, I used Suspense for nesting.

[wking-io]

@brophdawg11 this is still happening in v2.3.1

[brunocascio]

By removing Promise.all() it resolved my issue. I've just replaced resolve={Promise.all([promise1, promise2])} with resolve={[promise1, promise2]} and adding await within the defer() function

export const loader = async ({ params }: LoaderFunctionArgs) => {
  return defer({
    template: await templates(),
    items: await templateItems(),
    teams: await teams(),
  });
};

export default function TemplateDetail() {
  const { template, items, teams } = useLoaderData<typeof loader>()
  return (
    <>
      <Suspense fallback={'loading...'}>
        <Await resolve={[items, teams]}>
          <TemplateItemsList />
        </Await>
      </Suspense>
    </>
  );
}

NOTE: I'm using remix 2.5.0

[jmezzacappa]

By removing Promise.all() it resolved my issue. I've just replaced resolve={Promise.all([promise1, promise2])} with resolve={[promise1, promise2]} and adding await within the defer() function

export const loader = async ({ params }: LoaderFunctionArgs) => {
  return defer({
    template: await templates(),
    items: await templateItems(),
    teams: await teams(),
  });
};

export default function TemplateDetail() {
  const { template, items, teams } = useLoaderData<typeof loader>()
  return (
    <>
      <Suspense fallback={'loading...'}>
        <Await resolve={[items, teams]}>
          <TemplateItemsList />
        </Await>
      </Suspense>
    </>
  );
}

NOTE: I'm using remix 2.5.0

🤔 If I'm understanding the code correctly, that loader won't send a response until after all of those promises are resolved, right? In that case, there's not really any need for defer, Suspense, or Await, as a simple json() response would perform similarly.

Unfortunately, that wouldn't be a viable workaround in our case.

Off-topic performance suggestion

If I may make a suggestion, in case you have a loader like that in production...

That loader fetches data sequentially (i.e. waiting for one request to finish before starting the next). It could be significantly faster to run those async functions in parallel with Promise.all, like this:

export const loader = async ({ params }: LoaderFunctionArgs) => {
  const [templatesResponse, itemsResponse, teamsResponse] = await Promise.all([
    templates(),
    templateItems(),
    teams(),
  ]);
  return json({
    template: templatesResponse,
    items: itemsResponse,
    teams: teamsResponse,
  });
};

[jmezzacappa]

As a temporary workaround, I threw this component together. Feels like the types could be simplified a bit, but I didn't bother refactoring once I got it working, as it's just a (hopefully) temporary solution.

import { Await } from '@remix-run/react'
import { type ComponentProps } from 'react'

type AwaitProps = Omit<ComponentProps<typeof Await>, 'children' | 'resolve'>

type AwaitAllRecursiveProps<TResolved, TPending, TAllValues> = {
	awaitProps: AwaitProps
	children: React.ReactNode | ((resolvedValues: TAllValues) => React.ReactNode)
	pendingValues: TPending
	resolvedValues: TResolved
}

type AllValues<
	TResolved extends readonly unknown[] | [],
	TPending extends readonly unknown[] | [],
> = [...TResolved, ...{ [K in keyof TPending]: Awaited<TPending[K]> }]

function AwaitAllRecursive<
	TResolved extends readonly unknown[] | [],
	TPending extends readonly unknown[] | [],
	TAllValues extends TResolved | readonly unknown[] | [] = AllValues<
		TResolved,
		TPending
	>,
>({
	awaitProps,
	children,
	pendingValues,
	resolvedValues,
}: AwaitAllRecursiveProps<TResolved, TPending, TAllValues>): JSX.Element {
	if (pendingValues.length === 0) {
		return (
			<>
				{typeof children === 'function'
					? children(resolvedValues as TAllValues)
					: children}
			</>
		)
	}

	const [pendingHead, ...pendingTail] = pendingValues

	return (
		<Await {...awaitProps} resolve={pendingHead}>
			{value => (
				<AwaitAllRecursive
					awaitProps={awaitProps}
					pendingValues={pendingTail}
					resolvedValues={[...resolvedValues, value]}
				>
					{children}
				</AwaitAllRecursive>
			)}
		</Await>
	)
}

type AwaitAllProps<T extends readonly unknown[] | []> = AwaitProps & {
	resolve: T
	children:
		| React.ReactNode
		| ((resolvedValues: {
				-readonly [K in keyof T]: Awaited<T[K]>
		  }) => React.ReactNode)
}

/**
 * Wrapper around Remix's {@link Await} component to handle multiple promises.
 *
 * This was created because the `Await` component tends to introduce strange
 * [bugs] when used with `Promise.all`, and the bugs vary depending on the Remix
 * version. For example, after upgrading from Remix 1.16.1 to 1.19.3, fetchers
 * would get stuck in the "loading" state when an `Await` component is used
 * somewhere else on the page with `Promise.all`.
 *
 * This component has the same API as Remix's `<Await>`, except the `resolve`
 * prop is an array of promises (similar to `Promise.all`).
 *
 * [bugs]: https://github.com/remix-run/remix/issues/7392
 *
 * @example
 *
 * // This component appears to work fine, but `useFetcher` gets buggy in
 * // other components.
 * function BadComponent() {
 *   const { deferred1, deferred2, deferred3 } = useLoaderData()
 *
 *   const allPromises = useMemo(
 *     () => Promise.all([deferred1, deferred2, deferred3]),
 *     [deferred1, deferred2, deferred3],
 *   )
 *
 *   return (
 *     <React.Suspense>
 *       <Await resolve={allPromises}>
 *         {([data1, data2, data3]) => (
 *           <p>This breaks fetchers.</p>
 *         )}
 *       </Await>
 *     </React.Suspense>
 *   )
 * }
 *
 * // To work around the issue reliably, `<Await>` can be nested.
 * function UglyComponent() {
 *   const { deferred1, deferred2, deferred3 } = useLoaderData()
 *   return (
 *     <React.Suspense>
 *       <Await resolve={deferred1}>
 *         {data1 => (
 *           <Await resolve={deferred2}>
 *             {data2 => (
 *               <Await resolve={deferred3}>
 *                 {data3 => (
 *                   <p>This is a very verbose workaround.</p>
 *                 )}
 *               </Await>
 *             )}
 *           </Await>
 *         )}
 *       </Await>
 *     </React.Suspense>
 *   )
 * }
 *
 * // `<AwaitAll>` renders the nested `<Await>`s for us.
 * function NoNestedAwait() {
 *   const { deferred1, deferred2, deferred3 } = useLoaderData()
 *   return (
 *     <React.Suspense>
 *       <AwaitAll resolve={[deferred1, deferred2, deferred3]}>
 *         {([data1, data2, data3]) => (
 *           <p>This does not break fetchers.</p>
 *         )}
 *       </AwaitAll>
 *     </React.Suspense>
 *   )
 * }
 */
export default function AwaitAll<T extends readonly unknown[] | []>({
	resolve: pendingValues,
	children,
	...awaitProps
}: AwaitAllProps<T>): JSX.Element {
	return (
		<AwaitAllRecursive
			pendingValues={pendingValues}
			resolvedValues={[]}
			awaitProps={awaitProps}
		>
			{children}
		</AwaitAllRecursive>
	)
}

(Tested with Remix 1.19.3 and 2.3.1)

[brophdawg11]

I spent a bit of time looking into this again today - as far as I can tell it's something about throwing a promise to a suspense boundary and that causing the useMemo not to see the "prior" value (the one from the render in which we threw a promise). It's reproducible outside of Remix/RR with a completely stable promise in the dep array. FWIW, it also looks like it is not an issue using React.use() instead of Await in a v19 canary.

https://codesandbox.io/p/sandbox/epic-fire-v47w53?file=%2Fsrc%2Findex.js%3A33%2C6