Question around seemingly optional `<Suspense />` usage

Hello RevereCRE, congratulations on the 2.0.0 release!

I've been adopting relay-nextjs where I work and it's pretty unbelievable how much of an improvement the 2.x release is in terms of boilerplate reduction.

In my testing, I've found that in the component there appears to be an optional suspense boundary. And I'm wondering if either I'm missing something, or if this can optionally be entirely removed.

I've been patching relay-nextjs and removing said Suspense boundary and it can create the illusion that the layout is "shared" across navigations..

The default behavior with the Suspense boundary:

https://github.com/RevereCRE/relay-nextjs/assets/2148168/58c15fdd-b61d-4828-a35c-41d763aea336

With a relay-nextjs patch, removing the Suspense boundary:

https://github.com/RevereCRE/relay-nextjs/assets/2148168/082aebc0-0037-4ced-9a00-5e851f3b66a0

And finally, the patch itself

relay-nextjs+2.0.1.patch

Hey @hanford, thanks for the kind words!

This behavior is actually something we were looking for at Revere. Rather than waiting on the data for the page to complete navigation we wanted an early transition. It's actually one of the main reasons this repo entirely avoids getServerSideProps! We have it set up in our app such that our placeholders render a similar UI to the real rendered page (basically <Wrapper> <LoadingLogo /> </WrapperForRelay>) which avoids the very jarring Loading... and makes the transitions much smoother.

I totally understand this isn't desirable for everyone using this library and we were hoping with the release of the startTransition API this would be configurable by users, however we haven't really investigated too deeply. I'm a bit concerned that removing that Suspense boundary may impact other use-cases like changing GraphQL parameters and will also look into that.

In the meantime I definitely recommend using a placeholder option that looks a lot more like the actual application UI. It's actually possible to get it really close to what would actually be rendered with user data too by using a useLazyLoadQuery to read from the store. Here's an example of how we do it:

function PlaceholderWithWrapper() {
  const { viewer } = useLazyLoadQuery<placeholder_WrapperQuery>(
    graphql`
      query placeholder_WrapperQuery {
        viewer {
          name
        }
      }
    `,
    {},
    { fetchPolicy: 'store-or-network' }
  );

  return (
    <Wrapper viewerName={viewer.name}>
      <LoadingLogo />
    </Wrapper>
  );
}

export function Placeholder() {
  return (
    <Suspense fallback={<LoadingLogo />}>
      <PlaceholderWithWrapper />
    </Suspense>
  );
}

Hope this helps!

RevereCRE / relay-nextjs

Question around seemingly optional `<Suspense />` usage #88