This repository has been archived by the owner. It is now read-only.
fix(build): Ensure revs/filenames are correctly created. #4210
Conversation
|
@jrgm, @jbuck - I think this'll make you both happy: ➜ scripts git:(issue-4189-fix-the-md5s) md5 4367e7e8.main.js
MD5 (4367e7e8.main.js) = 4367e7e80eb4ac144f82ed9dda86312f
➜ scripts git:(issue-4189-fix-the-md5s) md5 bb8c4e04.head.js
MD5 (bb8c4e04.head.js) = bb8c4e04a4d7bd12925aad4f21624cea
➜ scripts git:(issue-4189-fix-the-md5s) cd ../styles
➜ styles git:(issue-4189-fix-the-md5s) md5 8ed64f83.ar.css
MD5 (8ed64f83.ar.css) = 8ed64f83cfda717bf93cc4368d1cac63
➜ styles git:(issue-4189-fix-the-md5s) md5 abdc5d6d.de.css
MD5 (abdc5d6d.de.css) = abdc5d6d63149a9d4a6f000613897ed9
➜ styles git:(issue-4189-fix-the-md5s) md5 b74c6b69.az.css
MD5 (b74c6b69.az.css) = b74c6b69bd2d38e6b81cdaf7a1d59bb2 |
shane-tomlinson
force-pushed the
issue-4189-fix-the-md5s
branch
from
243b392
to
585f8f3
added 2 commits
Sep 23, 2016
No need to copy over jquery-ui, fxa-checkbox, and styles/fonts to dist. These JS resources are included into the built bundle and not loaded dynamically. There is no need to copy these over. Remove jquery-ui from the rev task. issue #4177
The main CSS and JS bundle reference URLs to other static resources that are themselves revved. We erroneously revved the main CSS and JS bundle before updating the referenced URLs with the revved variants and updating the JS bundle with SRI hashes. This led to files across two trains having different content but the same filename. Instead, we do a two phase approach to revving/SRI hash generation: 1) rev all the leaf node resources. Update references in the main JS and CSS bundle. Generate SRI hashes for revved JS. Insert SRI values into main JS bundle. 2) rev the resources with dependencies. Generate SRI hash for the main JS bundle. Update URL references in the HTML. Update SRI values in the HTML. fixes #4189
shane-tomlinson
force-pushed the
issue-4189-fix-the-md5s
branch
from
585f8f3
to
f5aa7bc
|
@jrgm, @jbuck, @vladikoff - r? |
|
I compared train-69-dist currently on prod with this patch and got the following results:
|
| // different revs of the file with different contents | ||
| // can have the same name because the rev was created before | ||
| // internal URLs were updated. | ||
| with_children: { //eslint-disable-line camelcase |
vladikoff
Sep 30, 2016
Contributor
👍
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Also contains #4186.
The main CSS and JS bundle reference URLs to other static resources that
are themselves revved. We erroneously revved the main CSS and JS bundle
before updating the referenced URLs with the revved variants and
updating the JS bundle with SRI hashes. This led to files across two
trains having different content but the same filename.
Instead, we do a two phase approach to revving/SRI hash generation:
CSS bundle. Generate SRI hashes for revved JS. Insert SRI values into
main JS bundle.
JS bundle. Update URL references in the HTML. Update SRI values in
the HTML.
fixes #4189