Put hashes in asset filenames, not querystrings

gulpfile.js

@@ -14,6 +14,8 @@ plugins.sass = require('gulp-sass');
 plugins.gulpStylelint = require('gulp-stylelint');
 plugins.gulpif = require('gulp-if');
 plugins.postcss = require('gulp-postcss');
+plugins.hash = require('gulp-sha256-filename');
+
 
 const paths = {
   src: 'src/assets/',
@@ -23,115 +25,83 @@ const paths = {
   assetsUrl: '/alerts/assets/'
 };
 
+const hashOptions = { format: '{name}-{hash:8}{ext}' }
+
 plugins.sass.compiler = require('sass');
 
 const copy = {
   govuk_frontend: {
     fonts: () => {
       return src(paths.govuk_frontend + 'govuk/assets/fonts/**/*')
         .pipe(dest(paths.dist + 'fonts/'));
+        // Fonts have their own filename hash so we don’t need to add
+        // our own
     },
     images: () => {
       return src(paths.govuk_frontend + 'govuk/assets/images/**/*')
+        // A few images used by GOVUK Frontend components can't have hashes added to their
+        // URLs so we can't change their filenames
+        .pipe(plugins.gulpif(
+          (file) => {
+            return [
+              'govuk-crest-2x.png',
+              'govuk-crest.png',
+              'govuk-logotype-crown.png'
+            ].includes(path.basename(file.path)) === false;
+          },
+          plugins.hash(hashOptions)
+        ))
         .pipe(dest(paths.dist + 'images/'));
     }
   },
   html5shiv: () => {
     return src(paths.node_modules + 'html5shiv/dist/*.min.js')
+      .pipe(plugins.hash(hashOptions))
       .pipe(dest(paths.dist + 'javascripts/vendor/html5shiv/'));
   },
   images: () => {
       return src(paths.src + 'images/**/*')
+        .pipe(plugins.hash(hashOptions))
         .pipe(dest(paths.dist + 'images/'));
   }
 };
 
-const javascripts = {
-  details: () => {
-    // Use Rollup to combine all JS in JS module format into a Immediately Invoked Function
-    // Expression (IIFE) to:
-    // - deliver it in one bundle
-    // - allow it to run in browsers without support for JS Modules
-    return rollup.rollup({
-      input: paths.src + 'javascripts/govuk-frontend-details-init.mjs',
-      plugins: [
-        // determine module entry points from either 'module' or 'main' fields in package.json
-        rollupPluginNodeResolve.nodeResolve({
-          mainFields: ['module', 'main']
-        }),
-        // gulp rollup runs on nodeJS so reads modules in commonJS format
-        // this adds node_modules to the require path so it can find the GOVUK Frontend modules
-        rollupPluginCommonjs({
-          include: 'node_modules/**'
-        }),
-        // Terser is a replacement for UglifyJS
-        rollupPluginTerser()
-      ]
-    }).then(bundle => {
-      return bundle.write({
-        file: paths.dist + 'javascripts/govuk-frontend-details.js',
-        format: 'iife',
-        name: 'GOVUK',
-        sourcemap: true
-      });
-    });
-  },
-  sharingButton: () => {
-    // Use Rollup to combine all JS in JS module format into a Immediately Invoked Function
-    // Expression (IIFE) to:
-    // - deliver it in one bundle
-    // - allow it to run in browsers without support for JS Modules
-    return rollup.rollup({
-      input: paths.src + 'javascripts/sharing-button-init.mjs',
-      plugins: [
-        // determine module entry points from either 'module' or 'main' fields in package.json
-        rollupPluginNodeResolve.nodeResolve({
-          mainFields: ['module', 'main']
-        }),
-        // gulp rollup runs on nodeJS so reads modules in commonJS format
-        // this adds node_modules to the require path so it can find the GOVUK Frontend modules
-        rollupPluginCommonjs({
-          include: 'node_modules/**'
-        }),
-        // Terser is a replacement for UglifyJS
-        rollupPluginTerser()
-      ]
-    }).then(bundle => {
-      return bundle.write({
-        file: paths.dist + 'javascripts/sharing-button.js',
-        format: 'iife',
-        name: 'GOVUK',
-        sourcemap: true
-      });
-    });
-  },
-  relativeDates: () => {
-    // Use Rollup to combine all JS in JS module format into a Immediately Invoked Function
-    // Expression (IIFE) to:
-    // - deliver it in one bundle
-    // - allow it to run in browsers without support for JS Modules
-    return rollup.rollup({
-      input: paths.src + 'javascripts/relative-dates-init.mjs',
-      plugins: [
-        // determine module entry points from either 'module' or 'main' fields in package.json
-        rollupPluginNodeResolve.nodeResolve({
-          mainFields: ['module', 'main']
-        }),
-        // gulp rollup runs on nodeJS so reads modules in commonJS format
-        // this adds node_modules to the require path so it can find the GOVUK Frontend modules
-        rollupPluginCommonjs({
-          include: 'node_modules/**'
-        })
-      ]
-    }).then(bundle => {
-      return bundle.write({
-        file: paths.dist + 'javascripts/relative-dates.js',
-        format: 'iife',
-        name: 'GOVUK',
-        sourcemap: true
-      });
+const rollupTask = (fileName) => () => {
+  return rollup.rollup({
+  // Use Rollup to combine all JS in JS module format into a Immediately Invoked Function
+  // Expression (IIFE) to:
+  // - deliver it in one bundle
+  // - allow it to run in browsers without support for JS Modules
+    input: {
+      [fileName]: paths.src + 'javascripts/' + fileName + '-init.mjs'
+    },
+    plugins: [
+      // determine module entry points from either 'module' or 'main' fields in package.json
+      rollupPluginNodeResolve.nodeResolve({
+        mainFields: ['module', 'main']
+      }),
+      // gulp rollup runs on nodeJS so reads modules in commonJS format
+      // this adds node_modules to the require path so it can find the GOVUK Frontend modules
+      rollupPluginCommonjs({
+        include: 'node_modules/**'
+      }),
+      // Terser is a replacement for UglifyJS
+      rollupPluginTerser()
+    ]
+  }).then(bundle => {
+    return bundle.write({
+      dir: paths.dist + 'javascripts/',
+      // [hash] here is the first 8 characters of a SHA256 sum of the
+      // file’s contents, as per
+      // https://github.com/rollup/rollup/blob/72125168ec6798b919a931742d2def5f4e69093b/src/utils/FileEmitter.ts#L52
+      // and
+      // https://github.com/rollup/rollup/blob/ce2592df6b369b68d61f58c2ef8bf4695421146a/browser/crypto.ts#L3-L6
+      entryFileNames: '[name]-[hash].js',
+      format: 'iife',
+      name: 'GOVUK',
+      sourcemap: true
     });
-  }
+  });
 };
 
 const scss = {
@@ -150,6 +120,7 @@ const scss = {
           },
           plugins.postcss([require('oldie')()])
         ))
+      .pipe(plugins.hash(hashOptions))
       .pipe(dest(paths.dist + 'stylesheets/'));
   }
 };
@@ -160,9 +131,9 @@ const defaultTask = parallel(
   copy.html5shiv,
   copy.images,
   scss.compile,
-  javascripts.details,
-  javascripts.sharingButton,
-  javascripts.relativeDates
+  rollupTask('govuk-frontend-details'),
+  rollupTask('sharing-button'),
+  rollupTask('relative-dates')
 );
 
 exports.default = defaultTask;

lib/utils.py

@@ -1,4 +1,5 @@
-import hashlib
+import os
+import re
 from pathlib import Path
 
 from jinja2 import Markup, escape
@@ -19,5 +20,14 @@ def paragraphize(value, classes="govuk-body-l govuk-!-margin-bottom-4"):
 
 
 def file_fingerprint(path, root=DIST):
-    contents = open(str(root) + path, 'rb').read()
-    return path + '?' + hashlib.md5(contents).hexdigest()
+    path = Path(path).relative_to('/')  # path comes in as absolute, rooted to the dist folder
+    path_regex = re.compile(f'^{path.stem}-[0-9a-z]{{8}}{path.suffix}$')  # regexp based on the filename + a 8 char hash
+    matches = [
+                filename for filename
+                in os.listdir(str(root / path.parent))
+                if path_regex.search(filename)]
+
+    if len(matches) == 0:
+        raise OSError(f'{str(root / path.parent / path.stem)}-[hash]{path.suffix} referenced but not available')
+
+    return f'/{path.parent}/{matches[0]}'

package.json

@@ -33,6 +33,7 @@
     "gulp-if": "^3.0.0",
     "gulp-postcss": "^9.0.0",
     "gulp-sass": "^4.1.0",
+    "gulp-sha256-filename": "^2.0.0",
     "gulp-stylelint": "^13.0.0",
     "jest": "^26.6.3",
     "oldie": "^1.3.0",

templates/_layout.html

@@ -21,17 +21,17 @@
   {%- endfor %}
   {# prefetch block for any files needed in future navigations - doesn't block resources for the current page #}
   {% block prefetch %}{% endblock %}
-  <link media="print" href={{ "/alerts/assets/stylesheets/main-print.css" | file_fingerprint }} rel="stylesheet">
+  <link media="print" href="{{ '/alerts/assets/stylesheets/main-print.css' | file_fingerprint }}" rel="stylesheet">
   <!--[if !IE 8]><!-->
-  <link media="screen" href={{ "/alerts/assets/stylesheets/main.css" | file_fingerprint }} rel="stylesheet">
+  <link media="screen" href="{{ '/alerts/assets/stylesheets/main.css' | file_fingerprint }}" rel="stylesheet">
   <!--<![endif]-->
   <!--[if IE 8]>
-  <link media="screen" href={{ "/alerts/assets/stylesheets/main-ie8.css" | file_fingerprint }} rel="stylesheet">
+  <link media="screen" href="{{ '/alerts/assets/stylesheets/main-ie8.css' | file_fingerprint }}" rel="stylesheet">
   <![endif]-->
 
   {# For older browsers to allow them to recognise HTML5 elements such as `<header>` #}
   <!--[if lt IE 9]>
-    <script src="/alerts/assets/javascripts/vendor/html5shiv/html5shiv.min.js"></script>
+    <script src="{{ '/alerts/assets/javascripts/vendor/html5shiv/html5shiv.min.js' | file_fingerprint }}"></script>
   <![endif]-->
 {% endblock %}
 

templates/components/alerts_icon.html

@@ -1,10 +1,10 @@
 {% macro alerts_icon(height, x=0, y=0, degrade=True, alert_active=True) %}
   {% if alert_active %}
     {% set fill = "#de1924" %}
-    {% set fallback = "alert.png" %}
+    {% set fallback = "/alerts/assets/images/alert.png" %}
   {% else %}
     {% set fill = "#b1b4b6" %}
-    {% set fallback = "alert-grey.png" %}
+    {% set fallback = "/alerts/assets/images/alert-grey.png" %}
   {% endif %}
 
   {% set width = (height / 100 * 106)|round(0, 'floor') %}
@@ -16,7 +16,7 @@
   {% if degrade %}
     <!--<![endif]-->
     <!--[if IE 8]>
-      <img src="/alerts/assets/images/{{ fallback }}" height="{{ height }}" width="{{ width }}", alt="{{ alt }}" class="alerts-icon alerts-icon--{{ height }}" />
+      <img src="{{ fallback | file_fingerprint }}" height="{{ height }}" width="{{ width }}", alt="{{ alt }}" class="alerts-icon alerts-icon--{{ height }}" />
     <![endif]-->
   {% endif %}
 {% endmacro %}

templates/components/example.html

@@ -41,6 +41,6 @@
       </g>
     </svg>
     <!--<![endif]-->
-    <!--[if IE 8]><img src="/alerts/assets/images/example.png" height="436" width="306", alt="{{ alt }}" class="example alerts-image__image--centred" /><![endif]-->
+    <!--[if IE 8]><img src="{{ '/alerts/assets/images/example.png' | file_fingerprint }}" height="436" width="306", alt="{{ alt }}" class="example alerts-image__image--centred" /><![endif]-->
   </div>
 {% endmacro %}

tests/lib/test_utils.py

@@ -1,13 +1,19 @@
 from pathlib import Path
 
+import pytest
 from jinja2 import Markup
 
 from lib.utils import file_fingerprint, paragraphize
 
 
-def test_file_fingerprint_adds_hash_to_file_path():
+def test_file_fingerprint_gets_variant_of_path_with_hash_in():
     new_path = file_fingerprint('/tests/test_files/example.txt', root=Path('.'))
-    assert new_path == '/tests/test_files/example.txt?4d93d51945b88325c213640ef59fc50b'
+    assert new_path == '/tests/test_files/example-4d93d519.txt'
+
+
+def test_file_fingerprint_raises_for_file_not_found():
+    with pytest.raises(OSError):
+        file_fingerprint('/tests/test_files/doesnt-exist.txt', root=Path('.'))
 
 
 def test_paragraphize_converts_newlines_to_paragraphs():