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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Doesn't work in project that depends on MDX 2 #234

Closed
bensmithett opened this issue Feb 11, 2022 · 8 comments 路 Fixed by #377
Closed

Doesn't work in project that depends on MDX 2 #234

bensmithett opened this issue Feb 11, 2022 · 8 comments 路 Fixed by #377
Labels
help wanted Extra attention is needed

Comments

@bensmithett
Copy link

bensmithett commented Feb 11, 2022

I have a project that I'm trying to upgrade to MDX 2 (where I use MDX outside of Storybook)

馃悶 Minimal reproduced test case here

After yarn installing, my node_modules looks like this:

node_modules/
  @mdx-js/
    mdx/ (2.0.0, the one my project explicitly depends on)
  storybook-builder-vite/
    node_modules/
      @mdx-js/ 
        mdx/ (1.6.22, the one storybook-builder-vite and vite-plugin-mdx depend on)
      vite-plugin-mdx/
        dist/
          imports.js (the source of the error)

I think that should be fine. storybook-builder-vite and vite-plugin-mdx should find 1.6.22 because dependency resolution.

But vite-plugin-mdx is actually attempting to load 2.0.0 from the root node_modules, and exploding because it's not finding the expected commonjs export from 1.6.22:

$ yarn storybook
yarn run v1.22.17
$ start-storybook -p 6006
info @storybook/react v6.4.18
info 
info => Loading presets
ERR! Error [ERR_REQUIRE_ESM]: require() of ES Module /Users/me/test-case-mdx-import/node_modules/@mdx-js/mdx/index.js from /Users/me/test-case-mdx-import/node_modules/storybook-builder-vite/node_modules/vite-plugin-mdx/dist/imports.js not supported.
ERR! Instead change the require of index.js in /Users/me/test-case-mdx-import/node_modules/storybook-builder-vite/node_modules/vite-plugin-mdx/dist/imports.js to a dynamic import() which is available in all CommonJS modules.
ERR!     at Object.requireMdx (/Users/me/test-case-mdx-import/node_modules/storybook-builder-vite/node_modules/vite-plugin-mdx/dist/imports.js:10:12)
ERR!     at Object.createTransformer (/Users/me/test-case-mdx-import/node_modules/storybook-builder-vite/node_modules/vite-plugin-mdx/dist/transform.js:16:27)
ERR!     at Object.configResolved (/Users/me/test-case-mdx-import/node_modules/storybook-builder-vite/node_modules/vite-plugin-mdx/dist/index.js:46:43)
ERR!     at /Users/me/test-case-mdx-import/node_modules/vite/dist/node/chunks/dep-c9c9d3e5.js:70984:127
ERR!     at Array.map (<anonymous>)
ERR!     at resolveConfig (/Users/me/test-case-mdx-import/node_modules/vite/dist/node/chunks/dep-c9c9d3e5.js:70984:35)
ERR!     at async createServer (/Users/me/test-case-mdx-import/node_modules/vite/dist/node/chunks/dep-c9c9d3e5.js:56354:20)
ERR!     at async Object.start (/Users/me/test-case-mdx-import/node_modules/storybook-builder-vite/dist/index.js:43:20)
ERR!     at async Promise.all (index 0)
ERR!     at async storybookDevServer (/Users/me/test-case-mdx-import/node_modules/@storybook/core-server/dist/cjs/dev-server.js:126:28)
ERR!     at async buildDevStandalone (/Users/me/test-case-mdx-import/node_modules/@storybook/core-server/dist/cjs/build-dev.js:115:31)
ERR!     at async buildDev (/Users/me/test-case-mdx-import/node_modules/@storybook/core-server/dist/cjs/build-dev.js:161:5)
ERR!  Error [ERR_REQUIRE_ESM]: require() of ES Module /Users/me/test-case-mdx-import/node_modules/@mdx-js/mdx/index.js from /Users/me/test-case-mdx-import/node_modules/storybook-builder-vite/node_modules/vite-plugin-mdx/dist/imports.js not supported.
ERR! Instead change the require of index.js in /Users/me/test-case-mdx-import/node_modules/storybook-builder-vite/node_modules/vite-plugin-mdx/dist/imports.js to a dynamic import() which is available in all CommonJS modules.
ERR!     at Object.requireMdx (/Users/me/test-case-mdx-import/node_modules/storybook-builder-vite/node_modules/vite-plugin-mdx/dist/imports.js:10:12)
ERR!     at Object.createTransformer (/Users/me/test-case-mdx-import/node_modules/storybook-builder-vite/node_modules/vite-plugin-mdx/dist/transform.js:16:27)
ERR!     at Object.configResolved (/Users/me/test-case-mdx-import/node_modules/storybook-builder-vite/node_modules/vite-plugin-mdx/dist/index.js:46:43)
ERR!     at /Users/me/test-case-mdx-import/node_modules/vite/dist/node/chunks/dep-c9c9d3e5.js:70984:127
ERR!     at Array.map (<anonymous>)
ERR!     at resolveConfig (/Users/me/test-case-mdx-import/node_modules/vite/dist/node/chunks/dep-c9c9d3e5.js:70984:35)
ERR!     at async createServer (/Users/me/test-case-mdx-import/node_modules/vite/dist/node/chunks/dep-c9c9d3e5.js:56354:20)
ERR!     at async Object.start (/Users/me/test-case-mdx-import/node_modules/storybook-builder-vite/dist/index.js:43:20)
ERR!     at async Promise.all (index 0)
ERR!     at async storybookDevServer (/Users/me/test-case-mdx-import/node_modules/@storybook/core-server/dist/cjs/dev-server.js:126:28)
ERR!     at async buildDevStandalone (/Users/me/test-case-mdx-import/node_modules/@storybook/core-server/dist/cjs/build-dev.js:115:31)
ERR!     at async buildDev (/Users/me/test-case-mdx-import/node_modules/@storybook/core-server/dist/cjs/build-dev.js:161:5) {
ERR!   code: 'ERR_REQUIRE_ESM'
ERR! }

If I yarn remove @mdx-js/mdx (so the only one left in the project is the one you're depending on) everything works.


I don't know anything about Vite plugins or Storybook builders to know where to start on this one.

I know vite-plugin-mdx isn't this repo, but I could only reproduce the error when I coupled it with a storybook setup like this.

Cheers!

@IanVS
Copy link
Member

IanVS commented Feb 15, 2022

@bensmithett does storybook itself support MDX 2? I thought I heard the maintainers talking about adding support or troubleshooting it, but I could be wrong there.

@bensmithett
Copy link
Author

@IanVS I don't think so, this is the only really relevant thing I could find that seems to indicate they're sticking on 1 for now storybookjs/storybook#17455 (comment)

@IanVS IanVS added the upstream This is an issue with another package label Feb 16, 2022
@IanVS
Copy link
Member

IanVS commented Feb 16, 2022

Thanks for looking into that. Do you think we can close this issue until/unless MDX 2 is supported by storybook?

@bensmithett
Copy link
Author

@IanVS couple of reasons I don't think we should close this (but I have a personal interest in getting it working 馃槃 so take this with a grain of salt)

  • It seems unique to storybook-builder-vite. If I delete this and go with the default builder, it works fine
  • It doesn't feel like the dependency versions used by Storybook internally should (or usually do) dictate the dependencies of application code. To be clear I'm not trying to get Storybook to use MDX 2 for .stories.mdx files, they can upgrade at their pace, but I think my application code should be able to specify its dependencies independent of Storybook's internals.

@IanVS
Copy link
Member

IanVS commented Feb 17, 2022

Ah I see, I misunderstood the original issue, thanks. I don't use MDX personally, so I'll need to rely on community members to step in here.

@IanVS IanVS added help wanted Extra attention is needed and removed upstream This is an issue with another package labels Feb 17, 2022
@Stunext
Copy link

Stunext commented May 6, 2022

I have this repository https://github.com/Stunext/docsBug with React 18 and MDX2... I have this error in the console:

Uncaught SyntaxError: The requested module '/node_modules/.vite-storybook/deps/@mdx-js_react.js?v=7dce4f03' does not provide an export named 'mdx'

This happens in mdx files. The root of this error can be seen in the source tab at the line:

import React from 'react'
import { mdx } from '@mdx-js/react' <- This line

I think it's important not to close this issue because mdx2 will be the base for storybook 7. Even mdx2 has support for @mdx-js/esbuild and @mdx-js/rollup.

mdx1.x use this sintax:

import mdx from '@mdx-js/mdx'

const result = await mdx('# hi')

console.log(result)

Instead, mdx2 uses:

import {compile} from '@mdx-js/mdx'

const result = await compile('# hi', {
  providerImportSource: '@mdx-js/react',
})

console.log(String(result))

Migration instructions here https://mdxjs.com/migrating/v2/

@IanVS
Copy link
Member

IanVS commented May 6, 2022

Thanks @Stunext. I think we need to check for the previewMdx2 feature flag and use @storybook/mdx2-csf here:

import { createCompiler } from '@storybook/csf-tools/mdx';
. I'll be happy to review a PR if anyone wants to jump in and make one. Otherwise, I'll try to get to it soon. Honestly I'm not sure if my suggestion is right, but it might be a starting point.

@IanVS IanVS mentioned this issue May 11, 2022
1 task
@IanVS
Copy link
Member

IanVS commented May 13, 2022

I've spent some time digging into this, and made some progress. The main issue right now is that the @storybook/mdx2-csf package uses babel to process the jsx, adding the react-runtime commonjs dependency. Instead, we need the jsx to be intact so that we can pipe the resulting code through @vitejs/plugin-react to get the vite react refresh set up. @shilman is going to take a look at providing a way to avoid this babel transform for the vite builder.

@IanVS IanVS mentioned this issue Jun 29, 2022
IanVS added a commit that referenced this issue Jun 29, 2022
Fixes #234 
Fixes #391
Fixes #398

To enable experimental MDX2 in a project, follow this guide: https://gist.github.com/shilman/6ff2d7e18db8846e8fc552fb432ae4f6

* Support MDXv2

* Fix formatting

* Remove explicit mdx-js/preact from example

* Remove workarounds from readme

* Add @storybook/mdx2-csf to examples using it

Co-authored-by: Ian VanSchooten <ian.vanschooten@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants