Closed
Bug 1336293
Opened 8 years ago
Closed 8 years ago
The react JS files are served with a short max-age since they do not have a cache-busting filename
Categories
(Tree Management :: Treeherder, defect, P2)
Tree Management
Treeherder
Tracking
(Not tracked)
RESOLVED
DUPLICATE
of bug 1336556
People
(Reporter: emorley, Unassigned)
Details
Aside from the recent bug (bug 1320963), static assets that have a cache-busting filename that includes their hash, get served with a large Cache-Control max-age by Whitenoise.
However the react JS files were excluded from the grunt build concatenation step, so are served separately, as:
https://treeherder.mozilla.org/vendor/react/react.min.js
https://treeherder.mozilla.org/vendor/react/react-dom.min.js
https://treeherder.mozilla.org/vendor/ngReact/ngReact.min.js
...and have a max-age of 60 seconds, rather than 1 year.
Options:
1) Include them in the main build step (after fixing whatever broke when trying before)
2) Rename the files in the repo so that they have hashes in the filename
3) Rename the files in the repo so that they have a version number, and adjust the regex (used to determine suitability for the 1 year max-age) to recognise those in addition to hashed filennames
A disadvantage of (1) is that by bundling everything into the same file, a one-line treeherder JS change means downloading all of the vendored JS files again. Admittedly until we use HTTP2 (not possible on Heroku at present), we still want to limit the number of files. (Perhaps the ideal compromise is to bundle all of the vendored files in one concatenated file, and the Treeherder parts in another?)
Thoughts?
Flags: needinfo?(wlachance)
Flags: needinfo?(cdawson)
Reporter | ||
Comment 1•8 years ago
|
||
Comment 2•8 years ago
|
||
I like your proposal, Ed. Casey is currently working on a PR for replacing Grunt with WebPack. So perhaps there's better minification options there as well. That's bug 1336556.
Flags: needinfo?(cdawson)
Comment 3•8 years ago
|
||
(In reply to Ed Morley [:emorley] from comment #0)
> (Perhaps the ideal compromise is to
> bundle all of the vendored files in one concatenated file, and the
> Treeherder parts in another?)
>
> Thoughts?
I suspect that or HTTP2 is the ideal end state, though you have to bear in mind that JS/HTML asset load times are a *very* small component of overall treeherder load times, which is completely dominated by the bottleneck of loading/rendering jobs last I checked.
To be honest I would be inclined to defer action on this bug for now, unless we can find a fix that's really easy and simple. It's aesthetically annoying to have this standalone file hanging around, but I don't think fixing that would have a large impact on Treeherder's UX. It might go away on its own by moving to webpack or updating a JS dependency.
Flags: needinfo?(wlachance)
Reporter | ||
Comment 4•8 years ago
|
||
Bug 1336556 resolved this by switching to webpack and bundling react properly with everything else. (Plus implemented the "put vendor files in a separate bundle" idea too.)
You need to log in
before you can comment on or make changes to this bug.
Description
•