Open Bug 1384255 Opened 3 years ago Updated 1 year ago

Reduce the size of the webpack bundles

Categories

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

enhancement

Tracking

(Not tracked)

People

(Reporter: emorley, Unassigned)

References

(Depends on 4 open bugs, Blocks 1 open bug)

Details

Attachments

(3 files, 1 obsolete file)

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!
> 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?
Duplicate of this bug: 1384254
Attached image treeherder-webpack-bundle-analysis.png (obsolete) —
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
Component: Treeherder → Treeherder: Frontend
Attachment #8920806 - Flags: checkin+
Some other ways to visualise the build are discussed here:
https://github.com/webpack/webpack/issues/690
Assignee: nobody → emorley
Depends on: 1426398, 1413156
Summary: Clean up the JS module require() usage to allow for a smaller webpack build → Reduce the size of the webpack bundles
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
Attachment #8916944 - Attachment is obsolete: true
Blocks: 1067846
Attached file stats.json 2017-12-20
Depends on: 1426714
Depends on: 1426902
Depends on: 1427295
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
Depends on: 1440616
Depends on: 1440774
Depends on: 1441614
Depends on: 1466494
No longer depends on: 1413156
Depends on: 1472542
Depends on: 1492003
Depends on: 1497931
Depends on: 1364894
Depends on: 1502192
Depends on: 1506805
Depends on: 1507903
Depends on: 1507906
Depends on: 1510250
Depends on: 1513527
Depends on: 1525239
Assignee: emorley → nobody
Depends on: 1529857
Depends on: 1530607
Depends on: 1450044
Duplicate of this bug: 1572103
No longer blocks: 1571643
No longer blocks: 1571404
You need to log in before you can comment on or make changes to this bug.