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

Worker Loader + Module Federation breaks Webpack #10033

Open
mlucool opened this issue Apr 1, 2021 · 4 comments
Open

Worker Loader + Module Federation breaks Webpack #10033

mlucool opened this issue Apr 1, 2021 · 4 comments

Comments

@mlucool
Copy link
Contributor

mlucool commented Apr 1, 2021

Description

If any of your code includes something that uses worker-loader, webpack fails to compile (it will eventually crash with OOM no matter how much memory you give it).

Reproduce

In an extension (e.g. extension-cookiecutter-ts):

Add pdfjs-dist as a dependency:

yarn add pdfjs-dist

Add the following line anywhere that will be compiled (e.g. index.ts):

// @ts-ignore
import('pdfjs-dist/webpack');

Now build:

yarn build

Expected behavior

This should compile, but after a while, crashes with OOM.

I debugged this quite deep and the following I found very helpful to pinpoint that it was related to worker-loader.

  1. Try to understand where it get's stuck. This can be accomplished by adding the following plugin to webpackConfig:
const {ProgressPlugin} = require('webpack');

module.exports = {
    plugins: [
        new ProgressPlugin({activeModules: true})
    ]
};

Then, go and edit the source of ProgressPlugin and add something like this (you can write you own plugin to do this, but I found this cheaper):

		const activeModules = new Set();
		let lastUpdate = 0;

        // Added code
        let numActive = 0;
        setInterval(() => {
            if(numActive === activeModules.size) {
                console.log(Array.from(activeModules).join('\n'))
            }
            numActive = activeModules.size;
        }, 5000).unref();

This helped me narrow it down to what it was stuck trying to do.

  1. Binary searching webpack plugins that are used from Jupyter. It turns out the only plugin that is causing this is the ModuleFederationPlugin. Comment this out to see webpack finishes (which makes the extension not work, but shows webpack is ok with import('pdfjs-dist/webpack');)
  2. Use the node debugger to see that this isn't stuck, but likely in some sort of infinite loop, which is why it goes OOM.
const inspector = require('inspector');
inspector.open(9999, process.env.HOST, true);

Context

  • Operating System and version: Linux
  • JupyterLab version: 3.0.12
@jasongrout
Copy link
Contributor

jasongrout commented Apr 1, 2021

Wow, thanks very much for debugging this deeply.

It sounds like it might be an upstream issue with webpack. Can you try to reproduce this with one of the webpack module federation example repos, in preparation for a bug report up to webpack? IIRC, the example at https://github.com/module-federation/module-federation-examples/tree/master/advanced-api/dynamic-remotes exercises some of the same parts of the module federation system that we exercise (compilation instructions for that repo are at https://github.com/module-federation/module-federation-examples#notes)

@telamonian
Copy link
Member

@mlucool interesting... dumb question: what happens if you explicitly install worker-loader first?

inspiration:

If you are getting the Setting up fake worker warning, make sure you are importing pdfjs-dist/webpack which is the zero-configuration method for Webpack users. You will need to install worker-loader (version 3.0.0 or higher is required), as a dependency in your project in order to use pdfjs-dist/webpack (configuring worker-loader is not necessary; just installing it is sufficient).

@mlucool
Copy link
Contributor Author

mlucool commented Apr 1, 2021

@mlucool interesting... dumb question: what happens if you explicitly install worker-loader first?

Same thing. This is not a pdfjs issue as I saw this with other modules that use worker-loader. I choose this as an example because it's very popular.

As a note, builder already depends on worker-loader, so that requirement was already met.

@mlucool
Copy link
Contributor Author

mlucool commented Apr 6, 2021

Hi @jasongrout,

This does not seem to be a clear module federation bug (although it could be module federation + something else we are doing here):

diff --git a/advanced-api/dynamic-remotes/app2/package.json b/advanced-api/dynamic-remotes/app2/package.json
index 1ac21f1..d1e04d9 100644
--- a/advanced-api/dynamic-remotes/app2/package.json
+++ b/advanced-api/dynamic-remotes/app2/package.json
@@ -10,7 +10,8 @@
     "serve": "11.3.2",
     "webpack": "5.18.0",
     "webpack-cli": "4.3.1",
-    "webpack-dev-server": "3.11.2"
+    "webpack-dev-server": "3.11.2",
+    "worker-loader": "^3.0.8"
   },
   "scripts": {
     "start": "webpack-cli serve",
@@ -21,6 +22,7 @@
   "dependencies": {
     "react": "^16.13.0",
     "react-dom": "^16.13.0",
-    "moment": "^2.24.0"
+    "moment": "^2.24.0",
+    "pdfjs-dist": "^2.7.570"
   }
 }
diff --git a/advanced-api/dynamic-remotes/app2/src/App.js b/advanced-api/dynamic-remotes/app2/src/App.js
index 82f4ba0..b8c3789 100644
--- a/advanced-api/dynamic-remotes/app2/src/App.js
+++ b/advanced-api/dynamic-remotes/app2/src/App.js
@@ -1,5 +1,7 @@
 import LocalButton from "./Widget";
 import React from "react";
+import('pdfjs-dist/webpack');
+
 const App = () => (
   <div>
     <h1>Dynamic System Host</h1>

And it builds:

module-federation-examples/advanced-api/dynamic-remotes/app2 (master) $ yarn build
yarn run v1.22.10
$ webpack --mode production
(node:1178) [DEP_WEBPACK_COMPILATION_ASSETS] DeprecationWarning: Compilation.assets will be frozen in future, all modifications are deprecated.
BREAKING CHANGE: No more changes should happen to Compilation.assets after sealing the Compilation.
        Do changes to assets earlier, e. g. in Compilation.hooks.processAssets.
        Make sure to select an appropriate stage from Compilation.PROCESS_ASSETS_STAGE_*.
assets by chunk 617 KiB (id hint: vendors)
  asset 762.js 285 KiB [compared for emit] [minimized] [big] (id hint: vendors) 1 related asset
  asset 372.js 212 KiB [compared for emit] [minimized] (id hint: vendors) 1 related asset
  asset 935.js 120 KiB [compared for emit] [minimized] (id hint: vendors) 1 related asset
asset pdf.worker.js 659 KiB [compared for emit] [minimized] [big] 1 related asset
asset 294.js 7.16 KiB [compared for emit] [minimized] 1 related asset
asset main.js 6.49 KiB [compared for emit] [minimized] (name: main)
asset remoteEntry.js 6.41 KiB [compared for emit] [minimized] (name: app2)
asset 700.js 4.16 KiB [compared for emit] [minimized]
asset 957.js 812 bytes [compared for emit] [minimized]
asset 630.js 533 bytes [compared for emit] [minimized]
asset 61.js 170 bytes [compared for emit] [minimized]
asset index.html 116 bytes [compared for emit]
runtime modules 36.5 KiB 27 modules
orphan modules 356 bytes [orphan] 1 module
modules by path ./node_modules/moment/locale/*.js 499 KiB 135 modules
modules by path ./src/ 1.59 KiB 3 modules
provide-module modules 126 bytes 3 modules
consume-shared-module modules 126 bytes 3 modules
modules by path ./node_modules/pdfjs-dist/ 402 KiB 3 modules
modules by path ./node_modules/react-dom/ 117 KiB 2 modules
modules by path ./node_modules/react/ 6.7 KiB 2 modules
optional modules 3.22 KiB [optional] 2 modules
modules by path ./node_modules/scheduler/ 5.12 KiB
  ./node_modules/scheduler/index.js 198 bytes [built] [code generated]
  ./node_modules/scheduler/cjs/scheduler.production.min.js 4.92 KiB [built] [code generated]
8 modules

WARNING in asset size limit: The following asset(s) exceed the recommended size limit (244 KiB).
This can impact web performance.
Assets: 
  pdf.worker.js (659 KiB)
  762.js (285 KiB)

webpack 5.18.0 compiled with 1 warning in 16124 ms
Done in 17.22s.

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

No branches or pull requests

4 participants