Shopify / hydrogen-v1

React-based framework for building dynamic, Shopify-powered custom storefronts.
https://shopify.github.io/hydrogen-v1/
MIT License
3.75k stars 326 forks source link

Performance: `<Link>` Component buffers full RSC response, which is sometimes slow #957

Open vlucas opened 2 years ago

vlucas commented 2 years ago

When using the <Link> component around same-site links, the full RSC request/response is buffered before the page changes or updates.

If that page takes a while to load (say over 500ms), then it is very noticeably slow to the end user. Additionally, there is no UI/UX for the user to know that something is happening - the link just appears to do nothing for a while until the page suddenly changes.

Devs can add their own transition UI to these links, but doing so on each and every link everywhere in the document is a ton of extra work for no discernible benefit. Given the way they work now, many devs are more likely to just not use the <Link> component at all, and have normal <a> links instead.

We need to fix this experience so that the <Link> component is the obvious choice for a better, faster experience.

vlucas commented 2 years ago

Example site where this slowness can be observed: https://notebinding.fly.dev/book/demo/demo-subpage-2-3665302c23e74024a6d8fe26376f94b8

beppek commented 2 years ago

Would it be possible to expose event hooks/listeners for Router in a fashion similar to Next.js: https://nextjs.org/docs/api-reference/next/router#routerevents ?

It would allow listening to route change events to show a primitive progress indicator from start to complete/error. At least there'd be some indication that something is happening.

beppek commented 2 years ago

Example site where this slowness can be observed: https://notebinding.fly.dev/book/demo/demo-subpage-2-3665302c23e74024a6d8fe26376f94b8

This is clearly noticeable even on the default starter template, at least on my poor Australian internet connection.

SamuelPinho commented 2 years ago

At first I thought it was something just on the development side, but it looks like it has the same problem on the prod version

blittle commented 2 years ago

Performance optimizations are priority for us right now. That said, there are still cases where page transitions will be slow. In those scenarios we still need to provide some hooks to know when a page transition has started / finished. We are unable to provide an API like next routerevents because the <Router> component is a server component, and the callbacks are executed on the client (functions cannot be serialized between server components and client components).

Instead, like next, we could expose a router object with useRouter() and allow you to listen to routing related events. Then, you could write a wrapper client component and use it inside App.server.jsx:

// Wrapper Client Component
import {useRouter} from '@shopify/hydrogen/client';

export default function LoadingWrapper({children}) {
  const router = useRouter();
  const [isLoading, setIsLoading] = useState(false);

  useEffect(() => {
    const routeChangeStart = () => {
      setIsLoading(true);
    }

    const routeChangeEnd = () => {
      setIsLoading(false);
    }

    router.events.on('routeChangeStart', routeChangeStart);
    router.events.on('routeChangeEnd', routeChangeEnd);

    return () => {
      router.events.off('routeChangeStart', routeChangeStart)
      router.events.off('routeChangeEnd', routeChangeEnd);
    }
  }, [router]);

  return <>
    {isLoading ? <SomePageLoadingIndicator /> : null}
    {children}
  </>
}
// inside App.server.jsx

function App({routes}) {
  return (
    <Suspense fallback={<LoadingFallback />}>
      <ShopifyProvider shopifyConfig={shopifyConfig}>
        <CartProvider>
          <DefaultSeo />
          <Router>
            <LoadingWrapper>
              <FileRoutes routes={routes} />
              <Route path="*" page={<NotFound />} />
            </LoadingWrapper>
          </Router>
        </CartProvider>
      </ShopifyProvider>
    </Suspense>
  );
}
vlucas commented 2 years ago

I think these events would be good for times when we really cannot fix a very slow page, but the ultimate solution to the root issue is to leverage RSC streaming to make the UI/UX changes instantaneous, and let the content stream load in. We need to do more research around that to see how we can enable that, even from one page to another page with a completely different layout.