-
-
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
Core: Fix package duplication issues by aliasing all storybook packages #11092
Conversation
@@ -21,6 +23,29 @@ import { toRequireContextString } from './to-require-context'; | |||
import { useBaseTsSupport } from '../config/useBaseTsSupport'; | |||
|
|||
const reactPaths = {}; | |||
const storybookPaths = { |
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.
const storybookPaths = ['addons', ...other ].reduce((paths,package) => ({
...paths,
[`@storybook/${package}`]: path.dirname(resolveFrom(__dirname, `@storybook/${package}/package.json`))`
}), {});
Would reduce boilerplate a bit :)
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.
yes please 🙏
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.
As you wish 🎩
@gaetanmaisse @tooppaaa @shilman @mrmckeb want to review this? 🙇 |
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.
To tell the truth, it's a bit mysterious for me, it LGTM ✅ but I'm not confident enough to click the "Approve" button 😛
'client-api', | ||
'client-logger', | ||
].reduce( | ||
(acc, package) => ({ |
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.
package
is not a valid variable name in JS 😉 => sbPackage
@@ -52,7 +52,9 @@ | |||
"@babel/register": "^7.8.3", | |||
"@storybook/addons": "6.0.0-beta.23", | |||
"@storybook/api": "6.0.0-beta.23", | |||
"@storybook/channels": "6.0.0-beta.23", |
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.
These are added to be sure to have them and be able to have a working alias?
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.
Yeah all packages of @storybook/* that are conceivably used are aliased here so that there can only 1 of them in the entire bundle.
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.
Looks good to me.
# Conflicts: # examples/cra-kitchen-sink/.storybook/main.js
Closes #50 Closes #157 This is the same fix as storybookjs/storybook#11092
Issue: #10887
What I did
I added aliases for all storybook related packages for the preview