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

Make the compiler aware of custom components #2473

Closed
4 tasks done
remcohaszing opened this issue Apr 15, 2024 · 5 comments
Closed
4 tasks done

Make the compiler aware of custom components #2473

remcohaszing opened this issue Apr 15, 2024 · 5 comments
Labels
🤷 no/invalid This cannot be acted upon

Comments

@remcohaszing
Copy link
Member

Initial checklist

Problem

MDX allows users to inject custom components, either via props or via a provider. The following content:

# Header

paragraph

is turned into the following code:

/*@jsxRuntime automatic*/
/*@jsxImportSource react*/
function _createMdxContent(props) {
  const _components = {
    h1: "h1",
    p: "p",
    ...props.components
  };
  return <><_components.h1>{"Header"}</_components.h1>{"\n"}<_components.p>{"paragraph"}</_components.p></>;
}
export default function MDXContent(props = {}) {
  const {wrapper: MDXLayout} = props.components || ({});
  return MDXLayout ? <MDXLayout {...props}><_createMdxContent {...props} /></MDXLayout> : _createMdxContent(props);
}

This works great! However, there’s a bit of redundant code. As a user, I know I’m only interested in injecting a specific component type, let’s say the h1 component. There’s no point in supporting the p or wrapper components. What I would really like, is the following output:

/*@jsxRuntime automatic*/
/*@jsxImportSource react*/
function _createMdxContent(props) {
  const _components = {
    h1: "h1",
    ...props.components
  };
  return <><_components.h1>{"Header"}</_components.h1>{"\n"}<p>{"paragraph"}</p></>;
}
export default function MDXContent(props = {}) {
  return _createMdxContent(props);
}

The intermediate _createMdxContent component is even redundant now, but let’s get into details later.

Because part of the resulting JSX no longer depends on props, it can now be optimized using @babel/plugin-transform-react-constant-elements. Because MDX typically contains a significant amount of JSX content, this optimization is very applicable to MDX. I imagine this could be a recma plugin, outside of MDX.

/*@jsxRuntime automatic*/
/*@jsxImportSource react*/
let _p;
function _createMdxContent(props) {
  const _components = {
    h1: "h1",
    ...props.components
  };
  return <><_components.h1>{"Header"}</_components.h1>{"\n"}{_p ||= <p>{"paragraph"}</p>}</>;
}
export default function MDXContent(props = {}) {
  return _createMdxContent(props);
}

Solution

Make MDX accept a list of component names at compile time. These names will be skipped by recma-jsx-rewrite.

If the components are available where compile() is called, the user could write something like:

import { compile } from '@mdx-js/mdx'

const components = {
  h1: Heading
}

const file = await compile(content, {
  componentNames: Object.key(components)
})

Otherwise, the user needs to be a bit more careful specifying the list of component names.

Alternatives

If the user doesn’t use props.components, and they don’t use a provider, e.g. when using Next.js’ mdx-components file, then _components could even be defined outside of _createMdxContent.

@wooorm
Copy link
Member

wooorm commented Apr 15, 2024

There’s no point in supporting the p or wrapper components.

Well, I mean, there are some points. You may know that you are only passing h1 and Alert currently, but it’s not great that you have to pass an ['h1', 'Alert'] array at compile time to do that?

There are some things we can decide at compile time. And some things at runtime. These are different times.
If you move options to the compile time, you lose abilities.
Say, for example, mdx-components is changed. All documents compiled by MDX will need rto be cache-busted.
Or say, someone now passes h1. It won’t work. If we’d do this, we’d need to introduce runtime errors for when people a) pass components that were not listed at compile time, and b) do not pass components that were listed at compile time.

What is the point of this change? If the point is to use a particular complex optimization plugin (a fork of @babel/plugin-transform-react-constant-elements), well, then a plugin could also do what you want?

@wooorm
Copy link
Member

wooorm commented Sep 13, 2024

@remcohaszing this is currently describing a nice to have for faster/smaller code at the loss of useful features. I’d like to reiterate my earlier “What is the point of this change?” — why should this go here and not in a plugin?

@wooorm
Copy link
Member

wooorm commented Oct 2, 2024

Closing as this is not actionable yet and considerable time has passed. Open to discussing it again, if my above comments are addressed.

@wooorm wooorm closed this as not planned Won't fix, can't repro, duplicate, stale Oct 2, 2024
@wooorm wooorm added the 🤷 no/invalid This cannot be acted upon label Oct 2, 2024
@remcohaszing
Copy link
Member Author

The point is that a lot of content could be static. That would reduce the number of jsx() calls during render, leading to improved performance, at least in React.

I suppose it could be a plugin. I can give it a try some time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤷 no/invalid This cannot be acted upon
Development

No branches or pull requests

2 participants