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

Allow Webpack 2 #637

Closed
wants to merge 1 commit into from
Closed

Allow Webpack 2 #637

wants to merge 1 commit into from

Conversation

jquense
Copy link

@jquense jquense commented 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. This may require that webpack be specified in devDeps as well if you want to test against v1 only

related: #556

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
@arunoda
Copy link
Member

arunoda commented Dec 5, 2016

I just checked and this is working pretty great with CRA setup too.
I think we could take this in.

@arunoda
Copy link
Member

arunoda commented Dec 5, 2016

Actually in that case it uses Webpack 1 behind the scene. (Since CRA uses Webpack 1 and even beta versions are resolved by default)

When we updated to Webpack 2 by force it gives us few errors. We may need to change our default config as well.

/private/tmp/abcd/node_modules/webpack/lib/webpack.js:17
		throw new WebpackOptionsValidationError(webpackOptionsValidationErrors);
		^
WebpackOptionsValidationError: Invalid configuration object. Webpack has been initialised using a configuration object that does not match the API schema.
 - configuration has an unknown property 'postcss'. These properties are valid:
   object { amd?, bail?, cache?, context?, dependencies?, devServer?, devtool?, entry, externals?, loader?, module?, name?, node?, output?, plugins?, profile?, recordsInputPath?, recordsOutputPath?, recordsPath?, resolve?, resolveLoader?, stats?, target?, watch?, watchOptions? }
   For typos: please correct them.
   For loader options: webpack 2 no longer allows custom properties in configuration.
     Loaders should be updated to allow passing options via loader options in module.rules.
     Until loaders are updated one can use the LoaderOptionsPlugin to pass these options to the loader:
     plugins: [
       new webpack.LoaderOptionsPlugin({
         // test: /\.xxx$/, // may apply this only for some modules
         options: {
           postcss: ...
         }
       })
     ]
 - configuration.resolve has an unknown property 'fallback'. These properties are valid:
   object { alias?, aliasFields?, cachePredicate?, descriptionFiles?, enforceExtension?, enforceModuleExtension?, extensions?, fileSystem?, mainFields?, mainFiles?, moduleExtensions?, modules?, plugins?, resolver?, symlinks?, unsafeCache? }
 - configuration.resolve.extensions[3] should not be empty.
    at webpack (/private/tmp/abcd/node_modules/webpack/lib/webpack.js:17:9)
    at exports.default (/private/tmp/abcd/node_modules/@kadira/storybook/dist/server/middleware.js:19:40)
    at Object.<anonymous> (/private/tmp/abcd/node_modules/@kadira/storybook/dist/server/index.js:127:34)
    at Module._compile (module.js:556:32)
    at Object.Module._extensions..js (module.js:565:10)
    at Module.load (module.js:473:32)
    at tryModuleLoad (module.js:432:12)
    at Function.Module._load (module.js:424:3)
    at Module.runMain (module.js:590:10)
    at run (bootstrap_node.js:394:7)

@arunoda arunoda mentioned this pull request Dec 15, 2016
@romeovs
Copy link

romeovs commented Jan 5, 2017

What's the status of this?

@silvenon
Copy link

It's probably tricky, because a single configuration can't cover both webpack versions?

@sapegin
Copy link

sapegin commented Jan 14, 2017

Most probably you’ll need to detect Webpack version and use appropriate config because they aren’t always compatible and Webpack 2 has validation that yells at valid Webpack 1 configs.

We do that in Styleguidist and it works well: https://github.com/styleguidist/react-styleguidist/blob/next/scripts/make-webpack-config.js

@gabeweaver
Copy link

I'd love to see this happen. The divide between webpack 1 and webpack 2 is so large at this point that it requires maintaining two separate configs. I regularly spend a few hours a week just tracking down weird webpack issues with storybooks since my app is on webpack 2 and I can't share any of the configs at this point.

The approach above seems like the best bet given the wide distribution of storybooks and the fact that it's likely to take months for everyone to fully migrate to webpack 2.

Anything I can do to help?

@TheSisb
Copy link

TheSisb commented Jan 24, 2017

Doubling down on what @gabeweaver said. Anything I could do to help move this forward?

@joscha
Copy link
Member

joscha commented Jan 31, 2017

Is there any news on this?

@romeovs
Copy link

romeovs commented Feb 1, 2017

I fiddled a bit in https://github.com/romeovs/react-storybook
I feel it's pretty hard to test whether this works because I have to install this project in another project to make sure it does.

At least with the changes in https://github.com/romeovs/react-storybook there's no more webpack errors 👍

@joscha
Copy link
Member

joscha commented Feb 7, 2017

@arunoda could we get some attention on this issue? With more and more projects adopting webpack 2, this means that it will become more and more common to have to maintain a duplicate webpack 1 config for storybook only.

@moimikey
Copy link
Contributor

moimikey commented Feb 8, 2017

i'm having the same scenario. i can't wait for this to get fixed, so my interim solution is deep renaming keys:

https://runkit.com/587e7a68a1e7580014cd2541/589b3097b360a100148838fe

var renameKeys = require("deep-rename-keys")
var config = {
module: { loaders: [ [Object], [Object], [Object], [Object], [Object] ] }}
var transformToWebpack2 = function(key) {
  if (key === 'loaders') return 'rules'
  return key
}
renameKeys(config, transformToWebpack2)

@moimikey
Copy link
Contributor

moimikey commented Feb 8, 2017

also, for what it's worth, i ended up doing this to get it to work:

// .storybook/webpack.config.js
const renameKeys = require('deep-rename-keys')
const baseWebpack = require('base-webpack')() // this is my own custom package with base webpack config
const storybookConfig = require('@kadira/storybook/dist/server/config/defaults/webpack.config.js')

module.exports = function (config, env) {
  const webpackConfig = storybookConfig(config, env)

  // add additional aliases to react-storybook's initial webpack config. i'm doing this because
  // i'm using 'preact' and need to resolve.alias react & react-dom to preact-compat
  for (alias in baseWebpack.resolve.alias) {
    webpackConfig.resolve.alias[alias] = baseWebpack.resolve.alias[alias]
  }

  // remove unneeded keys
  delete webpackConfig.devtool
  delete webpackConfig.plugins['OccurrenceOrderPlugin']
  delete webpackConfig.postcss // this i need to figure out :/
  delete webpackConfig.resolve.extensions
  delete webpackConfig.resolve.fallback

  // store plugins in a temporary object, so renameKeys doesnt screw
  // up the paths
  const webpackPlugins = webpackConfig.plugins

  // rename webpack config keys for webpack 2 support until react-storybook
  // supports this out of the box
  const transformToWebpack2 = function (key) {
    // 'loaders' and 'loader' i commented out since theres now interop support for these
    // if (key === 'loaders') return 'rules' 
    // if (key === 'loader') return 'use'
    if (key === 'query') return 'options'
    if (key === 'modulesDirectories') return 'modules'
    return key
  }

  const transformedWebpack = renameKeys(webpackConfig, transformToWebpack2)

  transformedWebpack.plugins = webpackPlugins

  return transformedWebpack
}

@joscha
Copy link
Member

joscha commented Feb 9, 2017

@moimikey :-O - we also use a few features from webpack 2 however that are not easily translatable into a webpack 1 world - @arunoda it seems there are quite a few people stranded with workarounds - would you have a look at https://github.com/romeovs/react-storybook to see if this is something we can merge or adapt to get to a solution, please?

@augbog
Copy link

augbog commented Feb 10, 2017

👍 for this issue would love to see support

@joscha joscha mentioned this pull request Feb 12, 2017
@gconaty
Copy link

gconaty commented Feb 14, 2017

I found one issue with @romeovs fork otherwise it worked well for me.
Namely the process.env doesn't get built properly. However this workaround works:
gconaty@9769cbe

Would be helpful to get these merged. A bit odd that I've got to check in the 'dist' folders to get it to work with other repos. Guessing because only the 'official' publish to NPM will run the prepublish script.

@joscha
Copy link
Member

joscha commented Feb 14, 2017

I found one issue with @romeovs fork otherwise it worked well for me.
Namely the process.env doesn't get built properly. However this workaround works:
gconaty/react-storybook@9769cbe

We have a similar workaround that is independent of the webpack version and could potentially go directly into master - see https://github.com/canva/react-storybook/commit/4bc2435109c46c35c76f8a81fb9547b16d939648 /cc @WearyMonkey

@mnquintana
Copy link

@arunoda 👋 What would it take to get the Webpack 2 upgrade merged? It looks like this PR's pointing to an old beta release of Webpack 2 – I'd be happy to open a new one targeting the latest stable release of Webpack 2, if that would be helpful.

@ntucker
Copy link

ntucker commented Feb 27, 2017

@romeovs I tried using your fork via github include with yarn, and I got "Cannot find module './WatchMissingNodeModulesPlugin'"

#656 seems to suggest this is yarn related? Any ideas?

@ntucker
Copy link

ntucker commented Feb 27, 2017

I use my own webpack config. If I could simply make storybook use webpack 2 that I installed, nothing would need to be converted. For the interim could storybook simply use webpack 2 if it's already installed. people using it would likely have their own configs.

Changing this would easily make this work for those who already migrated, while allowing others to stay on webpack 1 until a better plan is laid out.

@xtuc
Copy link

xtuc commented Feb 28, 2017

Changing this would easily make this work for those who already migrated, while allowing others to stay on webpack 1 until a better plan is laid out.

Please consider releasing a major version of storybook with Webpack2 support. This will avoid users with custom Webpack configuration to break unnecessarily.

@joscha
Copy link
Member

joscha commented Mar 9, 2017

@arunoda is there anything we can do to help to get this sorted? I am happy to throw some time at it. We are currently running on a fork because we thought it would be a matter of days to get at least any of the solutions here merged, however now three months later I am getting a bit nervous :)

@SEAPUNK
Copy link

SEAPUNK commented Mar 9, 2017

For reference, we have a fork of Canva's fork over at https://github.com/hammerandchisel/react-storybook that seems to work well enough for us in the meantime.

@ntucker
Copy link

ntucker commented Mar 9, 2017

I'm running off a fork too (steckel). Had to build the lib files to install via github so I forked the fork here: https://github.com/ntucker/react-storybook

@PavelPolyakov
Copy link

looking forward for the option of webpack 2

@scinos
Copy link

scinos commented Mar 27, 2017

What is the status of this change? Is there anything blocking it?

@xtuc
Copy link

xtuc commented Mar 27, 2017

I guess the reason is #731 (comment)

If someone wants to fork it, merge this and maintain it 👍

@joscha
Copy link
Member

joscha commented Mar 27, 2017

@arunoda would you be happy to give some people here contrib access, so we can keep this awesome tool alive?

@PavelPolyakov
Copy link

oh god, it was just presented on the recent React conf and now it's dead?
looking forward for the community support.

even in the current state I found storybook very useful.

@mnquintana
Copy link

@arunoda @joscha I would be more than happy to help out with the project as a contributor if y'all will have me - we ❤️ storybooks here at Slack, and I'd love to be able to give back.

@joscha
Copy link
Member

joscha commented Mar 27, 2017

@mnquintana sounds great! Unfortunately @arunoda has not answered, yet, I am holding off for another few days before sending an email. Would be great if we could jointly continue this project!

@Vinnl Vinnl mentioned this pull request Mar 28, 2017
@ndelangen
Copy link
Member

ndelangen commented Mar 28, 2017

I will add webpack 2 support ASAP!

@ndelangen
Copy link
Member

So this is for subscribers on this PR: Here's my progress:
I'm in the process of creating a monorepo that will hold all storybook repos as project. This will make maintenance and testing a lot better and easier.

I've done all the work to support webpack 2 on here.
https://github.com/storybooks/mono/blob/master/packages/react-storybook/package.json#L61

I'd like to thank @jquense for this PR. I hope it's understandable this PR will not get merged, as it does not solve the problem, as @arunoda has commented.

I'll keep you all updated on the progress! Thank you all!

@SEAPUNK
Copy link

SEAPUNK commented Apr 1, 2017

@ndelangen Thank you! We appreciate it.

@ndelangen
Copy link
Member

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

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

@ndelangen ndelangen closed this Apr 1, 2017
@possibilities
Copy link

possibilities commented Apr 1, 2017 via email

@domwashburn
Copy link

I'm only just now finding this thread. I wish I would have seen it a lot earlier today as it would have saved me a lot of time and headaches trying to add Storybook to my Webpack 2 build.

I really want to use Storybook, but I dread the thought of having to re-write my config in webpack 1 just to do it... I'd really appreciate any update that you can give on the status of Webpack 2 integration.

@dozoisch
Copy link

@domwashburn Im using my webpack2 config right now with this fork ntucker/react-storybook#steckel/webpack-2-compatible

@domwashburn
Copy link

Thanks @dozoisch! That's going to be a huge help, hopefully Webpack 2 support can be integrated into Storybook soon :)

@ntucker
Copy link

ntucker commented Apr 24, 2017

I updated my fork to include some needed fixes for webpack 2 support: https://github.com/ntucker/react-storybook/commits/steckel/webpack-2-compatible (commit here: ntucker@d01b097)

@ndelangen You might consider supporting resolveLoader and custom module.rules like I did here: ntucker@d01b097 as my heavily customized webpack wouldn't work without such things.

@jquense jquense deleted the patch-1 branch April 24, 2017 20:20
@row1
Copy link

row1 commented May 1, 2017

@ntucker what is the recommended way of consuming your fork? Install the older version via npm then copy your dist folder somewhere?

@creaux
Copy link

creaux commented May 1, 2017

@ntucker are you planning to make a pull-request for your updates around webpack 2?

@ntucker
Copy link

ntucker commented May 1, 2017

@row1 yarn add @kadira/storybook@git+ssh://git@github.com/ntucker/react-storybook#steckel/webpack-2-compatible

@ntucker
Copy link

ntucker commented May 1, 2017

@creaux No plans for a PR. The maintainers are already working toward a release with compatibility. However, since they use a monorepo one cannot install via the git method so my repo is still quite useful.

@Pau1fitz
Copy link
Contributor

Pau1fitz commented Jun 6, 2017

@ndelangen so what came of this? is react-storybook now compatible with webpack 2?

@SEAPUNK
Copy link

SEAPUNK commented Jun 6, 2017

@Pau1fitz yep, 3.x was recently released; see the changelog and docs for more info.

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.

None yet