ci: fix PnP e2e test #13059
ci: fix PnP e2e test #13059
Conversation
| @@ -159,6 +159,7 @@ export const sfcVue: Parameters = { | |||
| generator: fromDeps('vue', 'vue-loader', 'vue-template-compiler'), | |||
| additionalDeps: [ | |||
| 'webpack@webpack-4', | |||
| 'react', | |||
There was a problem hiding this comment.
While this is what is required I'm assuming you don't actually want Vue users to have to install React. Perhaps Storybook should set an alias for react to make sure it's resolved from the core?
@gaetanmaisse seems to have "regressed" in #12972
There was a problem hiding this comment.
~It's weird as in the stated PR I added react to the deps of @storybook/vue: https://github.com/storybookjs/storybook/pull/12972/files#diff-3133f51bc714c0aa1f09aa806d0b57b4727c48342f4c570ffe531942fa17e074R42
So we shouldn't need to add react as a direct dep to make everything work together 🤔 ~
There was a problem hiding this comment.
Hmm pb comes from @storybook/addon-docs and not @storybook/vue:
ERR! Module not found: Error: @storybook/addon-docs tried to access react (a peer dependency) but it isn't provided by your application; this makes the require call ambiguous and unsound.
ERR!
ERR! Required package: react (via "react")
There was a problem hiding this comment.
So we are back to the issue we discussed a week ago with other SB foxes: #12972 (comment)
@merceyz As a Yarn 2 master, do you have any idea?
There was a problem hiding this comment.
Perhaps Storybook should set an alias for react to make sure it's resolved from the core?
Are you speaking of a webpack alias? Cause I guess we already have one to be sure to use a single version of react, @ndelangen can confirm. However, it doesn't solve our missing (peer)dependency issues 😞
There was a problem hiding this comment.
do you have any idea?
Either dependency injection or have @storybook/vue export react so that the vue part of the docs would import it from there but that only works if nothing else dependson react. The latter would give better treeshaking.
Are you speaking of a webpack alias?
Yes
However, it doesn't solve our missing (peer)dependency issues disappointed
That is true, if none of the dependencies the docs has needs react, importing react from @storybook/vue seems to be the best option
|
I'm not a big fan of adding yarn "binary" in SB already huge repo (ok, it's only ~1.5Mb but still). I followed the |
|
@merceyz I more or less apply what you did in that PR but prefer what you suggest here as we are trying to move the PR #13129. I would also enjoy discussing more deeply with you the |
|
Sounds good, i'll close this then |
Issue: Fixes #12928
The PnP e2e tests are failing due to PnP issues in
create-react-appand a recent update to fsevents.What I did
Applied the changes from facebook/create-react-app#9872 using
packageExtensionsand grabbed the latest build of Yarn from the repo.The
yarn.jsfile is a copy of the bundle from https://github.com/yarnpkg/berry/actions/runs/354405528 if you want to verify that it isn't malicious.How to test
CI should be green