-
Notifications
You must be signed in to change notification settings - Fork 106
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
Inject mdx compiler for mdx1 #556
Conversation
Hm, based on the CI failures we might need to also require react to be a peer dependency like we do in SB 7. :-/ |
Looks good. :) |
Rollup does not respect NODE_PATH
Turns out that rollup does not respect NODE_PATH, so we need to do a bit of resolution hackery to get it to resolve react relative to the example, and not from within builder-vite's node_modules. Normal projects will not need to worry about this, it's just due to our non-standard multi-project example structure. |
resolveId wants an os-compatible absolute path. 🙄
@joshwooding I'm having trouble with Windows here. Any chance you can give this a shot? |
Hi @IanVS from issue 21023 CI passed when enabled so maybe this is builder-vite/packages/builder-vite/plugins/mdx-plugin.ts Lines 20 to 26 in 4d49af6
|
Yes, mdx1 requires us to inject an import into the code, and the yarn portal we are using for the examples doesn't seem to be working very well, because imports in builder-vite are not resolving back to the example's node_modules without some help. It's working fine on posix systems, but not windows. I'm tempted to merge anyway and hope that Josh can get our CI green later. It's only an issue in our examples I believe. |
fixes #557
I found that mdx was not working correctly here, and @valentinpalkovic realized my fallback in Storybook 7 wasn't either (storybookjs/storybook#20823). It was because we used to inject the mdx compiler into the source code that we get back from the
@storybook/mdx1-csf
compiler, but we lost that in https://github.com/storybookjs/builder-vite/pull/548/files#diff-f2dfc4dfa9074d77a728a88e6629a1d66be8a0765cab8562526cd00fbae910e5L6. This re-introduces it, with a bit better of a guarantee that the correct version is going to be loaded, by starting from inside@storybook/mdx1-csf
.I've also pinned mdx1-csf here, in case the import is moved from the loader to the compiler, as @shilman has suggested doing. We'll need to adjust this if that happens.