Closed Bug 1336293 Opened 7 years ago Closed 7 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)

defect

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)
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)
See Also: → 1336556
(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)
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.)
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → DUPLICATE
See Also: 1336556
You need to log in before you can comment on or make changes to this bug.