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

Upgrade webpack and associated packages #4702

Merged
merged 17 commits into from Jun 12, 2018

Conversation

@jasongrout
Copy link
Contributor

@jasongrout jasongrout commented Jun 9, 2018

Upgrade webpack to the latest release.


Edit: here is the high-level effect, copied from below (#4702 (comment)):

pip install -ve .
jlpm run build
cd dev_mode
yarn run build:prod
ls -lash static/*.js

gives this with current master:

456K static/0.d22431518ad1366208f5.js
 34K static/index.out.js
2.2M static/main.da3aadf58699cc652bee.js
1.6K static/manifest.351c00360b8d27f69af0.js
1.6M static/vendor.26f362dc3ec62e65e88d.js

and this with this PR

442K static/0.694c1bea111cef5ab380.js
3.6K static/1.f07d1dd71ae3c4532523.js
 34K static/index.out.js
755K static/main.422d8f1e07c68d925ed0.js
523K static/vega~main.6c6420135a192f937426.js
2.3M static/vendors~main.348f8e42a1dc9c2b4233.js
@jasongrout jasongrout added this to the Beta 3 milestone Jun 9, 2018
@jasongrout jasongrout changed the title WIP Upgrade webpace and associated packages WIP Upgrade webpack and associated packages Jun 9, 2018
@bollwyvl
Copy link
Contributor

@bollwyvl bollwyvl commented Jun 9, 2018

Having a look at this now... very exciting!

During --watch, main.js seems bigger (11MB), and incremental build seems to take a bit longer, but that could be an artifact of devmode/sourcemaps/other things that happened since i last had a spin on master.

With jlpm build:dev:prod, main.js is clocking in at 3.5mb, which also seems bigger than 0.32. I'll have a gander at what might be driving this.

The one warning i started seeing:

DeprecationWarning: Tapable.plugin is deprecated. Use new API on `.hooks` instead

After figuring out that it's from our little embedded JupyterLabPlugin, I took a whack at fixing it:

diff --git a/jupyterlab/staging/webpack.config.js b/jupyterlab/staging/webpack.config.js
index a653b053c..f06a404b8 100644
--- a/jupyterlab/staging/webpack.config.js
+++ b/jupyterlab/staging/webpack.config.js
@@ -82,10 +82,9 @@ function JupyterLabPlugin() { }
 
 JupyterLabPlugin.prototype.apply = function(compiler) {
 
-  compiler.plugin('after-emit', function(compilation, callback) {
+  compiler.hooks.afterEmit.tap('JupyterLabPlugin', function() {
     var staticDir = jlab.staticDir;
     if (!staticDir) {
-      callback();
       return;
     }
     // Ensure a clean static directory on the first emit.
@@ -94,7 +93,6 @@ JupyterLabPlugin.prototype.apply = function(compiler) {
     }
     this._first = false;
     fs.copySync(buildDir, staticDir);
-    callback();
   }.bind(this));
 };

and, un-DRY-ly (but for good reason, i think):

diff --git a/jupyterlab/staging/webpack.config.js b/jupyterlab/staging/webpack.config.js
index a653b053c..f06a404b8 100644
--- a/jupyterlab/staging/webpack.config.js
+++ b/jupyterlab/staging/webpack.config.js
@@ -82,10 +82,9 @@ function JupyterLabPlugin() { }
 
 JupyterLabPlugin.prototype.apply = function(compiler) {
 
-  compiler.plugin('after-emit', function(compilation, callback) {
+  compiler.hooks.afterEmit.tap('JupyterLabPlugin', function() {
     var staticDir = jlab.staticDir;
     if (!staticDir) {
-      callback();
       return;
     }
     // Ensure a clean static directory on the first emit.
@@ -94,7 +93,6 @@ JupyterLabPlugin.prototype.apply = function(compiler) {
     }
     this._first = false;
     fs.copySync(buildDir, staticDir);
-    callback();
   }.bind(this));
 };

These are now async events, so there isn't a callback to call, but given where we were calling them, I don't think it matters. I do wonder whether this will cause a 🏇 condition, but my intuition suggests the incremental build will always take longer than the copy... or the Sync part will block stuff.

It's hard to test the end-user build in $PREFIX/.../staging due to the missing @jupyterlab/attachments, but I'll be firing up verdaccio and doing a local publish to try that in a tick...

@bollwyvl
Copy link
Contributor

@bollwyvl bollwyvl commented Jun 9, 2018

Ah. So master is currently showing

Hash: e2dfb7bb2f74b612a9f8
Version: webpack 2.7.0
Time: 41804ms
                                 Asset     Size  Chunks                    Chunk Names
        vendor.26f362dc3ec62e65e88d.js  1.63 MB       2  [emitted]  [big]  vendor
  674f50d287a8c48dc19ba404d20fe713.eot   166 kB          [emitted]         
  b06871f281fee6b241d60582ae9369b9.ttf   166 kB          [emitted]         
 fee66e712a8a08eef5805a46892932ad.woff    98 kB          [emitted]         
af7ae505a9eed503f8b8e6982036873e.woff2  77.2 kB          [emitted]         
             0.d22431518ad1366208f5.js   465 kB       0  [emitted]  [big]  
          main.cad19cf5ebf6aed1c2f1.js  2.28 MB       1  [emitted]  [big]  main  <<<< OLD MAIN
  912ec66d7572ff821749319396470bde.svg   444 kB          [emitted]  [big]  
      manifest.e4462570b1e3ed2621df.js  1.62 kB       3  [emitted]         manifest
         0.d22431518ad1366208f5.js.map  1.36 MB       0  [emitted]         
      main.cad19cf5ebf6aed1c2f1.js.map  8.13 MB       1  [emitted]         main
    vendor.26f362dc3ec62e65e88d.js.map  5.94 MB       2  [emitted]         vendor
  manifest.e4462570b1e3ed2621df.js.map  8.24 kB       3  [emitted]         manifest
                            index.html  1.53 kB          [emitted]   

vs this PR:


webpack v4.12.0

3c6d7a29656bd4442b4a
  size     name    module                                                                                                                                                                                                                                                                                                                                                                                           status
  14.1 kB  0E2z    ../packages/filebrowser-extension/lib/index.js                                                                                                                                                                                                                                                                                                                                                   optional built
  2.37 kB  0HjG    ../packages/markdownviewer-extension/lib/index.js                                                                                                                                                                                                                                                                                                                                                optional built
  7.2 kB   13xh    ../packages/inspector-extension/lib/index.js                                                                                                                                                                                                                                                                                                                                                     optional built
  1.74 kB  3gBh    ../packages/tabmanager-extension/lib/index.js                                                                                                                                                                                                                                                                                                                                                    optional built
  7 kB     IoZ4    ../packages/completer-extension/lib/index.js                                                                                                                                                                                                                                                                                                                                                     optional built
  34.6 kB  JiS9    ./static/index.out.js                                                                                                                                                                                                                                                                                                                                                                            built
  3.69 kB  KEwF    ../packages/shortcuts-extension/lib/index.js                                                                                                                                                                                                                                                                                                                                                     optional built
  2.25 kB          ../packages/json-extension/lib/index.js                                                                                                                                                                                                                                                                                                                                                          optional built
  11.8 kB  XR2L    ../packages/help-extension/lib/index.js                                                                                                                                                                                                                                                                                                                                                          optional built
  2.69 kB  YFbD    ../packages/vdom-extension/lib/index.js                                                                                                                                                                                                                                                                                                                                                          optional built
  15.7 kB  ZU4L    ../packages/docmanager-extension/lib/index.js                                                                                                                                                                                                                                                                                                                                                    optional built
  4.15 kB  xsPQ    ../packages/pdf-extension/lib/index.js                                                                                                                                                                                                                                                                                                                                                           optional built
  2.39 kB  zIeq    ../packages/javascript-extension/lib/index.js                                                                                                                                                                                                                                                                                                                                                    optional built
  328 B    6       multi @phosphor/algorithm @phosphor/application @phosphor/commands @phosphor/coreutils @phosphor/datagrid @phosphor/disposable @phosphor/domutils @phosphor/dragdrop @phosphor/messaging @phosphor/properties @phosphor/signaling @phosphor/virtualdom @phosphor/widgets ajv ansi_up codemirror comment-json es6-promise marked moment path-posix react react-dom sanitize-html url-parse xterm  built
  40 B     9       multi whatwg-fetch ./static/index.out.js                                                                                                                                                                                                                                                                                                                                                         built

  size     name    asset                                                                                                                                                                                                                                                                                                                                                                                            status
  1.54 MB  vendor  vendor.8f4aff1626bbf02527d2.js                                                                                                                                                                                                                                                                                                                                                                   emitted
  166 kB   eot     674f50d287a8c48dc19ba404d20fe713.eot                                                                                                                                                                                                                                                                                                                                                             emitted
  77.2 kB  woff2   af7ae505a9eed503f8b8e6982036873e.woff2                                                                                                                                                                                                                                                                                                                                                           emitted
  444 kB   svg     912ec66d7572ff821749319396470bde.svg                                                                                                                                                                                                                                                                                                                                                             emitted
  98 kB    woff    fee66e712a8a08eef5805a46892932ad.woff                                                                                                                                                                                                                                                                                                                                                            emitted
  423 kB   js      0.694c1bea111cef5ab380.js                                                                                                                                                                                                                                                                                                                                                                        emitted
  3.67 kB  js      1.f07d1dd71ae3c4532523.js                                                                                                                                                                                                                                                                                                                                                                        emitted
  166 kB   ttf     b06871f281fee6b241d60582ae9369b9.ttf                                                                                                                                                                                                                                                                                                                                                             emitted
>>>> NEW MAIN  3.49 MB  main    main.c5edd8ed2c54de68a1f1.js                                                                                                                                                                                                                                                                                                                                                                     emitted
  1.3 MB   map     0.694c1bea111cef5ab380.js.map                                                                                                                                                                                                                                                                                                                                                                    emitted
  9.45 kB  map     1.f07d1dd71ae3c4532523.js.map                                                                                                                                                                                                                                                                                                                                                                    emitted
  5.55 MB  vendor  vendor.8f4aff1626bbf02527d2.js.map                                                                                                                                                                                                                                                                                                                                                               emitted
  14.2 MB  main    main.c5edd8ed2c54de68a1f1.js.map                                                                                                                                                                                                                                                                                                                                                                 emitted
  1.43 kB  html    index.html       

This is troubling. I'll have a look with webpack-stats-duplicates and webpack-bundle-analyzer this evening...

@bollwyvl
Copy link
Contributor

@bollwyvl bollwyvl commented Jun 9, 2018

Of note, webpack-command does not support the --json flag, so can't be used with the analyzers. I didn't dig in too much...

master dupes:

yarn run v1.7.0
$ webpack-stats-duplicates stat.json
Duplicate module d3-interpolate found in webpack configuration 0
	../node_modules/d3-interpolate
	../node_modules/vega-scale/node_modules/d3-interpolate

Duplicate module @phosphor/widgets found in webpack configuration 0
	../node_modules/@phosphor/widgets
	../node_modules/css-loader!../node_modules/@phosphor/widgets

Duplicate module font-awesome found in webpack configuration 0
	../node_modules/font-awesome
	../node_modules/css-loader!../node_modules/font-awesome

Duplicate module xterm found in webpack configuration 0
	../node_modules/xterm
	../node_modules/css-loader!../node_modules/xterm

Duplicate module codemirror found in webpack configuration 0
	../node_modules/codemirror
	../node_modules/css-loader!../node_modules/codemirror

Duplicate module d3-format found in webpack configuration 0
	../node_modules/d3-format
	../node_modules/d3-scale/node_modules/d3-format

Duplicate module d3-interpolate found in webpack configuration 0
	../node_modules/d3-interpolate
	../node_modules/vega-encode/node_modules/d3-interpolate

Duplicate module d3-color found in webpack configuration 0
	../node_modules/d3-color
	../node_modules/vega-parser/node_modules/d3-color

Duplicate module hash-base found in webpack configuration 0
	../node_modules/hash-base
	../node_modules/md5.js/node_modules/hash-base

Duplicate module domelementtype found in webpack configuration 0
	../node_modules/domelementtype
	../node_modules/dom-serializer/node_modules/domelementtype

Duplicate module isarray found in webpack configuration 0
	../node_modules/readable-stream/node_modules/isarray
	../node_modules/buffer/node_modules/isarray

Duplicate module inherits found in webpack configuration 0
	../node_modules/inherits
	../node_modules/util/node_modules/inherits

Duplicate module ajv found in webpack configuration 0
	./node_modules/ajv
	../packages/coreutils/node_modules/ajv

this pr:

Duplicate module ajv found in webpack configuration 0
	./node_modules/ajv
	../packages/coreutils/node_modules/ajv

Duplicate module codemirror found in webpack configuration 0
	../node_modules/codemirror
	../node_modules/css-loader!../node_modules/codemirror

Duplicate module @phosphor/widgets found in webpack configuration 0
	../node_modules/@phosphor/widgets
	../node_modules/css-loader!../node_modules/@phosphor/widgets

Duplicate module font-awesome found in webpack configuration 0
	../node_modules/font-awesome
	../node_modules/css-loader!../node_modules/font-awesome

Duplicate module xterm found in webpack configuration 0
	../node_modules/xterm
	../node_modules/css-loader!../node_modules/xterm

Duplicate module hash-base found in webpack configuration 0
	../node_modules/hash-base
	../node_modules/md5.js/node_modules/hash-base

Duplicate module inherits found in webpack configuration 0
	../node_modules/inherits
	../node_modules/util/node_modules/inherits

Duplicate module domelementtype found in webpack configuration 0
	../node_modules/domelementtype
	../node_modules/dom-serializer/node_modules/domelementtype

Duplicate module isarray found in webpack configuration 0
	../node_modules/readable-stream/node_modules/isarray
	../node_modules/buffer/node_modules/isarray

master
screenshot from 2018-06-09 14-21-09

this pr
screenshot from 2018-06-09 14-21-45

@jasongrout
Copy link
Contributor Author

@jasongrout jasongrout commented Jun 9, 2018

Should we switch to webpack-cli?

@bollwyvl
Copy link
Contributor

@bollwyvl bollwyvl commented Jun 9, 2018

@bollwyvl bollwyvl mentioned this pull request Jun 10, 2018
@jasongrout
Copy link
Contributor Author

@jasongrout jasongrout commented Jun 11, 2018

So at first glance it looks like the big things in this PR's bundle are vega should be moved out, and phosphor should be deduplicated and codemirror should be deduplicated.

jasongrout added 5 commits Jun 11, 2018
This takes care of the ansi_up source map warning we keep getting.
webpack-command does not seem to support exporting the stats in json format.
@jasongrout
Copy link
Contributor Author

@jasongrout jasongrout commented Jun 11, 2018

I rebased on master (to get rid of the commits that were in other merged PRs) and updated to webpack-cli and changed the webpack commands to emit stats so I can check out those cool tools you are using.

@jasongrout
Copy link
Contributor Author

@jasongrout jasongrout commented Jun 11, 2018

@bollwyvl - I separated out the vega-* packages into their own bundle. Here is the dev build:

screen shot 2018-06-11 at 12 43 18 pm

 4.5K 0.960d76737586a0aac75c.js
 5.7K 0.960d76737586a0aac75c.js.map
 890K 1.30837281d24d6db701c1.js
 1.0M 1.30837281d24d6db701c1.js.map
 162K 674f50d287a8c48dc19ba404d20fe713.eot
 434K 912ec66d7572ff821749319396470bde.svg
  75K af7ae505a9eed503f8b8e6982036873e.woff2
 162K b06871f281fee6b241d60582ae9369b9.ttf
 788B error.html
  96K fee66e712a8a08eef5805a46892932ad.woff
 1.5K index.html
  34K index.out.js
 1.9M main.e4ebe8c8d0ec2bce3d63.js
 3.1M main.e4ebe8c8d0ec2bce3d63.js.map
  11K package.json
 3.2M vega~main.38126648547d1168770d.js
 2.9M vega~main.38126648547d1168770d.js.map
 6.1M vendors~main.3af66211a18b7e1ca7aa.js
 6.6M vendors~main.3af66211a18b7e1ca7aa.js.map

@jasongrout
Copy link
Contributor Author

@jasongrout jasongrout commented Jun 11, 2018

And here are the numbers for the production build:

415K 0.694c1bea111cef5ab380.js
1.2M 0.694c1bea111cef5ab380.js.map
3.6K 1.f07d1dd71ae3c4532523.js
9.2K 1.f07d1dd71ae3c4532523.js.map
162K 674f50d287a8c48dc19ba404d20fe713.eot
434K 912ec66d7572ff821749319396470bde.svg
 75K af7ae505a9eed503f8b8e6982036873e.woff2
162K b06871f281fee6b241d60582ae9369b9.ttf
788B error.html
 96K fee66e712a8a08eef5805a46892932ad.woff
1.5K index.html
 34K index.out.js
726K main.422d8f1e07c68d925ed0.js
2.7M main.422d8f1e07c68d925ed0.js.map
 11K package.json
516K vega~main.2affa1474c450e105530.js
3.4M vega~main.2affa1474c450e105530.js.map
2.2M vendors~main.18bcec02d43a1efb35b9.js
7.6M vendors~main.18bcec02d43a1efb35b9.js.map

@jasongrout
Copy link
Contributor Author

@jasongrout jasongrout commented Jun 11, 2018

New numbers in prod build after restoring our existing uglify options:

442K 0.694c1bea111cef5ab380.js
1.3M 0.694c1bea111cef5ab380.js.map
3.6K 1.f07d1dd71ae3c4532523.js
9.3K 1.f07d1dd71ae3c4532523.js.map
162K 674f50d287a8c48dc19ba404d20fe713.eot
434K 912ec66d7572ff821749319396470bde.svg
 75K af7ae505a9eed503f8b8e6982036873e.woff2
162K b06871f281fee6b241d60582ae9369b9.ttf
788B error.html
 96K fee66e712a8a08eef5805a46892932ad.woff
1.5K index.html
 34K index.out.js
755K main.422d8f1e07c68d925ed0.js
2.7M main.422d8f1e07c68d925ed0.js.map
 11K package.json
523K vega~main.6c6420135a192f937426.js
3.3M vega~main.6c6420135a192f937426.js.map
2.3M vendors~main.348f8e42a1dc9c2b4233.js
7.8M vendors~main.348f8e42a1dc9c2b4233.js.map

@jasongrout jasongrout changed the title WIP Upgrade webpack and associated packages Upgrade webpack and associated packages Jun 12, 2018
@jasongrout
Copy link
Contributor Author

@jasongrout jasongrout commented Jun 12, 2018

One last compare, to make sure it's clear:

pip install -ve .
jlpm run build
cd dev_mode
yarn run build:prod
ls -lash static/*.js

gives this with current master:

456K static/0.d22431518ad1366208f5.js
 34K static/index.out.js
2.2M static/main.da3aadf58699cc652bee.js
1.6K static/manifest.351c00360b8d27f69af0.js
1.6M static/vendor.26f362dc3ec62e65e88d.js

and this with this PR

442K static/0.694c1bea111cef5ab380.js
3.6K static/1.f07d1dd71ae3c4532523.js
 34K static/index.out.js
755K static/main.422d8f1e07c68d925ed0.js
523K static/vega~main.6c6420135a192f937426.js
2.3M static/vendors~main.348f8e42a1dc9c2b4233.js

@jasongrout
Copy link
Contributor Author

@jasongrout jasongrout commented Jun 12, 2018

And it's not super-clear that we should actually split vega off, without it being dynamically loaded. However, I think it's still a net win since the browser can download the vega js in parallel with the others.

This seems to be causing an error in Appveyor (such as https://ci.appveyor.com/project/jupyterlab/jupyterlab/build/1.0.7521/job/cc42qxscjdpm5t0n - python 3.6 has an error removing one of the cached files when cleaning up a temporary directory), so we’ll revert to the old non-cached behavior for now.
@jasongrout
Copy link
Contributor Author

@jasongrout jasongrout commented Jun 12, 2018

@bollwyvl, @ian-r-rose, @afshin - I think this is ready for review now. See #4702 (comment) for the high-level effect.

@jasongrout
Copy link
Contributor Author

@jasongrout jasongrout commented Jun 12, 2018

I turned off uglify caching in production builds because (a) we didn't have it on before, and (b) it was causing errors in appveyor (see the appveyor build log for 58723b9: https://ci.appveyor.com/project/jupyterlab/jupyterlab/build/1.0.7521). I think we can address uglify caching in a separate issue.

@jasongrout
Copy link
Contributor Author

@jasongrout jasongrout commented Jun 12, 2018

CC also @saulshanabrook for review too.

@jasongrout
Copy link
Contributor Author

@jasongrout jasongrout commented Jun 12, 2018

I think we can address uglify caching in a separate issue.

This is now #4716.

@saulshanabrook saulshanabrook self-requested a review Jun 12, 2018
Copy link
Member

@saulshanabrook saulshanabrook left a comment

This works for me locally. I also compared this to the old setup by using the Lighthouse tool to get a sense of total load time. It says we download ~200KB less data, but load times are about the same.

Old on the left, new on the right:
screen shot 2018-06-12 at 4 02 43 pm

@saulshanabrook saulshanabrook merged commit 296b92d into jupyterlab:master Jun 12, 2018
2 checks passed
@lock lock bot locked as resolved and limited conversation to collaborators Aug 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants