mdx-js / mdx

Markdown for the component era
https://mdxjs.com
MIT License
17.77k stars 1.14k forks source link

@mdx-js/rollup, add a framework option for vue #2265

Closed 0x-jerry closed 1 year ago

0x-jerry commented 1 year ago

Initial checklist

Problem

When I use @mdx-js/rollup with other vite plugin, I encounter an error:

vue-router.mjs:35 [Vue Router warn]: Component "default" in record with path "/about" is 
a function that does not return a Promise. If you were passing a functional component, make 
sure to add a "displayName" to the component. This will break in production if not fixed.

Here is my config file vite.config.ts

import path from 'path'
import { defineConfig } from 'vite'
import vue from '@vitejs/plugin-vue'
import imports from 'unplugin-auto-import/vite'
import components from 'unplugin-vue-components/vite'
import layouts from 'vite-plugin-vue-layouts'
import pages from 'vite-plugin-pages'
import icons from 'unplugin-icons/vite'
import IconsResolver from 'unplugin-icons/resolver'
import jsx from '@vitejs/plugin-vue-jsx'
import uno from 'unocss/vite'
import { mdx } from './vite/mdx'

export default defineConfig(() => {
  return {
    resolve: {
      alias: {
        '@': path.resolve(__dirname, 'src'),
      },
    },

    plugins: [
      vue(),

      // https://github.com/antfu/unplugin-icons
      icons(),

      // https://github.com/antfu/unplugin-auto-import
      imports({
        dts: 'src/auto-imports.d.ts',
        imports: ['vue', 'vue-router'],
      }),

      // https://github.com/antfu/unplugin-vue-components
      components({
        dts: 'src/auto-components.d.ts',
        dirs: ['src/components'],
        resolvers: [IconsResolver()],
      }),

      // https://github.com/JohnCampionJr/vite-plugin-vue-layouts
      layouts({
        exclude: ['**/components/*.vue', '**/*.ts'],
      }),

      // https://github.com/hannoeru/vite-plugin-pages
      pages({
        exclude: ['**/components/*.vue', '**/*.ts'],
        extensions: ['vue', 'mdx'],
      }),

      uno(),

      mdx(),

      jsx({
        include: ['**/*.tsx', '**/*.jsx', '**/*.mdx'],
      }),
    ],
  }
})

Solution

Then, I tried some modification, and it works.

The main idea is to rewrite the export statement.

So, I wander if we can provide an option to do this.

If yes, I think I can make a PR.

import mdxRaw from '@mdx-js/rollup'
import type { Options } from '@mdx-js/rollup'
import type { SourceDescription } from '@mdx-js/rollup/lib'
import type { Plugin } from 'vite'
import { basename } from 'path'

export function mdx(opt: Options = {}): Plugin {
  const plugin = mdxRaw({
    ...opt,
    jsx: true,
    outputFormat: 'program',
  })

  let isDev = false
  return {
    name: '@mdx-js/rollup',
    config(_conf, env) {
      isDev = env.command === 'serve'
    },
    async transform(value, path) {
      if (typeof plugin.transform !== 'function') return

      const file = (await plugin.transform.call(this, value, path)) as SourceDescription

      if (!file) return
      // rewrite export statement.
      let lines = file.code.trim().split(/\n/g)
      lines = lines.slice(0, -1)

      const devInfo = isDev ? `  __file: ${JSON.stringify(path)},` : ''

      lines.push(`import { defineComponent } from 'vue'

export default defineComponent({
  name: ${JSON.stringify(basename(path))},
  ${devInfo}
  setup: (props) => () => MDXContent(props),
})
`)
      file.code = lines.join('\n')

      return file
    },
  }
}

Alternatives

none

ChristianMurphy commented 1 year ago

Welcome @0x-jerry! Have you seen the recommendation in https://mdxjs.com/docs/getting-started/#jsx? You need to set jsx: true in the @mdx-js/rollup config (some how mdx is absent from you example, which may be the issue) and run @vitejs/plugin-vue-jsx after, with include on the .mdx extension. runnable example: https://stackblitz.com/edit/vitejs-vite-buvuvm?file=vite.config.ts,main.js,example.mdx&terminal=dev

vite.config.ts ```ts import { defineConfig } from 'vite'; import mdx from '@mdx-js/rollup'; import vueJsx from '@vitejs/plugin-vue-jsx'; export default defineConfig({ plugins: [mdx({ jsx: true }), vueJsx({ include: /\.mdx/ })], }); ```

No changes are needed to mdx to support this. Feel free to reach out in the discussion forum if you have questions https://github.com/mdx-js/mdx/discussions

0x-jerry commented 1 year ago

@ChristianMurphy Yes, I have checked the document, the vite config I provide is after I made the change. Sorry for the confuse.

And you can see, I move the mdx config to my custom plugin

  const plugin = mdxRaw({
    ...opt,
    jsx: true,
    outputFormat: 'program',
  })

The warning is telling us the compile result of mdx is a funciton component without a displayName, and that will cause some problem in production.

So, I think we still need a solution for this.

vue-router.mjs:35 [Vue Router warn]: Component "default" in record with path "/about" is 
a function that does not return a Promise. If you were passing a functional component, make 
sure to add a "displayName" to the component. This will break in production if not fixed.
ChristianMurphy commented 1 year ago

It still works without changes when configured with outputFormat: program, see the following example: https://stackblitz.com/edit/vitejs-vite-gbwhhd?file=vite.config.ts

vue.config.ts ```ts import { defineConfig } from 'vite'; import mdx from '@mdx-js/rollup'; import vueJsx from '@vitejs/plugin-vue-jsx'; export default defineConfig({ plugins: [ mdx({ jsx: true, outputFormat: 'program' }), vueJsx({ include: /\.mdx/ }), ], }); ```
ChristianMurphy commented 1 year ago

Please read though https://mdxjs.com/community/support for tip on how to frame questions so others can help. In particular focus in on creating a minimal reproducible example: https://stackoverflow.com/help/minimal-reproducible-example

Given the examples I shared: https://stackblitz.com/edit/vitejs-vite-buvuvm?file=vite.config.ts and https://stackblitz.com/edit/vitejs-vite-gbwhhd?file=vite.config.ts I tend to think MDX is not the root of the issue here, and that most likely another plugin has injected invalid content.

You are welcome to share a minimal reproducible example in a sandbox if you believe otherwise. Consider using the stackblitz links I shared as a starting point, and on reducing the dependencies and content down to just what is needed to replicate the issue.

0x-jerry commented 1 year ago

@ChristianMurphy yes, without vue-router, it works. but the warning is from vue-router with dynamic import, please see this: https://stackblitz.com/edit/vitejs-vite-cqesqn?file=example.mdx,main.js,about.mdx

Click about then click home, and some warning will display on the console.

BTW, thanks for your patience.

ChristianMurphy commented 1 year ago

That feels like a bug in vue-router, displayName should not be required for functional components.

from the React JSX docs:

The displayName string is used in debugging messages. Usually, you don’t need to set it explicitly because it’s inferred from the name of the function or class that defines the component. You might want to set it explicitly if you want to display a different name for debugging purposes or when you create a higher-order component, see Wrap the Display Name for Easy Debugging for details.

https://reactjs.org/docs/react-component.html#displayname

in addition, Vue itself does not suggest that should be required: https://vuejs.org/guide/extras/render-function.html#jsx-tsx

MDX generates a component with a valid name, should it should not need a displayName.


here is a workaround to help vue-router infer what it should be able to already, that a valid name is provided. https://stackblitz.com/edit/vitejs-vite-t338zx?file=main.js

router.js ```js import { createApp, defineComponent, h } from 'vue'; import { createRouter, createWebHashHistory, RouterView } from 'vue-router'; async function vueRouterJsxFix(importPromise) { const mdxExports = await importPromise; mdxExports.default.displayName = mdxExports.default.name; return mdxExports; } const router = createRouter({ routes: [ { component: () => vueRouterJsxFix(import('./example.mdx')), path: '/', }, { component: () => vueRouterJsxFix(import('./about.mdx')), path: '/about', }, ], history: createWebHashHistory(), }); createApp( defineComponent({ render: () => h(RouterView), }) ) .use(router) .mount('#app'); ```

Another alternative if vue-router opts not to fix the bug, you could also add a recmaPlugin like https://github.com/domdomegg/recma-mdx-displayname to generate a displayName.

0x-jerry commented 1 year ago

Thank you for providing such a detailed answer! I will give it a try.

0x-jerry commented 1 year ago

FYI, I found this is not a bug, https://github.com/vuejs/router/issues/915, but still, I agree with you that I should use another workaround.

wooorm commented 1 year ago

I should use another workaround.

Did you try the recma-mdx-displayname workaround? Does that work for you?

0x-jerry commented 1 year ago

@wooorm For now, I prefer my own solution, because I encounter some other problem that recma-mdx-displayname can't resolve.

You can find the code here: https://github.com/0x-jerry/try-mdx/blob/29da4c018ef26ba3da5a8c554d6ed368b172fe31/vite/mdx/index.ts