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

not working with CommonChunksPlugin with async chunk #23

Open
tswaters opened this issue May 1, 2017 · 4 comments
Open

not working with CommonChunksPlugin with async chunk #23

tswaters opened this issue May 1, 2017 · 4 comments
Labels

Comments

@tswaters
Copy link
Contributor

tswaters commented May 1, 2017

I'm getting an endless loop when using the async option on CommonChunksPlugin.

This config will create a chunk file for common pieces spread throughout split code. If you use the same common components on multiple split pages, it will place the goods in this chunk file.

It seems that the module loading never completes and it goes into an endless loop with modulesLoading always set to 1. This can be seen by making the following changes to react-router-server-complex-example:

diff --git a/src/components/About.jsx b/src/components/About.jsx
index d7bae53..a1a902b 100644
--- a/src/components/About.jsx
+++ b/src/components/About.jsx
@@ -1,4 +1,7 @@
 import * as React from 'react';
+import hello from './Common';
+
+hello()

 const Component = (props) => (
   <div>
diff --git a/src/components/Common.js b/src/components/Common.js
new file mode 100644
index 0000000..f08ec80
--- /dev/null
+++ b/src/components/Common.js
@@ -0,0 +1,4 @@
+
+export default () => {
+  console.log('hello world!')
+}
diff --git a/src/components/Home.jsx b/src/components/Home.jsx
index 0ad06d4..a3fe950 100644
--- a/src/components/Home.jsx
+++ b/src/components/Home.jsx
@@ -1,6 +1,9 @@
 import React, { Component, PropTypes } from 'react';
 import { fetchState } from 'react-router-server';
 import '../styles/home.css';
+import hello from './Common';
+
+hello()

 @fetchState(
   state => ({
diff --git a/src/components/Images.jsx b/src/components/Images.jsx
index a97e338..9ca0563 100644
--- a/src/components/Images.jsx
+++ b/src/components/Images.jsx
@@ -1,4 +1,7 @@
 import * as React from 'react';
+import hello from './Common';
+
+hello()

 const Images = (props) => (
   <div>
diff --git a/webpack.config.js b/webpack.config.js
index d367e06..a9bef18 100644
--- a/webpack.config.js
+++ b/webpack.config.js
@@ -2,6 +2,7 @@ import path from 'path';
 import StatsPlugin from 'stats-webpack-plugin';
 import fs from 'fs';
 import ExtractTextPlugin from 'extract-text-webpack-plugin';
+import webpack from 'webpack'

 const nodeModules = {};
 fs.readdirSync(path.join(__dirname, 'node_modules'))
@@ -47,6 +48,10 @@ const config = server => ({
   },

   plugins: [
+    new webpack.optimize.CommonsChunkPlugin({
+      async: 'common',
+      minChunks: 2
+    }),
     new StatsPlugin('stats.json', {
       chunkModules: true,
       exclude: [/node_modules/]

Visiting any page that references the common chunk will spin indefinitely.

@gabrielbull gabrielbull added the bug label May 2, 2017
@Everettss
Copy link

Everettss commented May 2, 2017

I found some kind of working workaround (app will not crash but still contain bug).
If you add new webpack.optimize.LimitChunkCountPlugin({ maxChunks: 1 }) only on server build:

index 0724306..e5f37e9 100644
--- a/webpack.config.js
+++ b/webpack.config.js
@@ -12,6 +12,19 @@ fs.readdirSync(path.join(__dirname, 'node_modules'))
 const extractTextPlugin = new ExtractTextPlugin('[name].css');
 extractTextPlugin.options.allChunks = true;

+
+const plugins = [
+  new webpack.optimize.CommonsChunkPlugin({
+    async: 'common',
+    minChunks: 2
+  }),
+  new StatsPlugin('stats.json', {
+    chunkModules: true,
+    exclude: [/node_modules/]
+  }),
+  extractTextPlugin
+];
+
 const config = server => ({
   entry: {
     app: path.join(__dirname, 'src', (server ? 'app.js' : 'client.js'))
@@ -47,17 +60,12 @@ const config = server => ({
     ]
   },

-  plugins: [
-    new webpack.optimize.CommonsChunkPlugin({
-        async: 'common',
-        minChunks: 2
-    }),
-    new StatsPlugin('stats.json', {
-      chunkModules: true,
-      exclude: [/node_modules/]
-    }),
-    extractTextPlugin
-  ]
+  plugins: server
+    ? [
+      new webpack.optimize.LimitChunkCountPlugin({ maxChunks: 1 }),
+      ...plugins
+    ]
+    : plugins,
 });

Site works, but in __INITIAL_MODULES__ you will find only one chunk with common code.
With this modification Home route network looks like this:
screen shot 2017-05-02 at 22 11 15

where:

  • 0.378dfe4c193138520721.js contains console.log('hello world!');
  • 2.378dfe4c193138520721.js contains Home component

What behaviour should be proper:

  • order of download this chunks should be opposite?
  • or all should be in __INITIAL_MODULES__?

@tswaters
Copy link
Contributor Author

tswaters commented May 6, 2017

I've done a bit more digging on this. When the common chunks plugin is in play, the single promise resolution for the System.import turns into a Promise.all, as it loads all required chunks.

e.g., in the case of the Home route, it turns from this:

return __webpack_require__.e/* import() */(1).then(__webpack_require__.bind(null, 14));

to this:

return Promise.all/* import() */([__webpack_require__.e(2), __webpack_require__.e(0)]).then(__webpack_require__.bind(null, 14));

So none of the regular expressions in getWebpackId will match.

The goal of the regex matching is to find the module it is loading - in the case of the first one it is 1 and in the second case , it's an array both 2 and 0. That information is used to key the values ./module/cache.js and, in the current state of loading a single chunk, once the module is available it is passed along to the child of <Module/> for rendering.

I would think that the regex would need to handle an array of entries and somehow identify which one ties to the actual component and which one ties to the common chunk. Both chunks would need to make their way to modules in renderToString's then block and, I imagine, the preload on client would continue to work.

How it identifies which is a common chunk and which one contains the component is the real trick, I think. It could be that the first one will always be the component, but I'm unsure if this is the case. The webpack stats I think can be used to figure this out definitively, but renderToString doesn't have access to webpack stats under the current API.

Also, I'm using a pretty basic example of async for CommonChunks. I would imagine with more complicated configurations, you could wind up with multiple common async chunks in play for a single import.

@gabrielbull
Copy link
Owner

So, what I propose as a solution is to get rid of the regex matching altogether and implement a webpack plugin to fulfil the need to match the modules from the server side with the client side.

All that would be needed to get this to work would be to add a plugin to the webpack config:

import { ModulesPlugin } from 'react-router-server';

module.exports = {
   plugins: [
      new webpack.optimize.CommonsChunkPlugin({
        async: 'common',
        minChunks: 2
      }),
      new ModulesPlugin()
    ]
};

@nilaybrahmbhatt
Copy link

ModulesPlugin is undefined in webpack;
_reactRouterServer.ModulesPlugin is not a constructor

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

No branches or pull requests

4 participants