web-infra-dev / rspress

🦀💨 A fast Rspack-based static site generator.
https://rspress.dev
MIT License
1.19k stars 106 forks source link

[Bug]: "the name `Tabs` is defined multiple" when flattening Mdx files #1071

Open SoonIter opened 1 month ago

SoonIter commented 1 month ago

Version

System:
    OS: macOS 14.2.1
    CPU: (10) arm64 Apple M1 Max
    Memory: 54.31 MB / 32.00 GB
    Shell: 5.9 - /bin/zsh
  Browsers:
    Chrome: 124.0.6367.119
    Chrome Canary: 126.0.6467.2
    Safari: 17.2.1
  npmPackages:
    @rspress/plugin-container-syntax: 1.19.1 => 1.19.1 
    @rspress/plugin-playground: 1.19.1 => 1.19.1 
    @rspress/plugin-preview: 1.19.1 => 1.19.1 
    rspress: 1.19.1 => 1.19.1

Details

Rspress uses flattenMdxContent to inline one mdx to another mdx file. However, this function currently has some bugs.

https://github.com/web-infra-dev/rspress/blob/5121cac6cdc08cc58ed613347d43d48cf90dcc3d/packages/core/src/node/utils/flattenMdxContent.ts#L59

case 1

// a.mdx
import { Tabs } from '@theme'
// index.mdx
import { Tabs } from ‘@theme’

import A from './a'

<A />

rspress uses flattenMdxContent to inline "a.mdx" into "index.mdx" for better analysis

  × Module build failed:
  ╰─▶   × Error:
        │   × the name `Tabs` is defined multiple times
        │    ╭─[/Users/appe/Documents/demos/rspress-preview-demo/docs/guide/index.mdx:1:1]
        │  1 │ const frontmatter = {};
        │  2 │ import { useMDXComponents as _provideComponents } from "@mdx-js/react";
        │  3 │ import { Tabs } from '@theme';
        │    ·          ──┬─
        │    ·            ╰── previous definition of `Tabs` here
        │  4 │ import { Tabs } from '@theme';
        │    ·          ──┬─
        │    ·            ╰── `Tabs` redefined here
        │  5 │ function _createMdxContent(props) {
        │  6 │     const _components = Object.assign({
        │  7 │         h1: "h1",
        │    ╰────
        │
        │

Case 2

// a.mdx
import { Tabs } from '@theme'
// b.mdx
import { Tabs } from '@theme'
// index.mdx
import A from './a'

import B from './b'

<A />

<B />

same error

Possible Solution

We need to refactor this function and find the best solution to combine two mdx fragments into one, just like concatenation module or scope hoisting

https://webpack.js.org/plugins/module-concatenation-plugin/#root

Reproduce link

https://github.com/SoonIter/rspress-mdx-flatten

Reproduce Steps

  1. pnpm install
  2. npm run dev
SoonIter commented 1 month ago

we need to rename the top-level variables when merging two mdx files to solve this issue

https://github.com/webpack/webpack/pull/18348/files#diff-820a04017e95cec05fd58ca44cd7f6c9cfa5b698a144471d5a0f523932486c56R1403

shulaoda commented 1 month ago

I can‘t reproduce the first case, is it correct?

SoonIter commented 1 month ago

I can‘t reproduce the first case, is it correct?

I'm sure

You can checkout https://github.com/SoonIter/rspress-mdx-flatten/tree/case_1 and https://github.com/SoonIter/rspress-mdx-flatten/tree/case_2

shulaoda commented 1 month ago

I'm sure

You can checkout https://github.com/SoonIter/rspress-mdx-flatten/tree/case_1 and https://github.com/SoonIter/rspress-mdx-flatten/tree/case_2

I got what you mean. The first case did not directly use the <a />, which caused me to misunderstand.