Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
karlhorky opened this issue Jan 26, 2025 · 7 comments
Open

Comments

@karlhorky
Copy link
Contributor

karlhorky commented Jan 26, 2025

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

@ljharb
Copy link
Member

ljharb commented Jan 26, 2025

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

@karlhorky
Copy link
Contributor Author

karlhorky commented Jan 26, 2025

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
Copy link
Contributor Author

karlhorky commented Jan 26, 2025

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
Copy link
Contributor Author

karlhorky commented Jan 26, 2025

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

@ljharb
Copy link
Member

ljharb commented Jan 27, 2025

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

@karlhorky
Copy link
Contributor Author

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

@ljharb
Copy link
Member

ljharb commented Jan 27, 2025

Oof, that's an unfortunate API design.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants