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

[Bug]: MDX breaks when importing a CSF from node_modules #27178

Closed
stephiescastle opened this issue May 16, 2024 · 5 comments
Closed

[Bug]: MDX breaks when importing a CSF from node_modules #27178

stephiescastle opened this issue May 16, 2024 · 5 comments

Comments

@stephiescastle
Copy link

Describe the bug

When importing a CSF from a package in node_modules, the MDX documentation fails to render, with error:

<Meta of={} /> must reference a CSF file module export or meta export

The CSF file is captured by the stories glob in main.js, and if I remove the MDX file and set up autodocs, everything works fine.

To Reproduce

I created a reproduction here: https://github.com/stephiescastle/sb-repro-mdx-import-story

Instructions are in the README and also below:

  1. pnpm install
  2. pnpm run --filter @monorepo/storybook storybook

System

Storybook Environment Info:

  System:
    OS: macOS 14.4.1
    CPU: (12) arm64 Apple M2 Pro
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 20.13.1 - ~/.nvm/versions/node/v20.13.1/bin/node
    npm: 10.5.2 - ~/.nvm/versions/node/v20.13.1/bin/npm
    pnpm: 9.1.1 - ~/.nvm/versions/node/v20.13.1/bin/pnpm <----- active
  Browsers:
    Chrome: 124.0.6367.208
    Edge: 124.0.2478.105
    Safari: 17.4.1
  npmPackages:
    @storybook/addon-essentials: ^8.1.1 => 8.1.1 
    @storybook/addon-interactions: ^8.1.1 => 8.1.1 
    @storybook/addon-links: ^8.1.1 => 8.1.1 
    @storybook/blocks: ^8.1.1 => 8.1.1 
    @storybook/html: ^8.1.1 => 8.1.1 
    @storybook/html-vite: ^8.1.1 => 8.1.1 
    @storybook/test: ^8.1.1 => 8.1.1 
    storybook: ^8.1.1 => 8.1.1

Additional context

I am working in a monorepo via pnpm workspaces. The monorepo structure has been set up in the reproduction repository.

@JReinhold
Copy link
Contributor

JReinhold commented May 17, 2024

I've done some investigation but I don't yet know why this happens.

The index looks fine:

{
  "v": 4,
  "entries": {
    "importedbutton--docs": {
      "id": "importedbutton--docs",
      "title": "ImportedButton",
      "name": "Docs",
      "importPath": "./stories/ImportedButton.mdx",
      "storiesImports": [
        "./node_modules/@monorepo/assets/src/ImportedButton.stories.js"
      ],
      "type": "docs",
      "tags": [
        "dev",
        "test",
        "attached-mdx"
      ]
    },
    "importedbutton--imported-button": {
      "type": "story",
      "id": "importedbutton--imported-button",
      "name": "Imported Button",
      "title": "ImportedButton",
      "importPath": "./node_modules/@monorepo/assets/src/ImportedButton.stories.js",
      "tags": [
        "dev",
        "test"
      ]
    }
  }
}

But

const csfFile = this.exportsToCSFFile.get(moduleExportOrType);
returns undefined, even though the story is actually in this.exportsToCSFFile.

In the repro, using the debugger and doing the get() manually with strict equality yields this:

image
image

Seems like they are the same value but the references are different.

I don't know if @tmeasday has insights into why this might happen. The fact that node_modules are a part of the path could be the problem or a red herring.

@shilman
Copy link
Member

shilman commented May 19, 2024

I'm pretty sure that node_modules is the problem. We should document this limitation in the official docs unless we are going to fix it (which is unlikely). cc @tmeasday @kylegach

@tmeasday
Copy link
Member

tmeasday commented May 20, 2024

Thanks for the reproduction @stephiescastle.

It looks like it is trying to do the right thing but the imported CSF file (from node_modules) ends up getting instantiated twice:

image

I think this is likely due to Vite's dependency prebundling leading to the file being imported twice. You can verify this by adding a console.log('imported') to the ImportedButton.stories.js file. There is a way to stop Vite doing that but I couldn't work it out 😊

I think @shilman's point remains that we don't officially support this and can't guarantee we won't break it in the future, but if you can figure out the workaround using something similar to this, please post here! (in .storybook/main.ts)

  viteFinal: (config) => ({
    ...config,
    optimizeDeps: {
      exclude: ['@monorepo/assets'],
    },
  }),

@stephiescastle
Copy link
Author

stephiescastle commented May 20, 2024

Thanks @tmeasday. I tried working with optimizeDeps as suggested, but I also couldn't work it out either.

However, I did try replacing pnpm's @monorepo/assets symlink with actual files. This fixed the issue without needing to customize the vite config, which helped me narrow down the problem to symlinks.

Vite does have a preserveSymlinks option. However, setting preserveSymlinks to true caused a couple issues with other dependencies:

  1. Different dependencies required different versions of tslib. I added a pnpm override for the specific package that gave me issues (react-remove-scroll)
  2. I had to install with --shamefully-hoist (not clear to me why this was necessary)

After both of those adjustments, things started working! Here's the commit that fixed the issue: stephiescastle/sb-repro-mdx-import-story@f6b0c89

@tmeasday
Copy link
Member

tmeasday commented May 21, 2024

Hey @stephiescastle - glad you figured it out. So I guess the TLDR is:

You can import CSF from node_modules in MDX files (although it's not officially supported by SB). If you are symlinking into node_modules, you'll need to set Vite's preserveSymlinks option [and possibly work around other issues]
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants