Update Neutrino from v4 to v9



2 years ago
2 months ago


(Reporter: emorley, Assigned: emorley)


(Blocks: 4 bugs)



(5 attachments, 2 obsolete attachments)



2 years ago
Broken out of bug 1353014.

PR in progress:
Created attachment 8867693 [details] [review]
[treeherder] eliperelman:neutrino-v5 > mozilla:master
Created attachment 8870558 [details] [review]
[treeherder] mozilla:neutrino-v5 > mozilla:master
I'm going to see if I can finish this up.  I have a lot to learn about Webpack and Neutrino to do so, though.  :)
Assignee: nobody → cdawson
Ed--  Is it ok to assign this back to you?
Assignee: cdawson → emorley

Comment 5

2 years ago
Yup :-)


a year ago
Summary: Update from Neutrino 4.x to 5.x → Update from Neutrino 4.x to 6.x


a year ago
Blocks: 1351907
Hey Ed--  I'm hoping to merge the test-group UI into the main repo at some point before too long.  I still have one feature to add, but was wondering if this is on your soon-to-do list and/or if us tag-teaming would help move this along.

Thanks!  :)
Blocks: 1366909

Comment 7

a year ago
Yeah will return to this in the next week or two :-)


a year ago
Priority: P2 → P1


a year ago
Depends on: 1402532


a year ago
Depends on: 1403195
Created attachment 8914280 [details] [review]
[treeherder] mozilla:eslint-fixes > mozilla:master


a year ago
Attachment #8914280 - Flags: review?(cdawson)

Comment 9

a year ago
An update on work so far...

Neutrino v7 has now been released, which brings with it webpack 3 as well as a number of changes that reduce the amount of custom config we need (for example switching away from svg-url-loader, disabling babel-polyfill, ...). Updating my local branch to use it instead of v6 was very painless, so I'm now targeting v7.

I also wrote a POC for merging the manifest prototype into this repo with Neutrino v4 (bug 1366909 comment 3), so the Neutrino upgrade is no longer technically blocking that bug.

I've opened upstream PRs to fix a few things:

Debugging the inline SVG issue (introduced in Neutrino v6, resolved in v7) took quite a while, see:

And I opened issues for a few other things that I encountered:

Working on the upgrade has also exposed several existing issues in Treeherder's build process:
* Several of the .gif and .svg image assets (eg `dancing_cat.gif`, `logviewerIcon.svg`) were not being caught by the old custom Neutrino config, since it uses file-loader not html-loader so doesn't replace the references. As part of upgrading, the paths to these assets from the partials have to be updated, since they are currently incorrect relative to the source directory.
* There are also .png assets (eg `line_chart.png` and `tree_closed.png`) that are not currently being included in the webpack build. Fixing these is more involved, so I'll save this to a followup bug.
* The vendored package list is incomplete, meaning several libraries are ending up in the main app bundles (eg `index.HASH.bundle.js`, `perf.HASH.bundle.js`, ...) rather than the vendor bundle (eg `vendor.HASH.bundle.js`). I won't fix this as part of this bug, but will file another for doing so later.
* The webpack-dev-server and react hot reload functionality is broken on any page but index. Though I'll also save this to a followup bug.
* (We knew this since it was deliberately disabled in bug 1373376, but for completeness..) The bundle hashes changed after every build, meaning that the vendor bundle would never be cached across deploys.

Things that are finished:
* neutrino build (the production build)
* neutrino lint
* neutrino test (the karma test run)
  -> though there's a require path bug I'm having to work around that would be good to figure out

* neutrino start (the dev server mode)
  -> Working, but need to re-wire up the API proxying
  -> Need to also resolve bug 1363722, to save having to re-implement `neutrino start:local`
* Running a final comparison of the generated build assets before/after
* Updating docs
Depends on: 1363722
Summary: Update from Neutrino 4.x to 6.x → Update Neutrino from v4 to v7
Comment on attachment 8914280 [details] [review]
[treeherder] mozilla:eslint-fixes > mozilla:master

This is truly awesome.  I really prefer strict linting and styling.  :)  I almost don't care what it is, as long as our code is consistent.  Thanks for doing this!!
Attachment #8914280 - Flags: review?(cdawson) → review+
Commits pushed to master at https://github.com/mozilla/treeherder

Bug 1364894 - eslint --fix for array-bracket-spacing


Bug 1364894 - eslint --fix for brace-style


Bug 1364894 - eslint --fix for computed-property-spacing


Bug 1364894 - eslint --fix for dot-location


Bug 1364894 - eslint --fix for dot-notation


Bug 1364894 - eslint --fix for lines-around-directive


Bug 1364894 - eslint --fix for newline-per-chained-call


Bug 1364894 - eslint --fix for no-confusing-arrow


Bug 1364894 - eslint --fix for no-multi-spaces


Bug 1364894 - eslint --fix for no-tabs


Bug 1364894 - eslint --fix for no-unneeded-ternary


Bug 1364894 - eslint --fix for no-useless-concat


Bug 1364894 - eslint --fix for object-property-newline


Bug 1364894 - eslint --fix for operator-assignment


Bug 1364894 - eslint --fix for semi-spacing


Bug 1364894 - eslint --fix for space-in-parens


Bug 1364894 - eslint --fix for space-unary-ops


Bug 1364894 - eslint --fix for wrap-iife



a year ago
Depends on: 1408776


a year ago
No longer blocks: 1364010


a year ago
Component: Treeherder → Treeherder: Frontend
No longer blocks: 1366909

Comment 12

a year ago
Neutrino 8 now has the multi-entrypoint support we need to simplify the configs considerably.

Bug 1408784 is a dep since with newer Neutrino the dependency resolution is fixed, such that the duplicate version of Angular.js pulled in by the broken pre-1.0 UI router will now affect us. By updating to UI router 1.0+ that issue no longer occurs.
Depends on: 1408784
Summary: Update Neutrino from v4 to v7 → Update Neutrino from v4 to v8
Commits pushed to master at https://github.com/mozilla/treeherder

Bug 1364894 - List vendor deps alphabetically and one per line

To improve readability and reduce conflicts between PRs.

Bug 1364894 - List eslint globals alphabetically and one per line

To improve readability and reduce conflicts between PRs.

Bug 1364894 - Remove unnecessary eslint global exemptions

Since the instances that required these exemptions no longer exist.


a year ago
Attachment #8867693 - Attachment is obsolete: true


a year ago
Attachment #8870558 - Attachment is obsolete: true


a year ago
Attachment #8914280 - Flags: checkin+


a year ago
Attachment #8939031 - Flags: checkin+

Comment 15

a year ago
Updating angular-ui-router to a less broken version was a blocker for this work to prevent a double-angular-bundling issue (see bug 1408784 comment 7), however that's turned out to be pretty involved. 

As such, updating to Angular 1.6 is the next best bet (whilst the build will still try and include angular twice, yarn hoisting will de-dupe the versions so long as we're always on the latest version so it matches angular-ui-router's tilde-range).
Depends on: 1364888
No longer depends on: 1408784

Comment 16

a year ago
Now that bug 1364888 is fixed, I rebased the work here on top of the changes from bug 1398386, bug 1408100, bug 1366909, bug 1427295, bug 1426902 - and then updated it to use Neutrino 8 instead of Neutrino 7, so it can take advantage of the features we need added in v8.

However first pass at building with that resulted in much longer build times compared to Neutrino v7, and the build failed at the end anyway.

I've then rolled back to Neutrino 7 so I can at least get a finished baseline before debugging the Neutrino v8 issues, however it appears the Bootstrap 4 upgrade has triggered some strange bug with the treeherder menu popup only seen in development mode, not in a production build. There are zero exceptions in the console or anything else to go on, so is proving to be a bit of a pain (next step is to bisect the config that differs between `start` and `build` to figure out what is causing the issue).

Comment 18

a year ago
Comment on attachment 8944814 [details] [review]
Link to GitHub pull-request: https://github.com/mozilla/treeherder/pull/3161

Happy to have a Vidyo call about this / Neutrino or webpack in general, if it would be of interest? :-)
Attachment #8944814 - Flags: review?(cdawson)


a year ago
Depends on: 1432840
Attachment #8944814 - Flags: review?(cdawson) → review+
Commits pushed to master at https://github.com/mozilla/treeherder

Bug 1364894 - Remove unused support for .tmpl partials

The last `.tmpl` file was removed in #2786. The `test` regex in
`neutrino-custom/base.js` has been removed entirely, since without
the `.tmpl` extension it would be no different from the default:

Bug 1364894 - Add missing image file extensions to url-loader

Neutrino 4 only configured `url-loader` for use with `.png` and `.jpg`.
This means that once we start parsing HTML properly in later commits,
we would otherwise get the following error:

ERROR in ./ui/img/dancing_cat.gif
Module parse failed: C:\Users\Ed\src\treeherder\ui\img\dancing_cat.gif Unexpected character '�' (1:6)
You may need an appropriate loader to handle this file type.
(Source code omitted for this binary file)
 @ ./ui/partials/perf/comparectrl.html 1:214-250
 @ ./ui/partials \.html$
 @ ./ui/js/cache-templates.js
 @ ./ui/js/perfapp.js
 @ ./ui/entry-perf.js
 @ multi ./ui/entry-perf.js

Bug 1364894 - Override Neutrino 4's broken SVG mimetype

Neutrino 4 configured the SVG mimetype as `application/svg+xml`, which
Firefox doesn't appear to like. Neutrino 8 instead doesn't specify a
mimetype directly, allowing `url-loader` to autodetect via the NPM
`mime` package (which returns `image/svg+xml` for SVGs).

This switches to the Neutrino 8 approach, to prevent the job detail
panel's log viewer icon breaking in later commits when we start
parsing HTML properly (which causes url-loader to embed the icon as
a base64 encoded data URI rather than a URL to a file).

Bug 1364894 - Load HTML with html-loader not raw-loader

Neutrino 4 configures `file-loader` as the loader for HTML (rather than
the more usual `html-loader`), which means the HTML is not parsed to
look for further dependencies such as `<img src="...">` tags. Our
custom Neutrino config overrode that to `raw-loader` (presumably to
work around bugs caused by the use of `file-loader`), which doesn't
parse HTML either.

Instead, these assets were being manually copied to `dist/img/` by
`neutrino-custom/production.js`'s `CopyPlugin` rule, effectively
circumventing the webpack build process.

Newer Neutrino correctly uses `html-loader`, causing our HTML to be
parsed during the webpack build for the first time. However now that
the images are being resolved at build time rather than runtime, the
relative paths need to be updated to account for the directory layout
differences between `src/` and `dist/`, to prevent build errors.

A significant benefit of this change is that images referenced from
HTML will now be output with hashed filenames, meaning they get given
long-lived `Cache-Control` headers by WhiteNoise.


Bug 1364894 - Make html-loader treat favicons as dependencies too

Previously `html-loader` only parsed `<img src="...">` tags when
looking for assets/dependencies. Now the `<link href="...">` tags
for favicons are processed too, which means `img/tree_open.png`
and friends will be included in the webpack build and not need to
be manually copied into `dist/img/`:

This does not visible change the number of hashed images output to
`dist/`, since the favicons are small enough that `url-loader` inlines
them in the HTML as base64 encoded data URIs (this is adjustable if
not desired later):

Bug 1364894 - Use imports for images in thFavicons

This ensures that webpack knows they are a dependency, meaning:
* no need to manually copy them to `dist/img/` using `CopyPlugin`
  (the wildcard copy rules will be cleaned up in a later commit)
* they are inlined as a base64 encoded data URI by `url-loader`.

The changes to `thFaviconLink` are required to prevent:
Error: [$interpolate:interr] Can't interpolate: {{favicon}}
Error: [$sce:insecurl] Blocked loading resource from url not allowed
by $sceDelegate policy.  URL: data:image/png;base64,...

Which are due to AngularJS not trusting data URIs by default. See:

Bug 1364894 - Ensure revision.txt exists in development too

Currently `revision.txt` only exists on Heroku, since it's generated
by the Heroku-only `post_compile` script, just prior to `yarn build`.

However this means:
* HTTP 404s of `revision.txt` are seen in the browser console when
  developing locally, which gives the appearance of something being
  broken, even though it's not.
* when we convert the wildcard `CopyPlugin` rule to an explicit list
  of files to copy (in a later commit), it will cause errors when
  building locally, since `CopyPlugin` expects all declared files
  to exist.

Adding a placeholder file prevents both of the above.

Bug 1364894 - Whitelist rather than blacklist files copied to dist/

With the changes in previous commits, all of the assets that were
originally manually copied to `dist/img/` are now correctly handled
by webpack as dependencies (and so emitted to `dist/` automatically).

As such, this leaves only three files that need copying from `src/`,
so they are now listed explicitly to avoid having to continually
update `ignore` to prevent extra files from sneaking in:

As a result of this change, the following assets are no longer
needlessly created under `dist/` as part of `yarn build`:

To confirm that this would not break anything, the JS and HTML files
under `dist/` were grepped for the string `img/`, and there are no
references remaining.

Comment 20

8 months ago
I've finally had a chance to come back to this over the last couple of weeks.

Since the update in comment 16, I have:
* rebased the previous work
* tracked down the issues seen there, which turned out to be:
  - slow CSS minification (https://github.com/mozilla-neutrino/neutrino-dev/issues/678)
  - node OOM issue relating to `devtool: 'source-map'`, that can be worked around by running the build with `node --max_old_space_size=4096`

However the build times for Neutrino 8 (webpack 3) were still slightly slower than what they are currently (Neutrino 4, webpack 2) - so I had a quick experiment with webpack 4 to see if it would help.

The results were very promising, so I made a number of upstream Neutrino changes:
* webpack 3 -> 4
* babel 6 -> 7
* babel-minify -> uglify-es
* chunking/optimisation/caching improvements
* and a bunch more:
  - PRs: https://github.com/mozilla-neutrino/neutrino-dev/pulls?q=is%3Apr+author%3Aedmorley
  - Issues: https://github.com/mozilla-neutrino/neutrino-dev/issues?q=is%3Aissue+author%3Aedmorley

The end result is between 5-10x faster for both Treeherder and the various other projects that use Neutrino (such as the taskcluster sites). 

In some cases builds that were previously taking 7 minutes now take 30 seconds - plus are more optimized for both download and parsed JS size on top :-)

I'm now working on the final tweaks needed before we can publish a beta of Neutrino 9, which can then be used by Treeherder/Taskcluster/... (I'm currently having to use `yarn link`ed packages locally).


7 months ago
Depends on: 1468650


4 months ago
Attachment #9011146 - Flags: review?(cdawson)
Attachment #9011146 - Flags: review?(cdawson) → review+

Comment 22

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

Bug 1364894 - Fix ESLint issues found by Neutrino 9 (#4056)

This pre-emptively fixes the issues found by the newer ESLint and
ESLint plugins that come with Neutrino 9 - in order to reduce the
size of the Neutrino 9 PR.

Comment 23

3 months ago
The public class property support of newer Neutrino will allow as to fix bug 1480166 properly.
Summary: Update Neutrino from v4 to v8 → Update Neutrino from v4 to v9


3 months ago
Blocks: 1480166


3 months ago
Blocks: 1364045


3 months ago
Blocks: 1384255, 1183749


3 months ago
Blocks: 1502192


3 months ago
Attachment #9021657 - Flags: review?(cdawson)
Comment on attachment 9021657 [details] [review]
Link to GitHub pull-request: https://github.com/mozilla/treeherder/pull/4216

As noted in the PR, there is an issue with noise from the react-hot-loader after the first code change following a ``yarn start``.  But it's benign and likely an issue with react-hot-loader, not something we can fix on our end.  So we're going ahead with it and hoping for an update upstream.
Attachment #9021657 - Flags: review?(cdawson) → review+

Comment 26

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

Bug 1364894 - Upgrade from Neutrino 4 to 9 (#4216)

Neutrino controls our frontend linting, transpilation, source-maps,
testing, dev-server and optimisation of production builds.

Highlights of the upgrade are:

* Major version updates to the individual tools within (such as webpack,
  Babel and ESLint), significantly improving performance, fixing
  transpilation/minification correctness bugs, adding support for newer
  ECMAScript features, and increasing linter coverage.
* Hot reloading in the dev server now works for all entry-points and not
  just the jobs view, shortening the feedback cycle.
* Reduced bundle size due to webpack 4's tree shaking, scope hoisting,
  automatic shared/vendor code chunk splitting (no need for the manually
  maintained 'vendor' list).
* CSS is now extracted out of JS, which improves performance, reduces
  bundle size and prevents the initial white flash of un-styled content.
* Support for dynamic imports/code splitting (needed for bug 1502192).
* Support for Jest via a new Jest preset (unblocks bug 1364045).
* Support for public class field declarations (unblocks bug 1480166).
* Improved source-maps (increases the quality of production exception
  trace-backs and fixes several debugger breakpoint bugs).
* Reduced amount of custom configuration required for our fairly complex
  frontend needs, reducing maintenance burden and allowing for easier
  future Neutrino upgrades.

In addition this PR:

* Fixes the WhiteNoise `immutable_file_test()` regex, so that it now
  correctly enables browser caching of images, fonts and source maps.
* Enables webpack-dev-server's overlay feature, which displays any
  compilation errors in the browser, saving having to switch back
  to the console (this can be enabled for warnings too if desired).
* Enables webpack-dev-server's automatic browser-opening feature,
  which saves having to manually navigate to `localhost:5000` after
  running `yarn start`.
* Switches Karma tests to run Firefox in headless mode, reducing the
  workflow disruption when running `yarn test`.
* Uses the new webpack `performance` option to enable maximum asset
  file size thresholds, to help prevent bundle-size regressions.
* Rewrites the `package.json` script commands so that they now work
  correctly on Windows, even when setting environment variables.

Performance comparison:

* Local `yarn build`:
  - Cached: 2m34s -> 23s
  - Uncached: 2m34s -> 58s
* Local `yarn start`:
  - Cached: 34.5s -> 13.6s
  - Uncached: 34.5s -> 31.3s
* Local `yarn test`
  - Cached: 61.5s -> 19.8s
  - Uncached: 61.5s -> 22.0s
* Local `yarn lint`
  - Cached: 3.8s -> 1.8s
  - Uncached: 13.7s -> 13.4s
* Travis end-to-end time:
  9 minutes -> 6 minutes
* Heroku deploy end-to-end time:
  14 minutes -> 9 minutes


3 months ago
Last Resolved: 3 months ago
Resolution: --- → FIXED


2 months ago
Blocks: 1506340
You need to log in before you can comment on or make changes to this bug.