hashicorp / next-mdx-remote

Load MDX content from anywhere
Mozilla Public License 2.0
2.67k stars 140 forks source link

TypeError when replacing default components #290

Closed rfrowe closed 1 year ago

rfrowe commented 2 years ago

Howdy 🤠 I'm following the README's directions on overriding default components:

import { Typography } from "@material-ui/core";

const components = { Test, h2: (props) => <Typography variant="h2" {...props} /> }

However, when I try to use components as such:

import {Typography} from "@mui/material";
import {MDXComponents} from "mdx/types";

const components: MDXComponents = {
    h1: (props) => <Typography variant='h1' {...props} />,
};w

I receive the following type error:

Type '{ ref?: LegacyRef<HTMLHeadingElement> | undefined; key?: Key | null | undefined; defaultChecked?: boolean | undefined; defaultValue?: string | number | readonly string[] | undefined; ... 252 more ...; variant: "h1"; }' is not assignable to type 'Omit<Pick<DetailedHTMLProps<HTMLAttributes<HTMLSpanElement>, HTMLSpanElement>, "key" | keyof HTMLAttributes<HTMLSpanElement>> & { ...; }, keyof CommonProps | ... 8 more ... | "variantMapping">'.
    Types of property 'ref' are incompatible.
        Type 'LegacyRef<HTMLHeadingElement> | undefined' is not assignable to type '((instance: HTMLSpanElement | null) => void) | RefObject<HTMLSpanElement> | null | undefined'.
            Type 'string' is not assignable to type '((instance: HTMLSpanElement | null) => void) | RefObject<HTMLSpanElement> | null | undefined'.

If I modify my arrow function to

({ref: _, ...props}) => <Typography variant='h2' {...props} />

This works just fine. Is it important that this ref be passed to the Typography component and if so how can this be done in a typesafe way?

chrisakroyd commented 2 years ago

Hello mate, I ran into the same issue or a very similar one.

You can solve it by extending the MDXComponents type and then casting your components object to the resulting type. This has the triple benefit of being able to accept your component, the re-defined type may still be used with the default components, and it's about as quick of a fix as you can come up with. I leave it to the reader whether they consider this type-safe or not but the typescript compiler is satiated by this solution.

import React from 'react';
import { MDXComponents } from 'mdx/types';

type Props = {
  children: string,
  className: string,
};

function Code({ children, className }: Props) {
  return (
    <code className={className}>
      <pre className={className}>
        {children}
      </pre>
    </code>
  );
}

// When we override the default component, also need extend the pre-built type that expects
// anything to now also expect our React component.
type ProvidedComponents = MDXComponents & {
  code?: typeof Code,
};

export default function TestPage({ source }) {
  return (
    <div className="wrapper">
      <MDXRemote {...source} components={{ code: Code } as ProvidedComponents} />
    </div>
  )
}

The above worked example should give you the full gist of how to fix it, let me know if there are any questions or obvious mistakes.

badalsaibo commented 1 year ago

@chrisakroyd Your solution worked perfectly. The only thing I had to do was to install @types/mdx as a dev dependency since I wasn't using mdx as a package.

import { Typography } from '@mui/material-ui';
import { MDXRemote, MDXRemoteProps } from 'next-mdx-remote';
import type { MDXComponents } from 'mdx/types';

type Props = {
  children: React.ReactElement;
};

export default function Markdown(props: MDXRemoteProps) {
  const components = {
    h1: (props: Props) => <Typography level="h1" {...props} />,
    p: (props: Props) => <Typography level="body1" {...props} />,
  } as MDXComponents;

  return <MDXRemote {...props} components={components} />;
}

Also it doesn't show any errors if I just use the default MDXComponents instead of extending it as you did. Is there any issue with that?

BRKalow commented 1 year ago

Hi there, it looks like there is a type mismatch between what your material-ui components are expecting and what MDX is declaring the component types as. @badalsaibo's suggestion to narrow the prop types that you are accepting seems like a good approach to work around that.

I'm going to close this as I don't think there is anything actionable for us here. Thanks!

macguirerintoul commented 1 year ago

I was getting a similar error; casting components to MDXComponents fixed it. I wanted to dig deeper into what was going on.

Components:

const components = {
    h2: (props: { children: ReactNode }) => (
        <h2>
            <Balancer>{props.children}</Balancer>
        </h2>
    ),
    h3: (props: { children: ReactNode }) => (
        <h3>
            <Balancer>{props.children}</Balancer>
        </h3>
    ),
    img: (props: ImageProps) => (
        <Image
            alt={props.alt}
            width={props.width}
            height={props.height}
            src={props.src}
            loading="lazy"
        />
    ),
};

Error:

// Type '{ h2: (props: {    children: ReactNode;}) => JSX.Element; h3: (props: {    children: ReactNode;}) => JSX.Element; img: (props: ImageProps) => JSX.Element; }' is not assignable to type 'MDXComponents'.
<MDXRemote {...props.path.mdx} components={components} />

After inspecting the full error ("Copy Message" in VS Code and paste it somewhere), I was able to see the root problem:

Type '{ h2: (props: {    children: ReactNode;}) => JSX.Element; h3: (props: {    children: ReactNode;}) => JSX.Element; img: (props: ImageProps) => JSX.Element; }' is not assignable to type 'MDXComponents'.
  Type '{ h2: (props: {    children: ReactNode;}) => JSX.Element; h3: (props: {    children: ReactNode;}) => JSX.Element; img: (props: ImageProps) => JSX.Element; }' is not assignable to type '{ a?: Component<DetailedHTMLProps<AnchorHTMLAttributes<HTMLAnchorElement>, HTMLAnchorElement>> | undefined; ... 174 more ...; view?: Component<...> | undefined; }'.
    Types of property 'h2' are incompatible.
      Type '(props: {    children: ReactNode;}) => JSX.Element' is not assignable to type 'Component<DetailedHTMLProps<HTMLAttributes<HTMLHeadingElement>, HTMLHeadingElement>> | undefined'.
        Type '(props: {    children: ReactNode;}) => JSX.Element' is not assignable to type 'FunctionComponent<DetailedHTMLProps<HTMLAttributes<HTMLHeadingElement>, HTMLHeadingElement>>'.
          Types of parameters 'props' and 'props' are incompatible.
            Type 'DetailedHTMLProps<HTMLAttributes<HTMLHeadingElement>, HTMLHeadingElement>' is not assignable to type '{ children: ReactNode; }'.
              Property 'children' is optional in type 'ClassAttributes<HTMLHeadingElement> & HTMLAttributes<HTMLHeadingElement>' but required in type '{ children: ReactNode; }'.

React.DOMAttributes defines children as optional. Setting children as optional in my replacements fixed this issue:

h2: (props: { children?: ReactNode }) => (
    <h2>
        <Balancer>{props.children}</Balancer>
    </h2>
),

This brought me to the next set of errors regarding next/image:

Type '{ h2: (props: {    children?: ReactNode;}) => JSX.Element; h3: (props: {    children?: ReactNode;}) => JSX.Element; img: (props: ImageProps) => JSX.Element; }' is not assignable to type 'MDXComponents'.
  Type '{ h2: (props: {    children?: ReactNode;}) => JSX.Element; h3: (props: {    children?: ReactNode;}) => JSX.Element; img: (props: ImageProps) => JSX.Element; }' is not assignable to type '{ a?: Component<DetailedHTMLProps<AnchorHTMLAttributes<HTMLAnchorElement>, HTMLAnchorElement>> | undefined; ... 174 more ...; view?: Component<...> | undefined; }'.
    Types of property 'img' are incompatible.
      Type '(props: ImageProps) => JSX.Element' is not assignable to type 'Component<DetailedHTMLProps<ImgHTMLAttributes<HTMLImageElement>, HTMLImageElement>> | undefined'.
        Type '(props: ImageProps) => JSX.Element' is not assignable to type 'FunctionComponent<DetailedHTMLProps<ImgHTMLAttributes<HTMLImageElement>, HTMLImageElement>>'.
          Types of parameters 'props' and 'props' are incompatible.
            Type 'DetailedHTMLProps<ImgHTMLAttributes<HTMLImageElement>, HTMLImageElement>' is not assignable to type 'ImageProps'.
              Type 'DetailedHTMLProps<ImgHTMLAttributes<HTMLImageElement>, HTMLImageElement>' is not assignable to type '{ src: string | StaticImport; alt: string; width?: SafeNumber | undefined; height?: SafeNumber | undefined; fill?: boolean | undefined; loader?: ImageLoader | undefined; ... 11 more ...; lazyRoot?: string | undefined; }'.
                Types of property 'src' are incompatible.
                  Type 'string | undefined' is not assignable to type 'string | StaticImport'.
                    Type 'undefined' is not assignable to type 'string | StaticImport'.

ImageProps defines src as string | StaticImport, while React.ImgHTMLAttributes allows it to be string | undefined.

I'm new to TypeScript but I assume what this means is MDX is creating props of type DetailedHTMLProps<ImgHTMLAttributes<HTMLImageElement>, HTMLImageElement> and trying to give it as ImageProps, which doesn't work. There were several other issues related to other props like alt, width, etc.

What I did was create a new type based on ImageProps, which allows for the additional prop types that MDX allows:

type NewImageProps = Omit<
    ImageProps,
    "src" | "alt" | "width" | "height" | "placeholder"
> & {
    src?: ImageProps["src"] | undefined;
    alt?: ImageProps["alt"] | undefined;
    width?: ImageProps["width"] | string | undefined;
    height?: ImageProps["height"] | string | undefined;
    placeholder?: ImageProps["placeholder"] | string | undefined;
};

Then, use that type for the props going to img, and then ensure you are passing appropriate types to next/image which will expect the normal ImageProps:

img: (props: NewImageProps) => {
    const newAlt: string =
        typeof props.alt === "string" ? props.alt : "no alt provided";
    const newSrc: string = typeof props.src === "string" ? props.src : "nosrc";

    return (
        <Image
            alt={newAlt}
            width={Number(props.width)}
            height={Number(props.height)}
            src={newSrc}
            loading="lazy"
        />
    );
},

I don't know enough about this stuff to know if there are any actual issues here with any of the packages, but hopefully someone finds this helpful someday :)