-
-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
Renderer React: first example of multiframework support using decorators #6826
Conversation
This pull request is being automatically deployed with ZEIT Now (learn more). 🔍 Inspect: https://zeit.co/storybook/monorepo/g9ijbqng0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since it's a POC I am not sure how deep we need to go with comments, but anyway =)
-
If we already moving presets out (here in react, but after there will be others), it's better to move them into their own packages, that's how it will be easier to customise them after (maybe even to the sb/presets repo)
-
in the babelrc we have an overrides for the server code to transpile for node 8, so we need to add there frameworks as well (if there will be a server code there)
app/react/src/server/options.js
Outdated
@@ -3,8 +3,8 @@ import packageJson from '../../package.json'; | |||
export default { | |||
packageJson, | |||
frameworkPresets: [ | |||
require.resolve('./framework-preset-react.js'), | |||
require.resolve('@storybook/addon-react/dist/server/framework-preset-react.js'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's better to have the presets more accessible, something like:
import { presets } from '@storybook/addon-react';
/* blah blah blah */
frameworkPresets: [
presets.react,
require.resolve('./framework-preset-cra.js'),
presets.reactDocgen,
]
also it will be more formatted way for frameworks to expose their presets.
Another option will be hide the particular presets, for example:
import { createPresets } from '@storybook/addon-react';
/* blah blah blah */
frameworkPresets: [
...createPresets({
afterReact: [require.resolve('./framework-preset-cra.js')],
});
]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we can mix preview and server code in one endpoint, so it probably can't be just @storybook/addon-react
.
I think I'll make it just @storybook/addon-react/preset
after we merge #6828
Creating a separate package makes sense to me, but can we leave the presets that we use internally in monorepo for easier syncing?
It's already under |
@igor-dv BTW it's not only about presets. I want stuff like Honestly, I think it's no harm to have the app depend on the add-on. Maybe at some point we can even make the app just a wrapper for addon + |
makes sense.
That what I see now in the babelrc.
I think those kind of pkgs we can put under the
Agree with that |
# Conflicts: # .teamcity/OpenSourceProjects_Storybook/buildTypes/OpenSourceProjects_Storybook_Bootstrap.kt # app/react/package.json # examples/html-kitchen-sink/package.json
# Conflicts: # app/react/package.json # examples/html-kitchen-sink/package.json
# Conflicts: # .babelrc.js # app/react/package.json # examples/html-kitchen-sink/.storybook/presets.js # examples/html-kitchen-sink/package.json
@shilman what's the easiest way to migrate our own examples to module format? I'm talking about https://github.com/storybookjs/storybook/tree/addon-react/examples/html-kitchen-sink/stories/react |
# Conflicts: # app/react/src/client/preview/render.tsx
I'm so excited about this. My team will benefit from this exact scenario. We have a storybook for HTML "components" and React components right now, but we're forced to render the React components now. |
Issue: #3889
TODO:
@storybook/html
@storybook/react
to@storybook/renderer-react
Includes changes from #7791