search

mdx-js / mdx

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

Vite + Vue + MDX Components (Warn) #2512

Closed henrikruscon closed 4 months ago

henrikruscon commented 4 months ago

Initial checklist

Affected packages and versions

https://mdxjs.com/packages/vue/#usemdxcomponentscomponents

Link to runnable example

No response

Steps to reproduce

MarkdownComponent.vue

<template>
    <MDXProvider :components="components">
        <component :is="markdown" />
    </MDXProvider>
</template>

<script>
import LinkComponent from '../components/LinkComponent.vue'
import { h } from 'vue'
import { MDXProvider } from '@mdx-js/vue'

export default {
    components: {
        MDXProvider
    },

    props: {
        markdown: {
            type: Function,
            required: true
        }
    },

    setup() {
        const components = {
            h2: h('h2', {
                class: ['text-3xl font-semibold font-read tracking-tight mt-14 mb-2']
            }),
            p: h('p', {
                class: ['text-lg font-read tracking-[-0.0125em] py-4']
            }),
            a: LinkComponent
        }

        return { components }
    }
}
</script>

LinkComponent.vue

<template>
    <a class="group flex items-center outline-none" :href="href">
        <slot />
    </a>
</template>

<script>
export default {
    props: {
        href: {
            type: String,
            default: ''
        }
    }
}
</script>

vite.config.js

mdx({
    remarkPlugins: [remarkGfm, remarkFrontmatter, remarkMdxFrontmatter],
    jsxImportSource: 'vue',
    providerImportSource: '@mdx-js/vue'
}),

Expected behavior

No warnings.

Actual behavior

Returns warning for all links in the imported MDX.

[Vue warn]: Non-function value encountered for default slot. Prefer function slots for better performance.

Is there any documentation on how to properly replace MDX components with this setup?

Runtime

No response

Package manager

No response

OS

macOS

Build and bundle tools

Vite

wooorm commented 4 months ago
henrikruscon commented 4 months ago

Here is a much simplified version in form of a CodeSandbox of what was shared prev. Check the console output and you'll see the same warnings.

Which part of my implementation is causing the Vue warnings and how to avoid them? I fail to find any documentation on how to properly override components with MDXProvider using my current stack on MDX.

Thank you for taking your time.

wooorm commented 4 months ago

There are so many files there, it does not feel simplified. There are more components than before. They all contain more code. Can you please remove anything that is not relevant? Please help me help you

henrikruscon commented 4 months ago

Removed all the fluff, please check again, it's basically the base Vue + Vite from CodeSandbox and two components. Link.vue component content is totally redundant, it's only the Markdown.vue component file that's relevant, everything else is base setup.

wooorm commented 4 months ago

Why are you asking these things here btw? You are doing dynamic import()s which are different from our docs and getting a Vue warning. Why not ask Vue people about Vue warnings?

wooorm commented 4 months ago

Anyway, you say Link.vue is not relevant. But when I change that, the warning goes away / appears. Sounds like Link.vue, unrelated to MDX itself, is what you need to change.

henrikruscon commented 4 months ago

There are no issues with the dynamic import, nor is it causing any warnings.

Is there a better approach to dynamically load a specific MDX file based on a slug? There are no examples for this.

When you say you change the Link, what exactly do you mean? The link component is as basic as they get, it has a single prop and a default slot, what exactly is there to change besides not importing it at all? Care to share an example that works with a custom component using MDXProvider? The docs for @mdx-js/vue only show how to replace a tag with another tag, not replacing a tag with a component. This is what I'm trying to do and this is what's causing the warning. How is this related to Vue and not MDX in relation to Vue?

Thanks for taking your time with this.

henrikruscon commented 4 months ago

Here is a forked version without the dynamic import entirely just to prove that it has nothing to do with the warnings: CodeSandbox

wooorm commented 4 months ago

Is there a better approach to dynamically load a specific MDX file based on a slug? There are no examples for this.

Well, a) what you do doesn’t normally work, so there’s already something in play here (browsers cannot normally import() MDX files), and b) see https://mdxjs.com/guides/mdx-on-demand/.

The link component is as basic as they get, it has a single prop and a default slot, what exactly is there to change besides not importing it at all?

Well, read the warning: that warning is about Link. And it is about default slots. Removing that default slot makes the warning disappear. Adding it again, makes it appear.

The docs for @mdx-js/vue only show how to replace a tag with another tag

You don’t need @mdx-js/vue

How is this related to Vue and not MDX in relation to Vue?

Vue is choosing to warn. MDX works with any JSX runtime. Vue is one of them. It still works.

This is what I'm trying to do and this is what's causing the warning.

I don‘t know. Apparently Vue warns for certain cases where all other frameworks don’t warn.

henrikruscon commented 4 months ago

Did you see the version I just shared that imports the file with @mdx-js/rollup instead? That rules out the dynamic import being related to the issue.

Removing the default slot makes no difference at all for the warning, here's a new fork that removes both slot and prop entirely, basically just leaving the HTML syntax and it's still throwing the warning.

Remaining syntax for Link.vue is following:

<template>
  <a>Test</a>
</template>

This is as barebone as it gets, it's still throwing issues. Seems specifically related to MDXProvider component to me. Any ideas?

wooorm commented 4 months ago

That rules out the dynamic import being related to the issue.

Sorry for the confusion, that is not what I was implying when I talked about dynamic imports. I wanted to point at that you were doing things not documented by us. Doing lots of different things.

This is as barebone as it gets, it's still throwing issues. Seems specifically related to MDXProvider component to me. Any ideas?

Thanks for making this smaller. Note, I still make files smaller. For example, contributing.mdx can be replaced with [x](https://y.com).. The provider in Markdown.vue can be removed, as <Changelog :components="components" /> is enough. It seems the “hot reloading vue tools” don’t work well. The warning disappeared before but now it doesn’t.

As I tried to get at before: I think this has nothing to do with MDX. It works with many runtimes, and they work. Vue is choosing to warn. Duckduckgo’ing your warning seems to indicate that Vue dislikes certain h calls (https://stackoverflow.com/questions/69875273/non-function-value-encountered-for-default-slot-in-vue-3-composition-api-comp). But we use the automatic runtime. Vue chooses to support it. It should not warn.

Please raise this issue, as minimal as you can (by removing every file that does nothing) to the Vue people. They should not warn.

henrikruscon commented 4 months ago

Thanks for the detailed info, will look further.

henrikruscon commented 4 months ago

FYI: Vue closed the issue with following reason: Please open an issue with @mdx-js/vue

https://github.com/vuejs/core/issues/11331#issuecomment-2223187342