Addon-Docs: Resolve providerImportSource to a path instead of a file:// URL#34841
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe MDX addon configuration now converts resolved module specifiers to filesystem paths using Node's fileURLToPath(import.meta.resolve(...)) when setting mdxCompileOptions.providerImportSource in both the standalone MDX plugin and the Webpack preset. ChangesMDX Provider Import Path Format
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Sidnioulz
left a comment
There was a problem hiding this comment.
I could not reproduce the original issue. Creating a new React project with Vite 5, and installing Storybook 10.4 on it, I did not encounter any errors.
Could you please provide a minimal reproduction repository so I can see how to reproduce the bug and correctly test your PR? Thanks 🙏
…// URL `import.meta.resolve()` returns a file:// URL in Node ESM. The MDX compiler emits that value verbatim as the import specifier for `providerImportSource`, and Vite's import-analysis plugin cannot resolve file:// URL specifiers — so .mdx files fail to compile. Wrap both `providerImportSource` resolutions (the Vite plugin and the webpack preset) in `fileURLToPath()`, matching how `mdx` and the MDX `loader` are already resolved in the same files. Fixes storybookjs#34545
60d6bf9 to
207cc7b
Compare
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 184 | 184 | 0 |
| Self size | 79 KB | 79 KB | 0 B |
| Dependency size | 33.24 MB | 33.34 MB | 🚨 +94 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/ember
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 188 | 188 | 0 |
| Self size | 15 KB | 15 KB | 🚨 +18 B 🚨 |
| Dependency size | 29.96 MB | 30.06 MB | 🚨 +94 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/nextjs
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 534 | 534 | 0 |
| Self size | 662 KB | 662 KB | 0 B |
| Dependency size | 61.37 MB | 61.46 MB | 🚨 +94 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 271 | 271 | 0 |
| Self size | 23 KB | 23 KB | 🚨 +12 B 🚨 |
| Dependency size | 45.91 MB | 46.00 MB | 🚨 +94 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/server-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 196 | 196 | 0 |
| Self size | 16 KB | 16 KB | 0 B |
| Dependency size | 34.51 MB | 34.60 MB | 🚨 +94 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/preset-react-webpack
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 164 | 164 | 0 |
| Self size | 18 KB | 18 KB | 0 B |
| Dependency size | 32.25 MB | 32.34 MB | 🚨 +94 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
|
Minimal reproduction: https://github.com/TheSeydiCharyyev/storybook-34545-repro
The Intro page fails (
|
Sidnioulz
left a comment
There was a problem hiding this comment.
Thanks for the repro environment @TheSeydiCharyyev ❤️
I could run a canary release of the PR against it and confirm it fixes the bug. Let's merge this!
Closes #34545
What I did
providerImportSourcefor the MDX compiler was set directly fromimport.meta.resolve('@storybook/addon-docs/mdx-react-shim'). In Node ESM,import.meta.resolve()returns afile://URL, and the MDX compiler uses that value verbatim as the import specifier in its output:Vite's
vite:import-analysisplugin cannot resolvefile://URL specifiers, so every.mdxfile fails to compile.providerImportSourcewas the onlyimport.meta.resolve()call in these files not wrapped infileURLToPath()—mdxand the MDXloaderinpreset.tsalready convert to a path. This applies the same treatment in both the Vite plugin (mdx-plugin.ts) and the webpack preset (preset.ts).Manual testing
Minimal repro — a
@storybook/react-viteproject with one.mdxfile, on Vite 5:Before —
storybook devfails immediately with avite:import-analysiserror:After — the MDX docs page renders:
What I changed
code/addons/docs/src/mdx-plugin.ts— importfileURLToPath, wrapproviderImportSourcecode/addons/docs/src/preset.ts— wrapproviderImportSource(fileURLToPathalready imported)Checklist
addon-docscompiles (yarn nx compile addon-docs)No unit test added:
providerImportSourceis build-time preset wiring aroundimport.meta.resolve(), which is environment-dependent and not meaningfully isolatable. The end-to-end repro is the verification.Summary by CodeRabbit