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

[BREAKING CHANGE] webpack 4 #3148

Merged
merged 26 commits into from
Mar 26, 2018
Merged

[BREAKING CHANGE] webpack 4 #3148

merged 26 commits into from
Mar 26, 2018

Conversation

Hypnosphi
Copy link
Member

@Hypnosphi Hypnosphi commented Mar 4, 2018

Issues: #3044, #3083, #3085, #3092, #3115, #3116, #3135

Things that block this right now

# Conflicts:
#	app/angular/package.json
#	app/polymer/package.json
#	app/react-native/package.json
#	app/react/package.json
#	app/react/src/server/config/webpack.config.js
#	app/vue/package.json
#	lib/core/package.json
@Hypnosphi Hypnosphi mentioned this pull request Mar 4, 2018
@Hypnosphi Hypnosphi changed the title [WIP, BREAKING CHANGE]Webpack 4 [WIP, BREAKING CHANGE] Webpack 4 Mar 4, 2018
@Hypnosphi Hypnosphi added this to the v4.0.0 milestone Mar 4, 2018
@Hypnosphi
Copy link
Member Author

Hypnosphi commented Mar 4, 2018

Weird, storybook build for polymer works OK locally on my machine, but on CI it's failing:

ERROR in /tmp/storybook/addons/knobs/dist/polymer/WrapStory.html
Module not found: Error: Can't resolve 'polymer-webpack-loader/register-html-template' in '/tmp/storybook/addons/knobs/dist/polymer'
 @ /tmp/storybook/addons/knobs/dist/polymer/WrapStory.html 7:27-83
 @ /tmp/storybook/addons/knobs/dist/polymer/index.js
 @ /tmp/storybook/addons/knobs/polymer.js
 @ ./src/stories/addon-knobs.stories.js
 @ ./src/stories sync \.stories\.js$
 @ ./.storybook/config.js
 @ multi /tmp/storybook/app/polymer/dist/server/config/polyfills.js /tmp/storybook/app/polymer/dist/server/config/globals.js /tmp/storybook/node_modules/webpack-hot-middleware/client.js?reload=true ./.storybook/config.js

UPD: this should be fixed with #3161

@codecov
Copy link

codecov bot commented Mar 6, 2018

Codecov Report

Merging #3148 into master will increase coverage by 0.01%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3148      +/-   ##
==========================================
+ Coverage   36.19%   36.21%   +0.01%     
==========================================
  Files         444      444              
  Lines        9741     9736       -5     
  Branches      914      905       -9     
==========================================
  Hits         3526     3526              
- Misses       5650     5655       +5     
+ Partials      565      555      -10
Impacted Files Coverage Δ
app/vue/src/server/config/webpack.config.prod.js 0% <ø> (ø) ⬆️
app/react/src/server/config/webpack.config.prod.js 0% <ø> (ø) ⬆️
...p/angular/src/server/config/webpack.config.prod.js 0% <ø> (ø) ⬆️
app/vue/src/server/config/webpack.config.js 0% <ø> (ø) ⬆️
app/polymer/src/server/config/webpack.config.js 0% <ø> (ø) ⬆️
.../core/src/server/config/defaults/webpack.config.js 0% <ø> (ø) ⬆️
...p/polymer/src/server/config/webpack.config.prod.js 0% <ø> (ø) ⬆️
app/angular/src/server/config/webpack.config.js 0% <ø> (ø) ⬆️
app/react/src/server/config/webpack.config.js 0% <ø> (ø) ⬆️
app/react-native/src/server/config/utils.js 0% <ø> (ø) ⬆️
... and 74 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1805927...530cc59. Read the comment docs.

@Hypnosphi
Copy link
Member Author

For now, I've just disabled InterpolateHtmlPlugin and WatchMissingNodeModulesPlugin which means that first 4.0 alpha will probably lack the corresponding features: .env support in *-head.html, and watching for newly installed packages

@igor-dv
Copy link
Member

igor-dv commented Mar 7, 2018

Both plugins are rather simple, maybe it worth copypasting/fixing them into the core until the official support?

@Hypnosphi
Copy link
Member Author

Hypnosphi commented Mar 7, 2018

They say they're almost ready: facebook/create-react-app#4077 (comment)

@pkuczynski
Copy link

pkuczynski commented Mar 7, 2018

Could you also in this work stream upgrade babel to 7.0 (currently in beta.40) and babel-loader to 8.0.0-beta.2? That allows using more dynamic .babelrc.js...

@Hypnosphi
Copy link
Member Author

Hypnosphi commented Mar 7, 2018

@pkuczynski sure, definitely makes sense. I think it'll be a separate PR though
And I'd like to wait for babel/babel#7472 to get merged and released first

@pkuczynski
Copy link

Cool! I can provide a PR even now, as this is not related to webpack and I am being able to successfully use babel@7 with webpack@3 in my project...

And why do you need to wait for babel/babel#7472?

@Hypnosphi
Copy link
Member Author

Hypnosphi commented Mar 7, 2018

Because it will help to solve a lot of issues we currently have because of custom babelrc resolution

See e.g. #2582 (comment) #2771 #754 #2635 #2610

@pkuczynski
Copy link

But then it would require quite some work on your side I guess? While basic upgrade to babel@7 should work out of the box now. So maybe it’s worth to do it in two steps?

@Hypnosphi
Copy link
Member Author

Hypnosphi commented Mar 7, 2018

Probably so. Feel free to give it a try

# Conflicts:
#	examples/polymer-cli/package.json
#	examples/vue-kitchen-sink/package.json
@SleepWalker
Copy link

Here is more clever candidates for fix:

InterpolateHtmlPlugin.js:

...
  apply(compiler) {
    compiler.plugin('compilation', compilation => {
      const handler = (data, callback) => {
        // Run HTML through a series of user-specified string replacements.
        Object.keys(this.replacements).forEach(key => {
          const value = this.replacements[key];
          data.html = data.html.replace(
            new RegExp('%' + escapeStringRegexp(key) + '%', 'g'),
            value
          );
        });
        callback(null, data);
      };

      if (compilation.hooks) {
        compilation.hooks.htmlWebpackPluginBeforeHtmlProcessing
          .tapAsync('InterpolateHtmlPlugin', handler);
      } else {
        compilation.plugin('html-webpack-plugin-before-html-processing', handler);
      }
    });
  }

WatchMissingNodeModulesPlugin.js:

I've converted Set into an Array, but I'm not sure whether webpack have changed only underlying type or set item type too. I hope that there are still strings as before :)

  apply(compiler) {
    compiler.plugin('emit', (compilation, callback) => {
      var missingDeps = compilation.missingDependencies;
      var nodeModulesPath = this.nodeModulesPath;

      // If any missing files are expected to appear in node_modules...
      if (Array.from(missingDeps).some(file => file.indexOf(nodeModulesPath) !== -1)) {
        // ...tell webpack to watch node_modules recursively until they appear.
        compilation.contextDependencies.push(nodeModulesPath);
      }

      callback();
    });
  }

@Hypnosphi
Copy link
Member Author

Hypnosphi commented Mar 29, 2018

It's actualy already fixed in facebook/create-react-app#4077 and we've released it from our fork as @storybook/react-dev-tools. Unfortunately, we have github path stored in our lockfile, which made me think that 5.0.0 works again because of some compatibility fix in webpack.

I'll send a hotfix PR ASAP

@danielduan
Copy link
Member

@ishantdigitalgenius try to use the same version of addons as the rest of the @storybook packages. even though there usually aren't any issues, we don't guarantee different versions working together.

@Hypnosphi
Copy link
Member Author

Fix released as 4.0.0-alpha.1

@peet86
Copy link

peet86 commented Mar 30, 2018

With 4.0.0-alpha.1 I still have the jantimon/html-webpack-plugin#870 issue.. :(

95% emitting HtmlWebpackPlugin/[..]/node_modules/@storybook/react/node_modules/toposort/index.js:29 throw new Error('Cyclic dependency: '+JSON.stringify(node)) ^ TypeError: Converting circular structure to JSON

@Hypnosphi
Copy link
Member Author

We probably need to apply workaround from jantimon/html-webpack-plugin#870 (comment)

For now, you can try to apply it from custom webpack config (full control mode):

export (baseConfig, env, defaultConfig) => ({
  ... defaultConfig,
  plugins: defaultConfig.plugins.map(plugin => {
    if (plugin.constructor.name === 'HtmlWebpackPlugin') {
      return { ...plugin, chunksSortMode: 'none' }
    }
    return plugin
  })
})

@jantimon
Copy link

jantimon commented Apr 3, 2018

chunkSortMode none will be the default in the next html-webpack-plugin major release. But I would like to combine it with other breaking changes so it will take some time to publish it

@andrerpena
Copy link

Thank you! Storybook is now working with Webpack 4 and custom Webpack configs but, just so you know, I had to add babel-core as a dev-dependency to my project in order for it to work.

@igor-dv
Copy link
Member

igor-dv commented May 1, 2018

babel-core is a peer dep, so you should add it

@rexonms
Copy link

rexonms commented Jun 10, 2018

The following settings worked for me.

   "@storybook/addon-actions": "^3.3.0",
   "@storybook/addon-info": "^3.3.0",
   "@storybook/addon-knobs": "^3.3.0",
   "@storybook/cli": "^4.0.0-alpha.0",
   "@storybook/react": "^4.0.0-alpha.3",
   "webpack": "^4.1.0",

@Hypnosphi
Copy link
Member Author

We strongly recommend to use the same versions for @storybook/<framework> and addons

@VRedondo-zz
Copy link

VRedondo-zz commented Aug 30, 2018

Following settings doesn't work for me

"dependencies": {
  "@angular/core": "^6.1.2",
....
"devDependencies": {
  "webpack": "^4.12.0",
  "@storybook/angular": "^4.0.0-alpha.18",
  "babel-core": "^6.26.3",
  "babel-loader": "^8.0.0",

when running storybook it returns the error


ERROR in ./.storybook/config.js
Module build failed (from ./node_modules/babel-loader/lib/index.js):
Error: Cannot find module '@babel/core'

any suggestion?

@SleepWalker
Copy link

@VRedondo you need to use babel@7 to run the latest alpha, or use the version with babel 6. I've used alpha14, but the most recent is alpha16

@Hypnosphi
Copy link
Member Author

you need to use babel@7 to run the latest alpha

That's not true. You should be able to use babel 6 with latest alpha, you just need babel-loader 7 for it

@VRedondo-zz
Copy link

Thanks @SleepWalker and @Hypnosphi actually both were needed, babel-loader 7 requires babel 7 so it got fixed with
"@babel/core": "^7.0.0",
"babel-loader": "^8.0.0",

@Hypnosphi
Copy link
Member Author

babel-loader 7 requires babel 7

What made you think so?

@VRedondo-zz
Copy link

@Hypnosphi the console output says

Module build failed (from ./node_modules/babel-loader/lib/index.js):
Error: Requires Babel "^7.0.0-0", but was loaded with "6.26.3".

when running with babel-loader 7 in case you don't have babel 7

@Hypnosphi
Copy link
Member Author

Hypnosphi commented Aug 31, 2018

Yeah, you also need your own .babelrc in this case, our default config is for Babel 7. Looks like you're OK with the latter though

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.