1384255 - Reduce the size of the webpack bundles

Status: RESOLVED WONTFIX

(Tree Management :: Treeherder: Frontend, enhancement, P3)

Description

[Ed Morley [:emorley]]

As part of switching to Neutrino/webpack, the vendored JS and traditional <script> tags were replaced with npm packages and usages of `require()`. Since it would have been too much of a pain to do at the time, the existing packages were on the most part `require()`ed globally, via the per-page entrypoint files.

For example:
https://github.com/mozilla/treeherder/blob/master/ui/entry-index.js

Since then we've also added some requires to specific modules/components, both in angular fixture form, eg:
https://github.com/mozilla/treeherder/blob/0f060a4773b366473fe71e945b5e6f3f6ca5c76b/ui/js/services/main.js#L217-L225
(which get used like so: https://github.com/mozilla/treeherder/blob/0f060a4773b366473fe71e945b5e6f3f6ca5c76b/ui/js/controllers/perf/compare.js#L874)

...and also a more usual webpack form:
https://github.com/mozilla/treeherder/blob/0f060a4773b366473fe71e945b5e6f3f6ca5c76b/ui/js/services/tcactions.js#L4-L8

From what I understand:
* requiring things without assigning to a variable isn't best practice
* the `import { foo } from "module-name"` form is better than `require()`
* we should require/import things where they are used rather than in the main entrypoint file

However what I don't understand is:
* why do some modules work anyway without being assigned to a variable / `window`?
* whether the angular fixture form is better/worse than the more usual webpack form linked above
* why webpack doesn't like the angular fixture form and sometimes breaks the minified build when used that way
* whether imports/requires for rarely used things should be inside the function or still at the top of the file (does webpack optimise that out? do they just get imported the first time the function is called or every time? etc)

I've tried looking around but there isn't much guidance as to how to how to combine angularjs 1 with webpack.

Eli, can you provide any guidance as to the above / suggest who might know more about the angular parts?

Thanks!

Comment 1

[:Eli Perelman]

> why do some modules work anyway without being assigned to a variable / `window`?

Most likely these modules were built as UMD modules instead of just CommonJS, which will assign them to the window if not imported using another mechanism.

> whether the angular fixture form is better/worse than the more usual webpack form linked above

I can't say whether Angular prefers this syntax, but if it were me, I would much more prefer the import syntax, and opting for dynamic imports, e.g. import(), when you need code splitting or dynamic assets. You can also use dynamic imports within the Angular fixture I'm assuming.

> why webpack doesn't like the angular fixture form and sometimes breaks the minified build when used that way

My guess is because Webpack statically analyzes requires/imports. I can't say *why* this would break, but I think moving to imports would improve this.

> whether imports/requires for rarely used things should be inside the function or still at the top of the file

This is a judgement call I think. If what you are importing is small, you may want to just use the static import syntax, while for things that may be larger or can be loaded on demand, use dynamic imports. We do this within Tools for the Inspector by dynamically loading modules for tabs that may not be opened, only loading the module when the route will be rendered:

https://github.com/taskcluster/taskcluster-tools/blob/master/src/views/UnifiedInspector/Inspector.js#L19-L23
https://github.com/taskcluster/taskcluster-tools/blob/master/src/views/UnifiedInspector/Inspector.js#L437

> does webpack optimise that out?

No, this is determined via usage of static or dynamic imports.

> do they just get imported the first time the function is called or every time?

A bundle is built for every page Webpack creates, so anything in the static import module tree will be loaded upon loading that page. Anything within that static bundle that then subsequently calls a dynamic import will make another HTTP request and load that additional module, which would then be cached.

Does that help?

Comment 3

[Ed Morley [:emorley]]

Attachment: treeherder-webpack-bundle-analysis.png

I ran the webpack-bundle-analyzer [1] against Treeherder and the results were pretty horrific - screenshot attached (the interactive live view is much easier to navigate, but this gets the point across for now).

Issues I can see from a quick glance:
* there are many vendor libs in the entrypoint bundles rather than the vendor bundle. This means we have 6 copies of bootstrap between them for example!
  -> having to keep the vendor list up to date is clearly not working -- perhaps we should change Neutrino to use the automatic `node_modules` path solution instead?
* due to the use of globals/provide plugin, each entrypoint actually includes the other pages content too
* due to the use of globals/provide plugin, there's a lot that isn't being removed by tree shaking

Some easy wins here! Plus many of the changes will hopefully reduce webpack build times, since it will mean a smaller graph of modules for webpack to perform tree shaking across.

[1] https://github.com/webpack-contrib/webpack-bundle-analyzer

Comment 4

[GitHub Autolander Bot]

Attachment: [treeherder] mozilla:clarify-cache-template > mozilla:master

Comment 5

[Treeherder GitHub Bugbot]

Commit pushed to master at https://github.com/mozilla/treeherder

https://github.com/mozilla/treeherder/commit/55e83c11245bdaa1212e37e171806665aabce293
Bug 1384255 - Improve the documentation of cache-template.js (#2862)

Comment 6

[Ed Morley [:emorley]]

Some other ways to visualise the build are discussed here:
https://github.com/webpack/webpack/issues/690

Comment 7

[Ed Morley [:emorley]]

Another useful tool:
https://medium.com/webpack/bundle-buddy-and-webpack-commons-chunk-101da29166bf

Comment 8

[Ed Morley [:emorley]]

Attachment: Bundle analysis report 2017-12-20

This analysis report was generated via `yarn start` (so is not minified). The minified build sizes are:

           vendor.acd0c03e91efa86abebb.bundle.js    1.96 MB
            index.6f4bfab64f946dcad8e9.bundle.js     759 kB
             perf.82d81f14365576372906.bundle.js     709 kB
        logviewer.ec683cd72869f349fe3e.bundle.js     539 kB
    failureviewer.ad8e6e8da53ec24b86c9.bundle.js     497 kB
         testview.8ba96efe3b10d34f3dad.bundle.js     383 kB
        userguide.75e5f4ef3ccb42f84191.bundle.js     242 kB
         manifest.a8c19c41f184dbb37cb8.bundle.js    1.64 kB

Comment 9

[Ed Morley [:emorley]]

Attachment: stats.json 2017-12-20

Comment 10

[Ed Morley [:emorley]]

There are a few options for tracking bundle size as part of CI - might be worth investigating to (a) confirm that changes made are improving them as expected, (b) stop future regressions. eg:
https://github.com/siddharthkp/bundlesize