How to deal with suspend?

[danielkcz]

I am mostly curious what was the initial motivation to have suspend = true by default already. There is a lack of information on how to actually use suspense for data fetching considering that it's planned for late 2019.

I just got burned by this badly when I forgot to set it false for one component deep down in the tree. I do have a Suspense way up in a tree to handle React.lazy. Suddenly when a query was rerun due to variables changing, everything got unmounted and fallback to top-level Suspense. It took me a while and some debugging to realize the culprit.

Do you have some actual examples of how to integrate it with react-apollo-hooks? Is it someone using it successfully already or is it everyone switching it off?

Let's consider a pretty simple scenario. A component that has useQuery and renders some markup with data from that query. Now with loading prop the decision what to show is done deeper in the tree. However, with suspension in play, it has to be done at top levels? I just cannot wrap my head around this for some reason.

[trojanowski]

I am mostly curious what was the initial motivation to have suspend = true by default already.

When I started implementation of this library there where no info when both hooks and suspense for data loading would be ready, but I thought that both of them could play nicely together. Right now we know that a stable release of React with hooks should be available soon and also it seems that the parts of suspense used by react-apollo-hooks can be considered as safe to use.

I just got burned by this badly when I forgot to set it false for one component deep down in the tree. I do have a Suspense way up in a tree to handle React.lazy. Suddenly when a query was rerun due to variables changing, everything got unmounted and fallback to top-level Suspense. It took me a while and some debugging to realize the culprit.

I think it's a quite common use case. You'd like to show a loading indicator if there is no all data or assets required for the current page already available. With suspense you can have the whole logic for that in a single place, e. g:

const MyLazyComponent = React.lazy(() => import("./AComponent"));

function ComponentWithData() {
  const { data } = useQuery(MY_QUERY);

  return <MyLazyComponent data={data} />;
}

ReactDOM.render(
  <Suspense
    fallback={
      <div>
        {"GraphQL data or lazy bundle loading (I don't care whitch one)"}
      </div>
    }
  >
    <ComponentWithData />
  </Suspense>
);

Without suspense, you'd have to support loading states separately in both MyLazyComponent and ComponentWithData.

Let's consider a pretty simple scenario. A component that has useQuery and renders some markup with data from that query. Now with loading prop the decision what to show is done deeper in the tree. However, with suspension in play, it has to be done at top levels?

Not necessary. You can put <Suspense /> anywhere in the tree. Please look at the following code:

const FirstLazyComponent = React.lazy(() => import("./FirstLazyComponent"));
const SecondLazyComponent = React.lazy(() => import("./SecondLazyComponent"));

function ComponentWithData() {
  const { data } = useQuery(MY_QUERY);

  return <FirstLazyComponent data={data} />;
}

function App() {
  return (
    <Suspense
      fallback={
        <div>
          This fallback will be shown during loading of GraphQL data or a bundle
          for "FirstLazyComponent"
        </div>
      }
    >
      <ComponentWithData />
      <div>
        <Suspense
          fallback={
            <div>
              This fallback will be shown during loading of a bundle for
              "SecondLazyComponent"
            </div>
          }
        >
          <SecondLazyComponent />
        </Suspense>
      </div>
    </Suspense>
  );
}

ReactDOM.render(<App />);

One thing to note. In order to avoid waterfall requests you have to do data preloading sometimes, e. g.:

const MyLazyComponent = React.lazy(() => import("./AComponent"));

function ComponentWithData() {
  // start preloading MyLazyComponent before suspending for GraphQL data
  import("./AComponent").then(() => {
    console.log("AComponent preloaded");
  });
  const { data } = useQuery(MY_QUERY);

  return <MyLazyComponent data={data} />;
}

ReactDOM.render(
  <Suspense
    fallback={
      <div>
        {"GraphQL data or lazy bundle loading (I don't care whitch one)"}
      </div>
    }
  >
    <ComponentWithData />
  </Suspense>
);

Please let me know if my examples are clear.

[danielkcz]

Thanks for the elaborate answer, but it's still confusing. It's exactly what I am fighting with. The component with useQuery is deeper in the tree and the logic for displaying "loading" state is above it. Furthermore when useQuery throws the Suspense replaces rendered stuff with a fallback, so everything just vanishes.

I have been finding such contrived examples everywhere, but it just doesn't make sense. Let's consider a page displaying some list of items on the left side and the google map with markers related to that list. When filter for that list is altered, it will rerun both queries. There should be some loading indicator for TheList, but TheMap should be still visible and update its markers when data arrive.

function TheMap() {
  // this component should never disappear when loading
  const { data } = useQuery(QUERY_FOR_MARKERS);
  return <GoogleMap>{makeMarkers(data)}</GoogleMap>;
}

function TheList() {
  const { data } = useQuery(QUERY_FOR_LIST);
  return <>
    <Filter />
    <List items={data.items} />
  </>    
}

ReactDOM.render(
  <Suspense fallback={<Spinner />}>
    <TheList />
    <TheMap />
  </Suspense>
);

Now I do realize I can render anything as a fallback, even the same components to display the same UI, but I would need to pass some loading prop to those anyway. So, in the end, it looks to me pretty much the same as without suspense and having loading prop available with query directly. Most likely it's some mindset issue, but I just don't see it.

  <Suspense fallback={
    <>
      <TheListIsLoading />
      <TheEmptyMap />
    </>
  }>
    <TheList />
    <TheMap />
  </Suspense>

Note that even this is bad because TheMap and TheEmptyMap are different component, React will throw away those DOM elements which will cause the whole google map refresh. It certainly is not pretty.

[gaearon]

FWIW lack of features like this is part of the reasons we don’t recommend anyone to start using Suspense for data fetching now. (The feature you’re asking for is essentially a render prop API for switching between fallback and main content. It’s planned but not ready yet.)

It doesn’t hurt to experiment with it but I think we should be clear that Suspense data fetching story (and patterns around it essential to creating good UX) is not ready for prime time.

[danielkcz]

Thanks for clarification @gaearon, that's what I thought :) @trojanowski Would you consider switching the suspend to false by default then? Are you honestly using it or switching it off everywhere as well? :)

[robertvansteen]

Any reason you don’t want to wrap <TheList /> in a fallback? Because that makes sense as you want to show the loading state independently there from the map.

I’m using it exactly in such way, I have some main spinners in the form of a fallback on certain positions in the tree and then depending on if multiple query components on a page should render independently of the loading state of another query I isolate them with another fallback.

It works the same as with error boundaries which made it (personally) easy to work with.

[danielkcz]

Any reason you don’t want to wrap <TheList /> in a fallback? Because that makes sense as you want to show the loading state independently there from the map.

@rovansteen Sorry, I am not sure what you mean, can you show some example?

I am failing to see how is it at the end different from loading prop which lets you decide how the component should look like without data. In the case of the map, I know that I need to keep the same component in the tree, otherwise the nasty effect of reloading it will happen. So there needs to be a prop that will decide.

[trojanowski]

Thank you for your comment @gaearon. BTW do you think it's still possible that Suspense for data fetching will be available mid-2019?

@trojanowski Would you consider switching the suspend to false by default then?

@FredyC Yes, now I think it makes sense. It would allow us to mark this library as production-ready (the suspense part would be still experimental). We'd eventually switch it back when Suspense for data fetching is stable (these changes are the part I like the least). What do you think @umidbekkarimov @sijad @seeden @dimitar-nikovski?

Are you honestly using it or switching it off everywhere as well? :)

I'm using it for an internal app, and it works for me.

Any reason you don’t want to wrap <TheList /> in a fallback? Because that makes sense as you want to show the loading state independently there from the map.

@rovansteen Sorry, I am not sure what you mean, can you show some example?

I was thinking about the same. I guess @rovansteen meant something like this:

ReactDOM.render(
  <>
    <Suspense fallback={<TheListIsLoading />}>
      <TheList />
    </Suspense>
    <Suspense fallback={<TheMapIsLoading />}>
      <TheMap />
    </Suspense>
  </>
);

[joenoon]

@FredyC I think I get what you mean about the fallback completely yanking/destroying the inner component and causing a full new component mount any time suspense kicks in. I'm in a similar place not sure how to handle that without suspend false. The issue I have when using suspend false is the whole hooks-in-order thing. I'm still unclear if its ok to return early and not call other hooks in the case when data.loading, data.error, etc. On one hand ensuring the "order" is easy, but in the case of returning early its possible to not invoke every single hook in the component - i.e. 1, next time 1,2,3. I'm not sure if that is ok or not.

I guess the point about Suspense is you are not limited to one - they can be as specific as you wish, so it only effects the granularity you want:

<Suspense fallback={somethingElseSuspended}>
    // ... deep
    <Suspense fallback={listSuspended}>
      <TheList />
   </Suspense>
    // ... deep
    <Suspense fallback={mapSuspended}>
      <TheMap />
   </Suspense>
 </Suspense>

[danielkcz]

@trojanowski & @joenoon Yea, your examples are kinda what I was afraid of. How is that actually better I wonder? I mean if there would be two components TheList and TheListIsLoading, is it really that much different from passing down the loading prop to TheList? I would say it's even worse because it means you basically have to copy&paste one component and modify how it looks in its loading state. I am failing to see a benefit in there.

@FredyC Yes, now I think it makes sense. It would allow us to mark this library as production-ready (the suspense part would be still experimental).

I am glad you agree. I was already thinking about forking and doing it by myself because it's starting to be rather annoying :)

I'm using it for an internal app, and it works for me.

Wish you were able to share at least some part of it because I cannot imagine how is that possible really :)

[robertvansteen]

In your particular use case it might not benefit you much but imagine a dashboard with a dozen cards/panels that each query their own data. Without suspense every query had to handle their own loading state. With suspense you can just wrap them all in one fallback and you can show one spinner for the whole dashboard until every card has its data.

[robertvansteen]

If you have a hard time understanding the benefits of Suspense I recommend watching this video from two members of the core team of React showing what you can do now and in the future with Suspense: https://youtu.be/ByBPyMBTzM0

[avocadowastaken]

Yes, now I think it makes sense. It would allow us to mark this library as production-ready (the suspense part would be still experimental). We'd eventually switch it back when Suspense for data fetching is stable (these changes are the part I like the least).

Currently I'm writing SSR heavy app with dashboard, so I can not utilize suspense much, I never gave it a real shot.

It doesn’t hurt to experiment with it but I think we should be clear that Suspense data fetching story (and patterns around it essential to creating good UX) is not ready for prime time.

Let's disable it by for default, give it a time, and eventually some one will came up with good api for production apps.

PS: This is how i'm using useQuery for my app:

export function useAppQuery(query, options) {
  return useQuery(query, {
    suspend: false,
    errorPolicy: "all",
    fetchPolicy: "cache-and-network",

    ...options,
  });
}

[danielkcz]

@rovansteen

In your particular use case it might not benefit you much but imagine a dashboard with a dozen cards/panels that each query their own data. Without suspense every query had to handle their own loading state. With suspense you can just wrap them all in one fallback and you can show one spinner for the whole dashboard until every card has its data.

You know, I am not totally convinced this is a good thing. I mean sure, dozens of loaders is bad. but throwing away everything and showing one loader just because a single card needs to refresh its data? How that can be a good UX really? And it's like you cannot even opt-out of Suspense conditionally. For example it would makes sense to have a single loader initially, but later when you are fetching data within a single card, there should be a loader for that one and others should stay there, right?

If you have a hard time understanding the benefits of Suspense I recommend watching this video from two members of the core team of React showing what you can do now and in the future with Suspense: https://youtu.be/ByBPyMBTzM0

I've seen those videos before, but I'll rewatch in case I've missed something there.

[danielkcz]

@rovansteen I have rewatched the video and as much as nice that demo feels, it's apparent it was built for that particular example. I still cannot imagine I could utilize that in my use case where I simply cannot afford to remount the map component.

[robertvansteen]

@FredyC The example in the video is quite common and I could think of at least a dozen more examples where suspense would greatly help.

I don't understand why there would a be remount if you implement it as @joenoon explained. Sure, that might not benefit you much compared to just checking the loading state yourself in this particular use case, but it works fine and it's just an alternative way to handle the loading state.

[danielkcz]

@rovansteen Strange, the app we are making for the last year seem pretty regular and yet I am not finding much of the resemblance with that demo there. Most likely it is some mindset issue for sure.

My main concern at this point is probably about that map component. I would have to do this to avoid UI hiccup. Is it worth the hassle? Not really...

   <Suspense fallback={<TheMap loading={true} />}>
      <TheMap />
   </Suspense>

I am only hoping that React will bring some other way in the future to tackle these scenarios more gracefully otherwise it's hard to imagine going full Suspense.

[robertvansteen]

@FredyC, first of all, I'm not saying your app isn't regular if you don't have a use case for Suspense. I'm saying that Suspense solves a lot of challenges for interfaces that deal with async data.

Second, why the binary thinking? Suspense is just a tool that allows you to work with async data in a more sync way. In your map example, I could imagine you are just fetching data to plot on your map, so you never want your map to re-render when the status of your data fetching changes.
In that case, suspense has no value. But you don't have to use suspense! Suspense is completely opt-in in React. Just like there are plenty of use-cases where Suspense is really helpful there are also plenty of use-cases where it does not add much value, and that's completely fine.

What do you expect from React to improve for your use-case?

[trojanowski]

@FredyC

Yea, your examples are kinda what I was afraid of. How is that actually better I wonder? I mean if there would be two components TheList and TheListIsLoading, is it really that much different from passing down the loading prop to TheList? I would say it's even worse because it means you basically have to copy&paste one component and modify how it looks in its loading state. I am failing to see a benefit in there.

Yes, in your example it isn't better. However, it is in many other cases (like the most I was working on). Let's say you have an app with many routes and each route needs some GraphQL data. Thanks to suspense it's enough to have loading state support just in one place instead of in each of those routes separately, e. g.:

function App() {
  return (
    <Layout>
      <Suspense fallback={<Loading />}>
        <Switch>
          <Route component={LogInPage} exact path="/login" />
          <Route component={SignUpPage} exact path="/signup" />
          <Route component={PostPage} exact path="/posts/:postId" />
          <Route component={UserPage} exact path="/@:username" />
          <Route component={ExplorePage} exact path="/explore" />
          <Route component={HomePage} exact path="/" />
        </Switch>
      </Suspense>
    </Layout>
  );
}

@FredyC Yes, now I think it makes sense. It would allow us to mark this library as production-ready (the suspense part would be still experimental).

I am glad you agree. I was already thinking about forking and doing it by myself because it's starting to be rather annoying :)

I still think that suspend: true is a better default long-term, but it wouldn't be fair to call this library production ready with it right now. Also instead of forking, I'd recommend considering creating a custom hook like the one in @umidbekkarimov's comment: #69 (comment).

I'm using it for an internal app, and it works for me.

Wish you were able to share at least some part of it because I cannot imagine how is that possible really :)

You can look at the port of Pupstagram sample app: https://codesandbox.io/s/8819w85jn9.

[danielkcz]

I still think that suspend: true is a better default long-term, but it wouldn't be fair to call this library production ready with it right now. Also instead of forking, I'd recommend considering creating a custom hook like the one in @umidbekkarimov's comment: #69 (comment).

Curious I haven't thought about that, such an obvious and easy solution, duh! 👍

There is also another project worth investigating: https://github.com/Urigo/WhatsApp-Clone-Client-React. Although briefly looking at it there is just <Suspense fallback={null}> in all occurrences, so nothing spectacular.

Yea I think I will give it a rest, for now, and enjoy released hooks and return to this when it gets more attention. Feel free to close.

[danielbischoff]

I ran into problems too. Using suspend (and especially as a default value) is very dangerous.
Why?
Because there are major bugs:
ReactDOM.findDOMNode doesn't work currently with suspend.
Many UI Toolkits use ReactDOM.findDOMNode.
For example Ant Design. There are many issues of people complaining about the library Ant Design not working correctly while the real problem lies in suspense alpha stadium (for data fetching).

Here is the reason from the react authors:
facebook/react#14198

[gaearon]

FWIW, findDOMNode is considered a legacy API and it is good to migrate away from regardless.

[gaearon]

@FredyC

I still cannot imagine I could utilize that in my use case where I simply cannot afford to remount the map component.

As I mentioned earlier, the feature you’re asking for is planned but it’s not available yet.
It’s not a flaw in the API and you’re correct it’s useful — but it is not ready.

[danielbischoff]

FWIW, findDOMNode is considered a legacy API and it is good to migrate away from regardless.

Hi Dan :),
yes you are right, but most of the major react ui toolkits still use the findDOMNode API. (Antd, Material UI, React Bootstrap) to name a few.

Users using this hooks library (with suspense default on) will run into these problems. (Like me)

We should wait for the react team to officially release the suspense with fetch feature and then activate it as default.

Because only then the ui toolkit maintainers have a chance to react to their library not working with features from react version x and can adapt the supported react versions until they migrated away from findDOMNode.

[gaearon]

The findDOMNode issue is unrelated to data fetching. You'll encounter it with lazy() too. I don't have a good solution but also want to highlight we're not likely to invest a lot of time into solving it since it's a legacy API anyway. But you might start bumping into it as you convert more code to lazy() and Suspense — independent of data fetching.

[trojanowski]

I just published version 0.4.0 which doesn't use Suspense by default. I'm going to close this issue and return to the discussion when Suspense for data fetching is stable.

[capaj]

@trojanowski is there a way to turn on the suspense globally or do I always have to pass it to useQuery()?

[danielkcz]

@capaj You can use the same trick mentioned here, but the opposite: #69 (comment)

[darrentorpey]

Just wanted to say this was a helpful, high-quality conversation and I appreciate the change (@trojanowski) to make Suspense opt-in. It lowers the barriers to entry and will allow more people to start using this library with far less trouble and confusion. 👍

[crstn210]

is there a way to use suspend with the official apollo react hooks implementation?

[heiwais25]

Any updates on this?

[danielkcz]

@heiwais25 You should probably watch official react-apollo, this project was superseded by it some time ago.