4

I'm taking advantage of Next.JS SSG to improve the loading speed of my website. The point is I need some data to be fetched client-side, as it belongs to the user logged-in.

Let's say we have a YouTube-like website, so we would have the Video page and a sidebar component, which would be the VideoRelated:

I'd generate the Video page static, by using the VideoQuery at getStaticProps. It would also fetch (client-side) the completion status of those for the user logged-in, so it would be something like:

export const VideoRelatedList = ({ videoId, relatedVideos }) => {
  const { data } = useViewerVideoStatusQuery({ variables: { videoId } });
  const relatedVideosViewerStatus = data?.videos;

  const videos = React.useMemo(() => {
    if (!relatedVideosViewerStatus) return relatedVideos;

    return relatedVideos.map((relatedVideo, index) => {
      const viewerHasCompleted = relatedVideosViewerStatus[index]?.id === relatedVideo.id && relatedVideosViewerStatus[index]?.viewerHasCompleted;
      return { ...relatedVideo, viewerHasCompleted };
    });
  }, [relatedVideosViewerStatus, relatedVideos]);

  return (
    <ol>
      {videos.map(({ id, name, viewerHasCompleted }, index) => (
        <li key={id}>
          {name} - {viewerHasCompleted && 'COMPLETED!'}
        </li>
      ))}
    </ol>
  );
};

What is the best way to combine both data?

Currently, what I'm doing is combining both by using React.memo but I'm not sure if this is a best practice or if there is a better way of achieving what I want.

blacksoul
  • 545
  • 4
  • 24
VanPersie
  • 65
  • 3
  • 13
  • Just to better understanding, VideoList is getting related videos as prop. so why there is a query inside this component? – armin yahya Jan 17 '21 at 08:09
  • 1
    Because the `relatedVideo` prop would be what the page would pass to the component, which has been static-site generated. But the `useViewerVideoStatusQuery` is getting the video status related to the user visiting the page, which is obviously logged-in and can't be fetched when building the static-site. – VanPersie Jan 17 '21 at 08:40

1 Answers1

2

tl;dr this seems like a fine approach, and you probably don't need useMemo

The Next.js documentation has a very brief blurb on client-side data fetching. It looks, essentially, like what you're doing except that they promote their swr library for generic data fetching.

Since you're using GraphQL and Apollo I'm going to operate on the assumption that useViewerVideoStatusQuery extends the useQuery hook from @apollo/client.

You mention:

Currently, what I'm doing is combining both by using React.memo but I'm not sure if this is a best practice or if there is a better way of achieving what I want.

Bear with me, I'm going to try to confirm my understanding of your usecase. If I'm reading correctly, it looks like the VideoRelatedList component can mostly be fully static, and it looks like that's what you've done - if the extra data hasn't loaded, it just uses the data passed via props that was build via SSG. That's good, and should give you the fastest First Contentful Paint (FCP) & Largest Contentful Paint (LCP).

Without the additional data, the component would just look like:

export const VideoRelatedList = ({ videoId, relatedVideos }) => {
  return (
    <ol>
      {relatedVideos.map(({ id, name, viewerHasCompleted }, index) => (
        <li key={id}>
          {name} - {viewerHasCompleted && 'COMPLETED!'}
        </li>
      ))}
    </ol>
  );
};

Now you additionally want to incorporate the user data, to provide the feature that tells the user what videos are complete. You are doing this by fetching data after the page has rendered initially with Apollo.

The main change I'd make (unless the relatedVideos array is huge or there's some expensive operation I've missed) is to remove useMemo as it probably isn't creating significant savings here considering that performance optimizations aren't free.

You might also implementing the loading and error properties from the useQuery hook. That component could look something like:

export const VideoRelatedList = ({ videoId, relatedVideos }) => {
  const { data, loading, error } = useViewerVideoStatusQuery({ variables: { videoId } });

  if (loading) {
    <ol>
      {relatedVideos.map(({ id, name, viewerHasCompleted }, index) => (
        <li key={id}>
          {name} - ...checking status...
        </li>
      ))}
    </ol>
  }

  if (error || !data.videos) {
    return (
      <ol>
        {videos.map(({ id, name, viewerHasCompleted }, index) => (
          <li key={id}>
            {name}
          </li>
        ))}
      </ol>
    );
  }

  const videos = relatedVideos.map((relatedVideo, index) => {
      const viewerHasCompleted = relatedVideosViewerStatus[index]?.id === relatedVideo.id && relatedVideosViewerStatus[index]?.viewerHasCompleted;
      return { ...relatedVideo, viewerHasCompleted };
    });

  return (
    <ol>
      {videos.map(({ id, name, viewerHasCompleted }, index) => (
        <li key={id}>
          {name} - {viewerHasCompleted && 'COMPLETED!'}
        </li>
      ))}
    </ol>
  );
};

Again, this is not a huge departure from what you've done besides no useMemo and a bit of rearranging. Hopefully it's at least helpful to see another perspective on the topic.

A couple references I found informative on the topic:

Ben
  • 5,079
  • 2
  • 20
  • 26