hashicorp / next-mdx-remote

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

Import free components in mdx? #398

Open mtr1990 opened 1 year ago

mtr1990 commented 1 year ago

Hi,

Do we support this import in MDXRemote ?


// test.mdx

import License from './license.md' // Assumes an integration is used to compile MDX -> JS.
import Contributing from './docs/contributing.mdx'

# Hello world

<License />

---

<Contributing />
// test.mdx
import {Box, Heading} from 'rebass'

MDX using imported components!

<Box>
  <Heading>Here’s a heading</Heading>
</Box>

Thanks!

Utzel-Butzel commented 1 year ago

This is not possible with next-mdx-remote. You may want to take a look at https://github.com/kentcdodds/mdx-bundler

dominik-sfl commented 1 year ago

Is mdx-bundler such an advantage anymore, though, now that next-mdx-remote supports server-components?

Utzel-Butzel commented 1 year ago

The main advantage is that it allows you to have inline imports inside your mdx. In next-mdx-remote you need to add all the components to your components={...components} list.

https://github.com/kentcdodds/mdx-bundler#faq

dominik-sfl commented 1 year ago

Sure. Just wondering how useful inline imports are, now that next-mdx-remote supports server components, which I assume reduces bundle size & loading time considerably?

karlhorky commented 8 months ago

@dstaley what changes would be necessary to support import in .mdx files? (to remove the caveat import / export listed in the readme)

(ideally imports of any extensions the bundler supports, eg. .mdx, .md, .tsx, .ts)

Would be great to be able to use next-mdx-remote for this and not have to switch to mdx-bundler.

cc @wooorm @remcohaszing in case you know details about what is necessary for this support in next-mdx-remote

remcohaszing commented 8 months ago

It looks like this project has its own eval logic in src/rsc.tsx instead of using MDX’s run(). run() supports a baseUrl option.

dstaley commented 8 months ago

It's important to remember that when this library was originally made, it wasn't possible to support import statements due to how code was generated/executed (hence the caveat from the README). However, in the upgrade to MDX v2 (released in next-mdx-remote v4), we actually did add support for import statements via the useDynamicImport option. That would generate code like the following:

 /*@jsxRuntime automatic @jsxImportSource react*/
const {Fragment: _Fragment, jsx: _jsx} = arguments[0];
const {default: t} = await import('./hello');
function _createMdxContent(props) {
  return _jsx(_Fragment, {});
}
function MDXContent(props = {}) {
  const {wrapper: MDXLayout} = props.components || ({});
  return MDXLayout ? _jsx(MDXLayout, Object.assign({}, props, {
    children: _jsx(_createMdxContent, props)
  })) : _createMdxContent(props);
}
return {
  default: MDXContent
};

The important bit is await import('./hello'). You'll notice though that the import statement remains relative, and thus is pretty tricky to get working correctly since the import is relative to the execution location, not the MDX file's location.

MDX v3 introduced a more robust dynamic import system that's enabled by default, with support for specifying a baseUrl to perform resolution against. In v3, the same MDX from above looks like this:

"use strict";
const {Fragment: _Fragment, jsx: _jsx} = arguments[0];
const _importMetaUrl = arguments[0].baseUrl;
if (!_importMetaUrl) throw new Error("Unexpected missing `options.baseUrl` needed to support `export … from`, `import`, or `import.meta.url` when generating `function-body`");
const {default: t} = await import(_resolveDynamicMdxSpecifier('./hello'));
function _createMdxContent(props) {
  return _jsx(_Fragment, {});
}
function MDXContent(props = {}) {
  const {wrapper: MDXLayout} = props.components || ({});
  return MDXLayout ? _jsx(MDXLayout, {
    ...props,
    children: _jsx(_createMdxContent, {
      ...props
    })
  }) : _createMdxContent(props);
}
return {
  default: MDXContent
};
function _resolveDynamicMdxSpecifier(d) {
  if (typeof d !== "string") return d;
  try {
    new URL(d);
    return d;
  } catch {}
  if (d.startsWith("/") || d.startsWith("./") || d.startsWith("../")) return new URL(d, _importMetaUrl).href;
  return d;
}

So while it's technically possible to use imports in v4 (albeit quite clunkily), the new v5 will be a bit easier to work with. As of writing v5 doesn't support imports, but in the next few days that option will be re-added. One important thing to keep in mind is that next-mdx-remote doesn't do any sort of bundling, and if it's executed in an environment that was built with a bundler these import statements might not resolve correctly, so I only suggest using this functionality if you can ensure your dynamic imports will actually resolve to a module correctly.

karlhorky commented 8 months ago

Ok nice, thanks for the notes! I'll try to keep an eye on #448 and the v5 canary releases for any new progress on that, and when support lands, we'll give it a shot with our Next.js RSC setup.

One important thing to keep in mind is that next-mdx-remote doesn't do any sort of bundling, and if it's executed in an environment that was built with a bundler these import statements might not resolve correctly, so I only suggest using this functionality if you can ensure your dynamic imports will actually resolve to a module correctly.

Good caveat to point out. I probably don't yet understand all of the implications of this yet, but it seems like there may be some module resolution details to work out here. Maybe it will be a similar flavor to how we had to configure cwd with mdx-bundler with our other experiments.

dstaley commented 8 months ago

Following up on my comment regarding useDynamicImport in v4: I'm not actually sure that ever worked. useDynamicImport emits await import statements at the top level, which won't run given that we eval MDX via Reflect.construct(Function, ...). We can certainly make it work, but that would be a breaking API change that I'd rather save for v6 given that I announced v5 would maintain API compatibility with v4.

That being said, there's a non-trivial amount of useDynamicImport usage according to GitHub, but even then I'm struggling to find an example of someone actually performing an import in their MDX and passing that to next-mdx-remote as a test case. Honestly I'm a little baffled at why there's so many instances given that I don't think it does anything, so I'm hoping I'm just missing something.

If anyone is currently using useDynamicImport to import components (or for some other reason!) and wouldn't mind sharing a link, that'd be greatly appreciated!

talatkuyuk commented 8 months ago

useDynamicImport emits await import statements at the top level, which won't run given that we eval MDX via Reflect.construct(Function, ...)

I agree with @dstaley, that is why I employed an async function construction in Reflect constructor in the implementation of the runAsync function in the package next-mdx-remote-client to support await at the top level.

Currently, the next-mdx-remote-client supports import-in-MDX in the app router.

Here is the proof it is working testing app. You see I am able to say HELLO from imported <Bar /> component in red color that is coming from imported component. See <Bar /> component and the source mdx file that imports the Bar.mjs

But the imported component <Bar /> is not .jsx or .tsx. I couldn't manage to import such a kind of react component but .mjs that is transpiled into javascript with React.createElement. It is a pain for importing a mdx component which is required to be transpiled before. Maybe someone can figure out how to import .jsx in mdx file in the next-mdx-remote-client.

I also agree with @dstaley, importing a module in mdx with relative path is a bit tricky, has close connection with baseUrl, and the relative location of mdx file to the function that you call for compiling the mdx source.

For the comment of @remcohaszing, I can say that, it is not related with whether or not use @mdx-js/mdx's run function in the next-mdx-remote, it is related with whether or not pass baseUrl entry into the function construction. As @remcohaszing mentioned, function construction in next-mdx-remote should take baseUrl inside the first argument.