Addon-docs: Replace source-loader with csf-plugin#19680
Conversation
shilman
added
maintenance
JReinhold
left a comment
JReinhold
left a comment
There was a problem hiding this comment.
This looks good!
I'm not worried about the breaking change since it's fairly easy for users to migrate.
| const addParameter = template(` | ||
| %%key%%.parameters = { storySource: { source: %%source%% }, ...%%key%%.parameters }; | ||
| `)({ | ||
| key: t.identifier(key), | ||
| source: t.stringLiteral(source), | ||
| }) as t.Statement; |
There was a problem hiding this comment.
Without knowing too much about what goes on behind the scenes, on the surface this looks like something that could be handled with a simple template literal instead of requiring babel/template?
There was a problem hiding this comment.
No, this needs to be babel AST nodes. The other option is to do it without @babel/template is to construct the AST by hand, which is more verbose and not very human-readable. I was thinking of doing it as an optimization, and will create a telescoping PR to see if we like that better.
There was a problem hiding this comment.
Yeah I see those changes. I don't know how much faster it is, but the raw AST is definitely less readable that template.
It might look okay now, but coming back to it in a year to do some modifications sounds like a nightmare.
Co-authored-by: Tom Coleman <tom@chromatic.com>
… into shilman/csf-plugin
| babelOptions, | ||
| mdxBabelOptions, | ||
| configureJSX = true, | ||
| sourceLoaderOptions = { injectStoryParameters: true }, |
There was a problem hiding this comment.
Would it make sense to still accept this property with the purpose of warning users/throwing an error and directing them to the migration docs?
So that it runs on code that still contains comments
…description CSF-tools: Turn story comments into docs descriptions
| ], | ||
| "scripts": { | ||
| "check": "../../../scripts/node_modules/.bin/tsc --noEmit", | ||
| "prep": "node ../../../scripts/prepare.js" |
There was a problem hiding this comment.
@shilman was there a reason not to use tsup for bundling this?
| }, | ||
| viteFinal: (vite) => ({ | ||
| ...vite, | ||
| plugins: [...(vite.plugins || []), csfPlugin({})], |
There was a problem hiding this comment.
@shilman coming back to this.
Why are we explicitly adding this to our UI SB instead of adding it per default through builder-vite ?
5 participants
Issue: N/A
csf-pluginis asource-loaderreplacement that is dramatically simpler & easier to maintain.It also supports both Webpack & Vite, providing a proof of concept for how we might do cross-builder addons
in the future.
In addition to the maintenance implications,
addon-storysourceandaddon-docsboth usedsource-loader, but in conflicting ways. So to use both at the same time, users needed to jump through configuration hoops.What I did
csf-toolsto provide basic static source extractioncsf-pluginthat provides that extraction for both Weback & Viteuistorybook to test ViteWhat I need
@joshwooding @IanVS let's figure out the best place to integrate it into Vite. Should
addon-docsget aviteFinal?How to test
Then navigate to the Controls stories & view source on the DocsPage