eslint-plugin-react icon indicating copy to clipboard operation
eslint-plugin-react copied to clipboard

react/no-unstable-nested-components not needed in React Server Components?

Open karlhorky opened this issue 10 months ago • 7 comments

Hi @ljharb, hope you're well!

I wanted to confirm a suspicion that I have:

Since React Server Components are rendered on the server and do not follow the same re-rendering patterns as Client Components, my guess is that declaring functions inside of Server Components does not follow the same pitfalls, and probably enabling react/no-unstable-nested-components does not enjoy the same benefits. Maybe it's not needed in RSC?

The pattern I'm trying to achieve is "currying in" various server-fetched pieces of information (in this case, props.video) into MDX components passed to an imported MDXContent:

import { LectureVideo } from './LectureVideo.tsx';

type Props = {
  lectureSlug: LectureType['slug'];
  video: Video;
};

export default async function Lecture(props: Props) {
  let MDXContent: (props: LectureMdxProps & { readonly components: MDXComponents | undefined; })
    => JSX.Element;

  try {
    MDXContent = (await import `./lectures/${props.lectureSlug}/index.mdx`).default;
  } catch {
    notFound();
  }

  return (
    <MDXContent
      components={
        {
          // eslint-disable-next-line react/no-unstable-nested-components -- Not problematic in this position in RSC?
          LectureVideo: (
            // propsFromMdx doesn't contain `videoYoutubeId`
            propsFromMdx: Omit<ComponentProps<typeof LectureVideo>, 'videoYoutubeId'>,
          ) => {
            return (
              <LectureVideo
                {...propsFromMdx}
                // "currying in" pattern:
                // props.video.youtubeId is queried from the database and passed down via props
                videoYoutubeId={props.video.youtubeId}
              />
            );
          },
        } satisfies MDXComponents
      }
    />
  );
}

CodeSandbox demo: https://codesandbox.io/p/devbox/fast-tree-x7zpn2?file=%2Fapp%2Fblog%2F%5Bslug%5D%2FLectureVideo.tsx%3A5%2C1

Image

karlhorky avatar Jan 26 '25 14:01 karlhorky

What’s the benefit of declaring it inline, regardless?

ljharb avatar Jan 26 '25 15:01 ljharb

To pass in props from 2 sources:

  1. MDX props (props in the ./lectures/<slug>/index.mdx file)
  2. Server-fetched information (the database-queried props.video information)

How props.video would look (database query in RSC async Page function in Next.js App Router):

type Props = {
  params: Promise<{
    slug: string;
  }>;
};

export default async function Page(props) {
  const slug = (await props.params).slug;
  const video = (await sql<Video[]>`
    SELECT
      id,
      youtube_id
    FROM
      videos
    WHERE
      slug = ${slug}
  `)[0];

  if (!video) notFound();

  // ...

  return (
    <Lecture
      video={video}

karlhorky avatar Jan 26 '25 16:01 karlhorky

Additional constraint / background for us:

I simplified our components to create the examples above, to make it a simpler reproduction and easier to understand.

In our case, the video record from the database is actually dependent on the MDX props - we actually pass in an array of videos (props.videos), and then filter it in the ad-hoc component beforehand:

import { LectureVideo } from './LectureVideo.tsx';

type Props = {
  lectureSlug: LectureType['slug'];
  videos: Video[];
};

export default async function Lecture(props: Props) {
  let MDXContent: (props: LectureMdxProps & { readonly components: MDXComponents | undefined; })
    => JSX.Element;

  try {
    MDXContent = (await import `./lectures/${props.lectureSlug}/index.mdx`).default;
  } catch {
    notFound();
  }

  return (
    <MDXContent
      components={
        {
          // eslint-disable-next-line react/no-unstable-nested-components -- Not problematic in this position in RSC?
          LectureVideo: (
            // propsFromMdx doesn't contain `videoYoutubeId`
            propsFromMdx: Omit<ComponentProps<typeof LectureVideo>, 'videoYoutubeId'>,
          ) => {
            const video = props.videos.find((v) => {
              return v.id === propsFromMdx.videoId;
            });
            return (
              <LectureVideo
                {...propsFromMdx}
                // "currying in" pattern:
                // props.videos is queried from the database and passed down via props
                videoYoutubeId={video.youtubeId}
              />
            );
          },
        } satisfies MDXComponents
      }
    />
  );
}

karlhorky avatar Jan 26 '25 16:01 karlhorky

Also added a CodeSandbox demo to the issue description, in case that helps for experimentation.

karlhorky avatar Jan 26 '25 17:01 karlhorky

I'm confused why you're passing components to MDXContent instead of passing elements, like in a typical react app?

ljharb avatar Jan 27 '25 05:01 ljharb

Ah, because the components prop passing an object of components is how MDX receives it:

  • https://mdxjs.com/docs/using-mdx/#components
  • https://mdxjs.com/table-of-components/

karlhorky avatar Jan 27 '25 08:01 karlhorky

Oof, that's an unfortunate API design.

I'll have to think more about this to get my head around it.

ljharb avatar Jan 27 '25 16:01 ljharb