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

v2 includes runtimeChunk twice with optimization.runtimeChunk option in webpack v4 #61

Closed
akash5474 opened this issue Jun 19, 2018 · 9 comments
Assignees

Comments

@akash5474
Copy link

akash5474 commented Jun 19, 2018

I noticed that my JS was running twice after upgrading to webpack-flush-chunks@2.0.0 when using optimization.runtimeChunk option in webpack v4.

My runtimeChunk bootstrap.js gets included twice in the html

<script type='text/javascript' src='/static/bootstrap.js' defer></script>
<script type='text/javascript' src='/static/Foo.js' defer></script>
<script type='text/javascript' src='/static/bootstrap.js' defer></script>
<script type='text/javascript' src='/static/main.js' defer></script>

I've confirmed that this no longer happens if I remove optimization.runtimeChunk from the webpack config or switch back to webpack-flush-chunks@1.2.3.

Example Repo

https://github.com/akash5474/universal-demo/tree/bug-demo

Changes Made

Forked faceyspacey/universal-demo and made the following changes (akash5474/universal-demo@764c1be and akash5474/universal-demo@fd65e57):

  1. Added a console.log("hello world") statement to src/index.js to demonstrate the issue.

  2. Updated to the following dependency versions:

"webpack-flush-chunks": "2.0.0",
"extract-css-chunks-webpack-plugin": "3.0.5",
"babel-plugin-universal-import": "3.0.0",
"webpack": "4.12.0",
  1. Updated webpack configs to work with new dependency versions

  2. Deleted some unnecessary code

Steps to reproduce

git clone -b bug-demo https://github.com/akash5474/universal-demo

cd universal-demo

npm install

npm run start:prod

Open http://localhost:3000 in the browser and open the console.

You will see "hello world" is logged twice.

Viewing the source html you will see bootstrap.js is included twice

This can be resolved by removing the following from webpack client config:

  optimization: {
    runtimeChunk: {
      name: "bootstrap"
    },
  }

However after downgrading to webpack-flush-chunks@1.2.3, bootstrap.js is only included once with the optimization.runtimeChunk option included. This suggests that the problem was introduced in webpack-flush-chunks@2.0.0

Let me know if you have any questions or if I can help in any way. Thanks.

@akash5474 akash5474 changed the title v2 causing JS to run twice with optimization.runtimeChunk in webpack v4 v2 causing JS to run twice with optimization.runtimeChunk option in webpack v4 Jun 19, 2018
@akash5474 akash5474 changed the title v2 causing JS to run twice with optimization.runtimeChunk option in webpack v4 v2 causing JS to run twice with optimization.runtimeChunk option in webpack v4 Jun 19, 2018
@akash5474 akash5474 changed the title v2 causing JS to run twice with optimization.runtimeChunk option in webpack v4 v2 includes runtimeChunk twice with optimization.runtimeChunk option in webpack v4 Jun 19, 2018
@alexkrautmann
Copy link

alexkrautmann commented Jun 20, 2018

I can verify this error. My optimization config is:

optimization: {
        runtimeChunk: {
            name: 'bootstrap'
        },
        splitChunks: {
            chunks: 'initial',
            cacheGroups: {
                vendors: {
                    test: /[\\/]node_modules[\\/]/,
                    name: 'vendor'
                }
            }
        },
}

And both bootstrap and vendor show up twice from the return of flushChunks.

@slavab89
Copy link

I have the same issue and use the same setup.

This is caused because the after assets include the before ones because they are included as part of the entry (which is logical).
I'm not sure what's the impact of just filtering the before assets from the after ones though.

To still be able to work with webpack 4 i'm using my own script to extract the list of files (until a fix will be up)

import { flatten, includes } from 'lodash';

function filesFromChunks(chunkNames, assets, checkChunkNames) {
  const hasChunk = entry => {
    const result = !!assets[entry];
    if (!result && checkChunkNames) {
      console.warn(`Unable to find ${entry} in Webpack chunks.`);
    }

    return result;
  };

  const entryToFiles = entry => assets[entry];

  return [].concat(...chunkNames.filter(hasChunk).map(entryToFiles));
}

export default function chunksMap(clientStats) {
  const publicPath = clientStats.publicPath.replace(/\/$/, '');
  const beforeAssets = filesFromChunks(['bootstrap', 'vendor'], clientStats.assetsByChunkName);
  const afterAssets = filesFromChunks(['main'], clientStats.assetsByChunkName);

  return function flushChunks(chunkNames) {
    const mainAssets = flatten(chunkNames.map(chunkName => clientStats.namedChunkGroups[chunkName].assets));
    const allNeededAssets = [...beforeAssets, ...mainAssets, ...afterAssets];

    const jsAssets = allNeededAssets
      .filter(asset => includes(asset, '.js'))
      .map(file => `<script type='${includes(file, '.map') ? 'application/json' : 'text/javascript'}' src='${publicPath}/${file}' defer></script>`)
      .join('\n');

    return {
      js: jsAssets,
    };
  };
}

That's only for JS files.

@ScriptedAlchemy
Copy link
Collaborator

Thanks for finding this! I’ll patch the duplicate

@ScriptedAlchemy ScriptedAlchemy self-assigned this Jun 22, 2018
ScriptedAlchemy added a commit that referenced this issue Jun 22, 2018
When using one of the older webpack config setups. Bootstrap is duplicated and executed twice

#61
@ScriptedAlchemy
Copy link
Collaborator

@slavab89 @akash5474 Ill push a next tag on npm as well

@ScriptedAlchemy
Copy link
Collaborator

#62

@ScriptedAlchemy
Copy link
Collaborator

on npm, let me know if thats better

@alexkrautmann
Copy link

@ScriptedAlchemy I just tried out next and it definitely works for me.

@ScriptedAlchemy
Copy link
Collaborator

Fantastic. Apologies for the error! Thank you @akash5474 for reporting it!

Gonna merge this baby

@akash5474
Copy link
Author

@ScriptedAlchemy Np and thanks for getting this fixed, working great now. Appreciate all the work you're doing with these libs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants