Skip to content
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

Remove dependency pinning #11474

Merged
merged 12 commits into from
Oct 1, 2021
Merged

Remove dependency pinning #11474

merged 12 commits into from
Oct 1, 2021

Conversation

mrmckeb
Copy link
Contributor

@mrmckeb mrmckeb commented Sep 22, 2021

This PR unpins babel-loader, which has been causing issues when using Create React App with Storybook.

We don't expect this to have any negative side-effects, and users can pin this themselves if needed via Yarn's resolutions or npm's upcoming overrides feature.

Related Storybook issue: storybookjs/storybook#5183.

@mrmckeb mrmckeb added this to the 5.0 milestone Sep 22, 2021
@iansu
Copy link
Contributor

iansu commented Sep 22, 2021

@mrmckeb can you take a quick look and check if there are any other pinned dependencies we want to unpin?

@merceyz
Copy link
Contributor

merceyz commented Sep 22, 2021

This is effectively treating the symptom and not the cause, the dependency check makes the flawed assumption that the dependencies of react-scripts will be hoisted to the root which is not guaranteed. The same problem will show up as soon as anything in the dependency tree depends on a version of babel-loader that is hoisted to the root and doesn't match the range react-scripts wants.

Ref storybookjs/storybook#4764 (comment)

@mrmckeb
Copy link
Contributor Author

mrmckeb commented Sep 25, 2021

Agreed @merceyz, but this will alleviate some of the pain.

I'm not sure we should keept the pre-flight checks long-term as we use require.resolve in most cases now... what do you think @iansu?

@mrmckeb
Copy link
Contributor Author

mrmckeb commented Sep 26, 2021

@iansu what do you think about unpinning Webpack too? Storybook is now ahead of us, and that's now flagged in the pre-flight check.

Long-term we need to solve this problem.

@iansu
Copy link
Contributor

iansu commented Sep 26, 2021

@mrmckeb Unpinning webpack sounds a bit scary but I think we should aim to unpin everything and see how that goes during the v5 alpha process.

@mrmckeb mrmckeb changed the title Unpin babel-loader version Remove dependency pinning Sep 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants