Adding React suspense + data fetching support

[hwillson]

Broken out from: https://github.com/apollographql/apollo-client/issues/8245

Now that @apollo/client 3.6 is out with updated React 18 support, this issue will track Apollo Client's React suspense + data fetching support (coming in 3.7).

[capaj]

@hwillson I am slightly confused-is react supporting suspense for data fetching? Looking at the docs: https://reactjs.org/docs/react-api.html#reactsuspense

It's not mentioned anywhere. They only explicitly list these three usecases:

• code splitting
• server side rendering
• hydration

Although looking at how urql team handles it-it seems that just throwing a promise anywhere should be enough for suspense to just work, am I correct in this assumption?

[SimenB]

https://reactjs.org/blog/2022/03/29/react-v18.html#suspense-in-data-frameworks

[capaj]

@SimenB thanks that explains it well enough.

[pkellner]

Does this issue imply that there will be a data fetching solution when using Suspense in React 18 for Apollo? That is, something like useQuery will be updated (though I think there needs to be support beyond useQuery). I've attempted to start a discussion here

[jpvajda]

@pkellner Just an update on the use of Github Discussions in this repo, we are actually disabling the use of Discussions on May 18th, so if you want just keep your comments on this issue, that works well for us. We are happy to respond when we can. 😄

As a note: Apollo has centralized discussions over in our Community Forum so we can better monitor them and respond across our many OSS projects. Feel free to join the community if you find value in it.

[benjamn]

@pkellner We can definitely keep discussing here, to be clear!

And yes, we do plan either to provide additional suspenseful versions of the relevant hooks, or (if @capaj's question above turns out in our favor 🤞), then perhaps we can just make the existing hooks return (for example) useQuery(...).suspensePromise, which would be the same Promise that would be thrown during suspense, and if you call useQuery(query, { suspend: true }) it automatically throws that suspensePromise, or something along those lines. On the other hand, a thin wrapper around useQuery might be adequate too.

[pkellner]

@benjamn glad to hear from you on this. I totally get how the simple sample works on the React Blog works with Suspense. That is, throwing the status, or returning the result. I also believe, based on @gaearon comment here, that it is unlikely that useQuery is able to support Suspense. That is, Dan says in this issue "We're not sure whether ad-hoc helpers like useSWR will ever be recommended to use with Suspense" making me think there must be a lot of locking/race type issues that need to be addressed, well beyond simple throws in a wrapped promise.

I really hope you at Apollo figure out how to make Suspense work with data. You are are in a perfect position with your well tested, well documented, and fully supported library to bring Suspense to lots and lots of people.

And also, @jpvajda , thanks for the note about discussions being disabled. I did not realize that and I will continue this discussion here.

[MarkLyck]

@hwillson and @benjamn Thanks for creating this public issue and commenting.

I was just about to start a huge painful migration from @apollo-client to another graphql client to get Suspense support, as apollo is the last library i have really preventing me from taking full advantage of React 18.

So happy to hear that Suspense hooks are coming!

[pkellner]

@hwillson and @benjamn Thanks for creating this public issue and commenting.

I was just about to start a huge painful migration from @apollo-client to another graphql client to get Suspense support, as apollo is the last library i have really preventing me from taking full advantage of React 18.

So happy to hear that Suspense hooks are coming!

Relay?

[MarkLyck]

@pkellner I was going to give gqty a try. I built a POC for it, It's amazing being able to just write the code as if it was an object and not have to think about queries at all. But I could not get a decent dynamic mocking system working since the queries don't have names. I would essentially have to rely on graphql-tools auto-mocking + using Type names for specific overrides. Another downside is no support for @defer or @stream directives. But graphql-mesh doesn't support that yet anyway 😞

with Apollo I use graphql-ergonomock for auto generated mocks, and overriding them for individual tests which just works great!

My backup would have been react-query or Urql (all 3 of those have had Suspense support for a long time), I've never really liked Relay, much prefer Apollo client.

[pkellner]

Thanks! Obviously mocking is very important to you. I get it.

Be very careful if libraries that claim proper Suspense support. I started this issue and Dan weighed in making it clear the only current properly working data library with Suspense is relay. Definitely SWR does not support Suspense correctly. They've talked about removing the option but I'm not sure why they have not.

https://github.com/vercel/swr/issues/1906#issuecomment-1099481998

[MarkLyck]

@pkellner Interesting 🤔 My app is client-side rendered, so I don't think this particular issue would affect my application.

I've been using react-query's implementation of Suspense in production for a while now and so far it has been working flawlessly.(Of course knowing that the final APIs could change at some point). But so far, no errors or any data issues with it.

But good to know in case I implement this on one of my SSR apps 😅

[pkellner]

I'll ping Dan, but I don't think it is a server side/client side issue. I think that Suspense is unstable in production on client side with anything but relay. Otherwise, they would endorse Suspense with CRA and they clearly don't do that.

[pkellner]

@MarkLyck Not sure if you are up for taking risks with your production but... Seems to me like it would be foolish to use Suspense on any kind of production web site until the React team endorses its use. Dan replied.. https://github.com/vercel/swr/issues/1906#issuecomment-1135166847

To be clear, I posted this to twitter and Dan responded here.

https://twitter.com/pkellner/status/1528852128160423936

[doflo-dfa]

We have been using the following pattern after split control approach (after admittedly trying a number of other apollo wrapper approaches .... this has proven by far the most predictable give the flux everything is in )

export const ControlName = ()=>{

const query = useQuery<any>(QUERY)

return <Suspense><ControlNameRenderer query={query} /></Suspense>

}

const ControlNameRenderer = (props: { query: QueryResult<any> })=>{

  if (props.query.loading) {
    throw Promise.resolve();
  }
  
  if (props.query.error) {
    throw props.query.error;
  }

  return <div>{props.query.data.value}</div>
  
}

default export memo(ControlName)

[laverdet]

We have been using the following pattern after split control approach (after admittedly trying a number of other apollo wrapper approaches .... this has proven by far the most predictable give the flux everything is in )

export const ControlName = ()=>{

const query = useQuery<any>(QUERY)

return <Suspense><ControlNameRenderer query={query} /></Suspense>

}

const ControlNameRenderer = (props: { query: QueryResult<any> })=>{

  if (props.query.loading) {
    throw Promise.resolve();
  }
  
  if (props.query.error) {
    throw props.query.error;
  }

  return <div>{props.query.data.value}</div>
  
}

default export memo(ControlName)

This will peg the CPU at 100% while any queries are loading.

[doflo-dfa]

I may have accidentally left out the promise helper we are using inplace of the generic Promise.resolve() ... cause I am dumb

ill post a better example

[doflo-dfa]

This will peg the CPU at 100% while any queries are loading.

Not the way I was originally doing it but ... what do you think about this ?

import { useQuery as _useQuery, } from '@apollo/client'

export type PromiseQueryResult<Q, V>  = QueryResult<Q, V> & { loadingPromise: Promise<Q> }

export const usePromiseQuery = <Q, V>(query: DocumentNode | TypedDocumentNode<Q, V>, options = {} as QueryHookOptions<Q, V>): PromiseQueryResult<Q,V> => {
    const reader = useRef(new Promise<Q>((good, bad) => {
        const _oc = options.onCompleted
        const _oe = options.onError
        options.onCompleted = (d) => {
            if (_oc) {
                _oc(d)
            }
            good(d)
        }
        options.onError = (e) => {
            if (_oe) {
                _oe(e)
            }
            bad(e)
        }
    }))
    return { loadingPromise: reader.current, ..._useQuery<Q,V>(query, options) }
}

...

export const ControlName = ()=>{

const query = usePromiseQuery<any>(QUERY)

return <Suspense><ControlNameRenderer query={query} /></Suspense>

}

const ControlNameRenderer = (props: { query: PromiseQueryResult<any> })=>{

if (props.query.loading) {
     throw props.query.loadingPromise;
}
  
  if (props.query.error) {
    throw props.query.error;
  }

  return <div>{props.query.data.value}</div>
  
}

default export memo(ControlName)

[laverdet]

@doflo-dfa it will have unpredictable behavior when query parameters change, tearing with concurrency mode, an abandoned promise on all renders after the first, and probably many other issues. It's a tough problem and there's a reason that this feature request has been open for a year. If this solution works for your team then I'm happy for you but I wanted to point out that the code you posted here may lead to problems in the future.

[doflo-dfa]

I totally appreciate that... and appreciate the comment ... we are rapidly working through at least having a solution that is workable as we try to release something that doesn't have to be completely gutted when everyone figures out how we are going to deal with this. The solution I posted has tested reliably in first render ... and seems to work reliably on observed change running the current alpha.. that being said, the closest thing I have seen to a real solution (https://reactjsexample.com/proof-of-concept-implementation-of-suspense-for-data-fetching-for-apollo/amp/) only works running the experimental build of 18 and i cant go that far.

[pkellner]

@doflo-dfa @laverdet and to pile on to the concern, Dan Abramov makes it clear that these adhoc type helpers should not be considered stable and he further goes on to say that the support for them will likely start in places like nextjs and not SWR or Apollo. https://github.com/vercel/swr/issues/1906#issuecomment-1135166847

[Negan1911]

@pkellner I think apollo is not an "ad hoc type helper" like SWR, but a data library like relay, so suspense should be possible as dan says here: https://github.com/vercel/swr/issues/1906#issuecomment-1099481998

[rajington]

Definitely outdated so not sure if it's relevant, or had it been mentioned now something like useSWR would be included, but in the v17 docs Apollo is mentioned directly.

I assume bc Apollo has that intermediate caching layer, it has the potential to be leveraged by React, but I would also assume that the integration doesn't come for free and that things are constantly evolving, case in point the v18 docs are very different.

I wish I understood more around the complexities and types of issues we might run into now. We experimented with Suspense for data fetching on a non-critical (yet highly trafficked) production page via (3rd-party deprecated) react-apollo-hooks more than 3 years ago! Then later we even PoC'd it with Next.js using react-async-ssr/react-ssr-prepass/getDataFromTree from early versions of React 18 experimental. In both cases, the components we were able to write were beautiful, which is what makes me excited about being able to use this more confidently throughout our sites. Likely we were not reaching some edge cases, but it'd be great to know if there's a certain type of data (maybe immutable, fetched once data) that it could work for.

[NTag]

Hello 👋

Do you have any update on this topic? Do you still plan to include it in 3.7 or will it be in a later release?

Thanks for the work!

[jpvajda]

@NTag Thanks for checking in on this feature. We have moved it to our 3.9 release due to other priorities that came up for the team recently. We are very interested in delivering support for suspense very soon to Apollo Client. So thanks for being patient. 🙏

[jamesmfriedman]

Wrote this today, surprisingly it works as a drop in replacement to useQuery with improved ergonomics.

• Use inside of a component in a React.Suspense component, works as expected
• data is no longer possibly undefined, so you can access it without the short circuit operator in your views.
• I didn't like any of the examples I found online of having to do data accessor like data.foo.read() so I'm using a proxy. Just access data like you normally would.
• If anyone is seeing something I'm not, please @ me, otherwise easier than I expected.

Implementation

import { useMemo } from 'react';

import {
  OperationVariables,
  DocumentNode,
  TypedDocumentNode,
  QueryHookOptions,
  QueryResult,
  useQuery,
} from '@apollo/client';

/**
 * This is a drop in replacement for Apollo's useQuery hook that works directly
 * with React.Suspense and has the improved ergonomics of `data` being non-nullable.
 */
export const useSuspenseQuery = <
  // eslint-disable-next-line @typescript-eslint/no-explicit-any
  TData extends any = any,
  TVariables extends OperationVariables = OperationVariables
>(
  query: DocumentNode | TypedDocumentNode<TData, TVariables>,
  options: QueryHookOptions<TData, TVariables> = {}
): Omit<QueryResult<TData, TVariables>, 'data'> & {
  data: NonNullable<QueryResult<TData, TVariables>['data']>;
} => {
  const { data, loading, error, observable, ...rest } = useQuery(query, options);

  const errorPolicy = options.errorPolicy || 'none';

  const promise = useMemo(() => {
    return new Promise((resolve) => {
      const resolver = () => {
        resolve(true);
        subscription.unsubscribe();
      };
      const subscription = observable.subscribe(({ data, loading }) => {
        data && !loading && resolver();
      });
    });
  }, [observable]);

  const proxy = useMemo(
    () =>
      new Proxy((data || {}) as object, {
        get: (target, prop) => {
          if (!Object.keys(target).length && loading) {
            throw promise;
          } else if (errorPolicy === 'none' && error) {
            throw error;
          }
          return target[prop as keyof typeof target];
        },
      }) as NonNullable<TData>,
    [data, loading, error, errorPolicy, promise]
  );

  return { data: proxy, loading, error, observable, ...rest };
};

Usage

function Example() {
  <React.Suspense fallback={'My Fallback'}>
    <MyComponent />
  </React.Suspense>
}

function MyComponent() {
  const { data } = useSuspenseQuery(MyQuery);
  return <>{data.someValue}</>
}

[pkellner]

I believe that the Facebook and react team will tell you that without underlying support for the promise completion in Apollo that suspense will not be reliable and it will be suggested not to use your solution in production. True @benjamn and @gaearon

[jamesmfriedman]

Maybe, there is no Apollo promise in this case, it's just queued off of the Apollo observable which is triggering a standard / memoized promise. Would love to know what I don't know though 🤓.

[pkellner]

As would I. Suspense and data is a very unclear story. I do know that straight forward solutions like yours proposed are in the category of problematic with suspense.

[Negan1911]

@pkellner not really, on the past we had apollo hooks with something similar to that, if the solution is reliable why it would cause problems?