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

Move webpack to peer dependencies #556

Closed
wants to merge 2 commits into from
Closed

Conversation

sheerun
Copy link

@sheerun sheerun commented Oct 20, 2016

This makes it possible to use react-storybook with webpack 2.

For now storybook will pass configuration from .storybook/webpack.config.js to webpack 1 even parent project specified webpack 2 as dependency. This makes storybook to re-use webpack version from parent project instead of introducing its own.

It really works, I've tested it on my own project.

@Zalastax
Copy link

The changes work for me as well. I just had to provide my own webpack config: https://github.com/Zalastax/openschedule/blob/master/.storybook/webpack.config.js

@arunoda
Copy link
Member

arunoda commented Oct 24, 2016

We can't make this a peer dep because, webpack doesn't installed by default with CRA.
But after the webpack 2 official release, we should try to do something.

@Zalastax
Copy link

Can't you just change the installation instructions/getstorybook? I've seen a lot of libraries do that.

@sheerun
Copy link
Author

sheerun commented Oct 24, 2016

If anyone wants these changes right now, you can try @sheerun/storybook

@arunoda Then maybe change require call to discover webpack dependency of parent package, if it exists?

@arunoda
Copy link
Member

arunoda commented Oct 24, 2016

@Zalastax that's also a good idea to do that on the getstorybook side.
So, we could try to add webpack as an dependency if there's not.

@jquense
Copy link

jquense commented Dec 1, 2016

can we at least increase the range on the webpack direct dep? at least then deduping would allow this to work

@arunoda
Copy link
Member

arunoda commented Dec 2, 2016

@jquense yep. That's something we could do now.
Could you send me a PR?

jquense added a commit to jquense/react-storybook that referenced this pull request Dec 2, 2016
Increase allowable webpack range, for folks using webpack 2, now webpack can be deduped in most cases so the users chosen version is used.

related:  storybookjs#556
@jquense jquense mentioned this pull request Dec 2, 2016
@cogell
Copy link

cogell commented Dec 15, 2016

@ndelangen
Copy link
Member

Thank you all, I hope we can do this fairly soon. I resolved the merge conflict, but this needs a very thorough review.

@ndelangen ndelangen self-assigned this Mar 28, 2017
@ndelangen
Copy link
Member

ndelangen commented Apr 1, 2017

Very similar to this #637

Thank you @sheerun for contributing, simply moving webpack to peerDependencies and allowing any version does not solve the problem we have with supporting webpack 2.

Good news is it IS now also under consideration here:
https://github.com/storybooks/mono/pull/4

I need to do a great deal of testing on this.

@ndelangen
Copy link
Member

So don't feel bad or like this isn't going to happen, because it's likely going to happen SOON!

I'm closing these PR because they will not be merged, because I'm doing them all in 1 swoop.

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.

6 participants