-
-
Notifications
You must be signed in to change notification settings - Fork 9.8k
Yarn PnP: Add missing dependencies for Webpack 4/5 work #13992
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
Conversation
shilman
left a comment
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.
Thanks so much for cleaning this up. How did you figure out the list of changes?
addons/docs/package.json
Outdated
| "@mdx-js/react": "^1.6.22", | ||
| "@storybook/addons": "6.2.0-alpha.32", | ||
| "@storybook/api": "6.2.0-alpha.32", | ||
| "@storybook/builder-webpack4": "^6.2.0-alpha.32", |
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.
@gaetanmaisse Why do we need 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.
Because of this line:
| paths: [require.resolve('@storybook/builder-webpack4')], // FIXME!!! |
lib/core-server/package.json
Outdated
| "@babel/preset-react": "^7.12.10", | ||
| "@storybook/addons": "6.2.0-alpha.32", | ||
| "@storybook/builder-webpack4": "6.2.0-alpha.32", | ||
| "@storybook/core-client": "6.2.0-alpha.32", |
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.
Why do we need 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.
As it's used as a default value in
| const { managerEntry = '@storybook/core-client/dist/esm/manager' } = options; |
I ran the PnP E2E test locally: yarn bootstrap --core
yarn local-registry --port 6000 --open --publish
# In another terminal
yarn test:e2e-framework --use-yarn-2-pnp --use-local-sb-cli cra
# To simplify the debug I also linked the SB packages to the monorepo to avoid to republish each time a change is made
# from the monorepo
cd ../storybook-e2e-testing/cra-latest
yarn link ../../storybook --allAnd then I fixed the |
What I did
I added all the missing dependencies listed by Yarn 2 E2E tests.
How to test