hashicorp / next-mdx-remote

Load MDX content from anywhere
Mozilla Public License 2.0
2.72k stars 141 forks source link

mdx3 #425

Closed hipstersmoothie closed 8 months ago

hipstersmoothie commented 10 months ago

It seems to work. Had to do some odd stuff with the runtime.

closes #419

I also had to upgrade a bunch of deps too

hashicorp-cla commented 10 months ago

CLA assistant check

Thank you for your submission! We require that all contributors sign our Contributor License Agreement ("CLA") before we can accept the contribution. Read and sign the agreement

Learn more about why HashiCorp requires a CLA and what the CLA includes

Have you signed the CLA already but the status is still pending? Recheck it.

talatkuyuk commented 10 months ago

Hi @hipstersmoothie, thanks for your work.

Did you test it with "import" and "export" statements in mdx? As you know, mdx v3 supports "import" and "export"s.

an example mdx: (taken from original test files in @mdx-js/mdx)

import {Pill} from "./components.js"

<Pill>Hello</Pill>
hipstersmoothie commented 10 months ago

Do import even make sense for mdx remote? Since the content is remote this import paths are never gonna make for relative paths. I could see the case for npm packages but since this package doesn't do any bundling or building I don't see how that's possible

hipstersmoothie commented 10 months ago

My reason to upgrade was to use the latest version of an unified plugins

talatkuyuk commented 10 months ago

Do import even make sense for mdx remote? Since the content is remote this import paths are never gonna make for relative paths. I could see the case for npm packages but since this package doesn't do any bundling or building I don't see how that's possible

Don't agree. Makes sense because it doesn't require to supply all components into MDXRemote and gives an option to include a specific component into some mdx content as @mdx-js/mdx supports. Any way, this is going to be available in the next next-mdx-remote versions somehow I believe. Thanks again.

talatkuyuk commented 10 months ago

May I ask, what is the reason you change the code

{
   opts: {
     ...mdx,
     ...jsxRuntime,
     jsxs: (jsxRuntime as any).jsxs || (jsxRuntime as any).jsxDEV,
     jsx: (jsxRuntime as any).jsx || (jsxRuntime as any).jsxDEV,
   },
},

instead of

{ opts: { ...mdx, ...jsxRuntime } },

As far as I know, jsxRuntime has already jsx, jsxs, jsxDEV, Fragment depending on development or production environment.

talatkuyuk commented 10 months ago

Hi @hipstersmoothie,

I have tried MDXRemote with using the package "@alisowski/next-mdx-remote/rsc" which is a adapted version of next-mdx-remote.

My nextjs version is 14.

The problem may not be related the package but related with the nextjs@14 default behaviour.

When I pass the components prop to MDXRemote as ususal, it gives error about strong: Error: Functions cannot be passed directly to Client Components unless you explicitly expose it by marking it with "use server".

return (
    <MDXRemote
      source={source}
      components={{
        strong: (props: React.HTMLAttributes<HTMLElement>) => (
          <strong className="custom-strong" {...props} />
        )
      }}
    />
  );

What do you think about that should the strong function be a server action or client component? This is my main question.

If it will be a server action I have to define the strong as a async function:

async function strong(props: React.HTMLAttributes<HTMLElement>) {
    "use server";

    return <strong className="custom-strong" {...props} />;
  }

then use it in the component props:

return (
    <MDXRemote
      source={source}
      components={{strong}}
    />
  );

Then the error goes away, no problem. But this use case bothers me, I couldn't understand why I need to create a server action just for that ? This doesn't make sense to me.

On the other way, if the strong should be a client component, in this case, I have to define it in another jsx/tsx file, putting "use client" on top of the file. This also bothers me as leading to create unnecessary files for simple html tag mapping.

I suppose this is a nextjs@14 awkward default behavior.

I need your opinion about that the components should be client components or server actions, or is another way that I don't know?

DavieReid commented 9 months ago

Hi @hipstersmoothie,

I have tried MDXRemote with using the package "@alisowski/next-mdx-remote/rsc" which is a adapted version of next-mdx-remote.

My nextjs version is 14.

The problem may not be related the package but related with the nextjs@14 default behaviour.

When I pass the components prop to MDXRemote as ususal, it gives error about strong: Error: Functions cannot be passed directly to Client Components unless you explicitly expose it by marking it with "use server".

return (
    <MDXRemote
      source={source}
      components={{
        strong: (props: React.HTMLAttributes<HTMLElement>) => (
          <strong className="custom-strong" {...props} />
        )
      }}
    />
  );

What do you think about that should the strong function be a server action or client component? This is my main question.

If it will be a server action I have to define the strong as a async function:

async function strong(props: React.HTMLAttributes<HTMLElement>) {
    "use server";

    return <strong className="custom-strong" {...props} />;
  }

then use it in the component props:

return (
    <MDXRemote
      source={source}
      components={{strong}}
    />
  );

Then the error goes away, no problem. But this use case bothers me, I couldn't understand why I need to create a server action just for that ? This doesn't make sense to me.

On the other way, if the strong should be a client component, in this case, I have to define it in another jsx/tsx file, putting "use client" on top of the file. This also bothers me as leading to create unnecessary files for simple html tag mapping.

I suppose this is a nextjs@14 awkward default behavior.

I need your opinion about that the components should be client components or server actions, or is another way that I don't know?

I don't think it should be a server action, but I also think if you moved the strong component to it's own file it could remain as server component (i.e. no 'user client' at the top).

dstaley commented 8 months ago

@hipstersmoothie I appreciate you opening this PR! We've recently updated to v3 in this PR, so I'm going to close this PR.