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

splitChunks can create initial chunks that are empty after CSS extraction #7300

Open
edmorley opened this Issue May 14, 2018 · 22 comments

Comments

Projects
None yet
@edmorley
Contributor

edmorley commented May 14, 2018

Bug report

What is the current behavior?

The splitChunks feature (using chunks: 'all') doesn't take into account CSS extraction (via mini-css-extract-plugin) when deciding whether to create a new inital chunk. A new chunk can end up being created that is empty (other than the webpack functions) and under the default 30KB splitChunks.minSize threshold so shouldn't have been created.

eg:
dist/vendors~pageA~pageB.js

For webpack build log output see:
https://github.com/edmorley/testcase-webpack-splitchunks-css#actual

If the current behavior is a bug, please provide the steps to reproduce.

  1. git clone https://github.com/edmorley/testcase-webpack-splitchunks-css.git
  2. yarn install
  3. yarn build

What is the expected behavior?

There is no dist/vendors~pageA~pageB.js chunk generated, since after the CSS is extracted, there should be no common JS code between the two pages.

Other relevant information:
webpack version: 4.8.3
Node.js version: 10.1.0
Operating System: Windows 10
Additional tools: mini-css-extract-plugin@0.4.0

@evilebottnawi

This comment has been minimized.

Show comment
Hide comment
Member

evilebottnawi commented May 14, 2018

@edmorley

This comment has been minimized.

Show comment
Hide comment
@edmorley

edmorley May 14, 2018

Contributor

Hi! I saw that issue before filing this, however in that issue a custom cacheGroups definition was being used to move all CSS to one chunk - whereas this case is with the default cacheGroups setting. The symptoms do seem similar, so it's not implausible that it's the same root cause - but I didn't want to assume and pile on there.

Contributor

edmorley commented May 14, 2018

Hi! I saw that issue before filing this, however in that issue a custom cacheGroups definition was being used to move all CSS to one chunk - whereas this case is with the default cacheGroups setting. The symptoms do seem similar, so it's not implausible that it's the same root cause - but I didn't want to assume and pile on there.

@sokra sokra added this to the webpack 4.x milestone May 15, 2018

@sokra

This comment has been minimized.

Show comment
Hide comment
@sokra

sokra May 15, 2018

Member

Yep that's true

Member

sokra commented May 15, 2018

Yep that's true

@edmorley

This comment has been minimized.

Show comment
Hide comment
@edmorley

edmorley Jun 8, 2018

Contributor

Another issue that this causes, is that the empty (plus also "not quite empty, but still less than the 30KB minimum size") chunks use up the maxInitialRequests allowance - forcing some of the larger modules into the entrypoint chunks where they are duplicated, meaning that the total built output size is greater.

This has the knock on effect of slower builds , since there is more to minify/source-map. (In one of my projects raising maxInitialRequests higher than the default of 3 reduces cold cache build times by 40% and warm by 30-35%.)

Contributor

edmorley commented Jun 8, 2018

Another issue that this causes, is that the empty (plus also "not quite empty, but still less than the 30KB minimum size") chunks use up the maxInitialRequests allowance - forcing some of the larger modules into the entrypoint chunks where they are duplicated, meaning that the total built output size is greater.

This has the knock on effect of slower builds , since there is more to minify/source-map. (In one of my projects raising maxInitialRequests higher than the default of 3 reduces cold cache build times by 40% and warm by 30-35%.)

@jpdesigndev

This comment has been minimized.

Show comment
Hide comment
@jpdesigndev

jpdesigndev Jun 21, 2018

@sokra Is there any workaround for this issue you can think of? I'm simply trying to split out all vendor css, but this is creating an "empty" (file contents: (window.webpackJsonp=window.webpackJsonp||[]).push([[0],[]]);) js file. Any suggestions?

jpdesigndev commented Jun 21, 2018

@sokra Is there any workaround for this issue you can think of? I'm simply trying to split out all vendor css, but this is creating an "empty" (file contents: (window.webpackJsonp=window.webpackJsonp||[]).push([[0],[]]);) js file. Any suggestions?

@azamat-sharapov

This comment has been minimized.

Show comment
Hide comment
@azamat-sharapov

azamat-sharapov Jun 22, 2018

@jpdesigndev try prepending those files' contents into you main entry manually.

azamat-sharapov commented Jun 22, 2018

@jpdesigndev try prepending those files' contents into you main entry manually.

@louisscruz

This comment has been minimized.

Show comment
Hide comment
@louisscruz

louisscruz Jun 22, 2018

Any news on this? I think this is the final blocker for our upgrade to Webpack 4.

louisscruz commented Jun 22, 2018

Any news on this? I think this is the final blocker for our upgrade to Webpack 4.

@kasperpeulen

This comment has been minimized.

Show comment
Hide comment
@kasperpeulen

kasperpeulen Jun 22, 2018

This is blocking me to upgrade to webpack 4, but it is planned for webpack 5. I guess I will stop trying to upgrade until 5 comes out.

kasperpeulen commented Jun 22, 2018

This is blocking me to upgrade to webpack 4, but it is planned for webpack 5. I guess I will stop trying to upgrade until 5 comes out.

@edmorley

This comment has been minimized.

Show comment
Hide comment
@edmorley

edmorley Jun 22, 2018

Contributor

The milestone for this used to be webpack 4.x, but has recently been changed to webpack 5. Was this due to the fix for this being seen as a breaking change, or because of timeframe/resources required to fix?

Contributor

edmorley commented Jun 22, 2018

The milestone for this used to be webpack 4.x, but has recently been changed to webpack 5. Was this due to the fix for this being seen as a breaking change, or because of timeframe/resources required to fix?

@evilebottnawi

This comment has been minimized.

Show comment
Hide comment
@evilebottnawi

evilebottnawi Jun 22, 2018

Member

Yes, it is breaking change

Member

evilebottnawi commented Jun 22, 2018

Yes, it is breaking change

@sokra sokra modified the milestones: webpack 5, webpack 4.x Jun 22, 2018

@sokra

This comment has been minimized.

Show comment
Hide comment
@sokra

sokra Jun 22, 2018

Member

hmm... maybe there is a way we could add it for webpack 4 too

Member

sokra commented Jun 22, 2018

hmm... maybe there is a way we could add it for webpack 4 too

@ev1stensberg

This comment has been minimized.

Show comment
Hide comment
@ev1stensberg

ev1stensberg Jun 26, 2018

Member

If this needs help, I'm happy to help out

Member

ev1stensberg commented Jun 26, 2018

If this needs help, I'm happy to help out

@Igloczek

This comment has been minimized.

Show comment
Hide comment
@Igloczek

Igloczek commented Jul 4, 2018

@evilebottnawi

This comment has been minimized.

Show comment
Hide comment
@evilebottnawi

evilebottnawi Jul 4, 2018

Member

/cc @sokra will be great solve this for webpack@4, because a lot of people have this problem

Member

evilebottnawi commented Jul 4, 2018

/cc @sokra will be great solve this for webpack@4, because a lot of people have this problem

@danechitoaie

This comment has been minimized.

Show comment
Hide comment
@danechitoaie

danechitoaie Jul 4, 2018

Right now I'm using this "workaround":

class MiniCssExtractPluginCleanup {
    apply(compiler) {
        compiler.hooks.emit.tapAsync("MiniCssExtractPluginCleanup", (compilation, callback) => {
            Object.keys(compilation.assets)
                .filter(asset => {
                    return ["*/scss/**/*.js", "*/scss/**/*.js.map"].some(pattern => {
                        return minimatch(asset, pattern);
                    });
                })
                .forEach(asset => {
                    delete compilation.assets[asset];
                });

            callback();
        });
    }
}

It's very specific for my use case and has things hardcoded and I even have just put it directly in the webpack.config.js file (so not published on npm).

So it would be awesome if this could be fixed at webpack level.

danechitoaie commented Jul 4, 2018

Right now I'm using this "workaround":

class MiniCssExtractPluginCleanup {
    apply(compiler) {
        compiler.hooks.emit.tapAsync("MiniCssExtractPluginCleanup", (compilation, callback) => {
            Object.keys(compilation.assets)
                .filter(asset => {
                    return ["*/scss/**/*.js", "*/scss/**/*.js.map"].some(pattern => {
                        return minimatch(asset, pattern);
                    });
                })
                .forEach(asset => {
                    delete compilation.assets[asset];
                });

            callback();
        });
    }
}

It's very specific for my use case and has things hardcoded and I even have just put it directly in the webpack.config.js file (so not published on npm).

So it would be awesome if this could be fixed at webpack level.

@maapteh

This comment has been minimized.

Show comment
Hide comment
@maapteh

maapteh Aug 16, 2018

@sokra will this still be fixed in 4? We are migrating to 4 currently and everything works except this "empty" file.

Or wait for 5 with css-webpack-plugin ? Awesome work by the way, i only had problems with the optimise part!

maapteh commented Aug 16, 2018

@sokra will this still be fixed in 4? We are migrating to 4 currently and everything works except this "empty" file.

Or wait for 5 with css-webpack-plugin ? Awesome work by the way, i only had problems with the optimise part!

@toastal

This comment has been minimized.

Show comment
Hide comment
@toastal

toastal Aug 17, 2018

@danechitoaie 's "workaround" worked for me, but it does unnecessary looping, incurs a dependency in minimatch, and isn't very flexible as mentioned. I modified to this which passes in a RegExp to be reused for what to delete, as well as not doing extra loops via higher-order functions:

class MiniCssExtractPluginCleanup {
  constructor(deleteWhere = /\.js(\.map)?$/) {
    this.shouldDelete = new RegExp(deleteWhere)
  }
  apply(compiler) {
    compiler.hooks.emit.tapAsync("MiniCssExtractPluginCleanup", (compilation, callback) => {
      Object.keys(compilation.assets).forEach((asset) => {
        if (this.shouldDelete.test(asset)) {
          delete compilation.assets[asset]
        }
      })
      callback()
    })
  }
}

Which then in their case could be used by passing in the RegExp

module.exports = {
   ...,
   plugins: [
     new MiniCssExtractPluginCleanUp(),
   ],
  ...,
}

toastal commented Aug 17, 2018

@danechitoaie 's "workaround" worked for me, but it does unnecessary looping, incurs a dependency in minimatch, and isn't very flexible as mentioned. I modified to this which passes in a RegExp to be reused for what to delete, as well as not doing extra loops via higher-order functions:

class MiniCssExtractPluginCleanup {
  constructor(deleteWhere = /\.js(\.map)?$/) {
    this.shouldDelete = new RegExp(deleteWhere)
  }
  apply(compiler) {
    compiler.hooks.emit.tapAsync("MiniCssExtractPluginCleanup", (compilation, callback) => {
      Object.keys(compilation.assets).forEach((asset) => {
        if (this.shouldDelete.test(asset)) {
          delete compilation.assets[asset]
        }
      })
      callback()
    })
  }
}

Which then in their case could be used by passing in the RegExp

module.exports = {
   ...,
   plugins: [
     new MiniCssExtractPluginCleanUp(),
   ],
  ...,
}
@Kajeczko

This comment has been minimized.

Show comment
Hide comment
@Kajeczko

Kajeczko Aug 20, 2018

Also remember you can just run console command to delete / modify this files after webpack done his job.

// package.json
"scripts": {
    "clean-css": "rm ./build/normalize.bundle.js",
    "build": "webpack --config webpack.dev.js && npm run clean-css",
}

Can be useful: Pre and Post hooks for npm scripting

Kajeczko commented Aug 20, 2018

Also remember you can just run console command to delete / modify this files after webpack done his job.

// package.json
"scripts": {
    "clean-css": "rm ./build/normalize.bundle.js",
    "build": "webpack --config webpack.dev.js && npm run clean-css",
}

Can be useful: Pre and Post hooks for npm scripting

@usebaz

This comment has been minimized.

Show comment
Hide comment
@usebaz

usebaz Aug 27, 2018

@Kajeczko in some cases this file can have code that run webpack. Example:

⇒  cat build/_js/route.styles.js 
(window["webpackJsonp"] = window["webpackJsonp"] || []).push([["styles"],[]]);

usebaz commented Aug 27, 2018

@Kajeczko in some cases this file can have code that run webpack. Example:

⇒  cat build/_js/route.styles.js 
(window["webpackJsonp"] = window["webpackJsonp"] || []).push([["styles"],[]]);
@Dwood15

This comment has been minimized.

Show comment
Hide comment
@Dwood15

Dwood15 Aug 27, 2018

@usebaz that kind of code isn't necessary to use the css files, and are safe to delete.

Dwood15 commented Aug 27, 2018

@usebaz that kind of code isn't necessary to use the css files, and are safe to delete.

@usebaz

This comment has been minimized.

Show comment
Hide comment
@usebaz

usebaz Aug 27, 2018

@Dwood15 without this code, webpack modules are not started on the client

UPD. I insert wrong code in file a have: (window.webpackJsonp=window.webpackJsonp||[]).push([[1],[]]);

usebaz commented Aug 27, 2018

@Dwood15 without this code, webpack modules are not started on the client

UPD. I insert wrong code in file a have: (window.webpackJsonp=window.webpackJsonp||[]).push([[1],[]]);

@timurgarif

This comment has been minimized.

Show comment
Hide comment
@timurgarif

timurgarif Aug 27, 2018

@Dwood15 @usebaz
I think the code in styles.js exports all the names generated by css-modules.

import styles from 'some.css'
...
<div className={styles.myClass} />

If you don't use auto generated css class names (e.g using css-modules), then probably you don't need this file.

timurgarif commented Aug 27, 2018

@Dwood15 @usebaz
I think the code in styles.js exports all the names generated by css-modules.

import styles from 'some.css'
...
<div className={styles.myClass} />

If you don't use auto generated css class names (e.g using css-modules), then probably you don't need this file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment