use bootstrap.js/app.css again (#3395)

* use bootstrap.js/app.css again

* include hashed sw assets

* add helper for fetching resource version

* unneeded dep

.eleventy.js

@@ -17,12 +17,11 @@
 const pluginRss = require('@11ty/eleventy-plugin-rss');
 const pluginSyntaxHighlight = require('@11ty/eleventy-plugin-syntaxhighlight');
 
+const resourcePath = require('./src/build/resource-path');
 const markdownIt = require('markdown-it');
 const markdownItAnchor = require('markdown-it-anchor');
 const markdownItAttrs = require('markdown-it-attrs');
 const slugify = require('slugify');
-const fs = require('fs');
-const path = require('path');
 
 const componentsDir = 'src/site/_includes/components';
 const ArticleNavigation = require(`./${componentsDir}/ArticleNavigation`);
@@ -79,6 +78,7 @@ const removeDrafts = require(`./${filtersDir}/remove-drafts`);
 const strip = require(`./${filtersDir}/strip`);
 const stripBlog = require(`./${filtersDir}/strip-blog`);
 const stripLanguage = require(`./${filtersDir}/strip-language`);
+const stripQueryParamsDev = require(`./${filtersDir}/strip-query-params-dev`);
 const getPaths = require(`./${filtersDir}/get-paths`);
 
 const transformsDir = 'src/site/_transforms';
@@ -190,6 +190,7 @@ module.exports = function (config) {
   config.addFilter('removeDrafts', removeDrafts);
   config.addFilter('stripBlog', stripBlog);
   config.addFilter('stripLanguage', stripLanguage);
+  config.addFilter('stripQueryParamsDev', stripQueryParamsDev);
   config.addFilter('getPaths', getPaths);
   config.addFilter('strip', strip);
 
@@ -247,25 +248,15 @@ module.exports = function (config) {
   if (isProd) {
     // We generate the paths to our JS and CSS entrypoints as a side-effect
     // of their build scripts, so make sure they exist in prod builds.
-    const checkJSONDataPath = (name) => {
-      const f = `src/site/_data/${name}.json`;
+    ['css', 'js'].forEach((name) => {
       try {
-        const raw = JSON.parse(fs.readFileSync(f), 'utf-8');
-        if (!raw['path']) {
-          throw new Error(`could not find 'path' key in: ${f}`);
-        }
-        const check = path.join('dist', raw['path']);
-        if (!fs.existsSync(check)) {
-          throw new Error(`path did not exist: ${check}`);
-        }
+        resourcePath(name);
       } catch (e) {
         throw new Error(
           `could not find valid JSON path inside src/site/_data/: ${name} (${e})`,
         );
       }
-    };
-    checkJSONDataPath('resourceCSS');
-    checkJSONDataPath('resourceJS');
+    });
   }
 
   // ----------------------------------------------------------------------------

build-sw.js

@@ -23,6 +23,7 @@ const rollupPluginReplace = require('rollup-plugin-replace');
 const OMT = require('@surma/rollup-plugin-off-main-thread');
 const rollup = require('rollup');
 const {getManifest} = require('workbox-build');
+const resourcePath = require('./src/build/resource-path');
 const buildVirtualJSON = require('./src/build/virtual-json');
 const minifySource = require('./src/build/minify-js');
 
@@ -40,23 +41,33 @@ const {buildDefaultPlugins, disallowExternal} = require('./src/build/common');
  * before the Rollup build script.
  */
 async function buildCacheManifest() {
-  const toplevelManifest = await getManifest({
+  const config = {
     // JS or CSS files that include hashes don't need their own revision fields.
     dontCacheBustURLsMatching: /-[0-9a-f]{8}\.(css|js)/,
     globDirectory: 'dist',
     globPatterns: [
       // We don't include jpg files, as they're used for authors and hero
       // images, which are part of articles, and not the top-level site.
       'images/**/*.{png,svg}',
-      '*.css',
-      '*.js',
+      '*-*.js',
       'sw-partial-layout.partial',
     ],
     globIgnores: [
       // This removes large shared PNG files that are used only for articles.
       'images/{shared}/**',
     ],
-  });
+  };
+  if (isProd) {
+    config.additionalManifestEntries = [
+      {url: resourcePath('js'), revision: null},
+      {url: resourcePath('css'), revision: null},
+    ];
+  } else {
+    // Don't use hash revisions in dev, or even check that the files exist.
+    config.globPatterns.push('bootstrap.js', 'app.css');
+  }
+
+  const toplevelManifest = await getManifest(config);
   if (toplevelManifest.warnings.length) {
     throw new Error(`toplevel manifest: ${toplevelManifest.warnings}`);
   }

build.js

@@ -28,6 +28,7 @@ const rollupPluginIstanbul = require('rollup-plugin-istanbul');
 const rollup = require('rollup');
 const buildVirtualJSON = require('./src/build/virtual-json');
 const minifySource = require('./src/build/minify-js');
+const {hashForFiles} = require('./src/build/hash');
 
 process.on('unhandledRejection', (reason, p) => {
   log.error('Build had unhandled rejection', reason, p);
@@ -127,7 +128,7 @@ async function build() {
     },
   });
   const bootstrapGenerated = await bootstrapBundle.write({
-    entryFileNames: '[name]-[hash].js',
+    entryFileNames: '[name].js',
     sourcemap: true,
     dir: 'dist',
     format: 'iife',
@@ -140,10 +141,13 @@ async function build() {
   const bootstrapPath = bootstrapGenerated.output[0].fileName;
   outputFiles.push(bootstrapPath);
 
+  const hash = hashForFiles(path.join('dist', bootstrapPath));
+  const resourceName = `${bootstrapPath}?v=${hash}`;
+
   // Write the bundle entrypoint to a known file for Eleventy to read.
   await fs.writeFile(
     'src/site/_data/resourceJS.json',
-    JSON.stringify({path: '/' + bootstrapPath}),
+    JSON.stringify({path: `/${resourceName}`}),
   );
 
   // Compress the generated source here, as we need the final files and hashes for the Service
@@ -152,6 +156,7 @@ async function build() {
     const ratio = await minifySource(outputFiles);
     log(`Minified site code is ${(ratio * 100).toFixed(2)}% of source`);
   }
+  log(`Finished JS! (${resourceName})`);
 
   return outputFiles.length;
 }

compile-css.js

@@ -121,17 +121,15 @@ function renderTo(result, fileName) {
 const out = compileCSS('src/styles/all.scss');
 const hash = hashForContent(out.css);
 
-// We write an unhashed CSS file (as well as the hashed one) for two reasons:
-//  - to not force an Eleventy rebuild during dev
-//  - to work around #3363 (prod clients loaded before 2020-06-16 will see an unstyled page flash)
+// We write an unhashed CSS file due to unfortunate real-world caching problems with a hash inside
+// the CSS name (we see our old HTML cached longer than the assets are available).
 renderTo(out, `dist/app.css`);
-renderTo(out, `dist/app-${hash}.css`);
 
-// Write the CSS entrypoint to a known file for Eleventy to read. In dev, use the unhashed version.
-const resourcePath = isProd ? `/app-${hash}.css` : '/app.css';
+// Write the CSS entrypoint to a known file, with a query hash, for Eleventy to read.
+const resourceName = `app.css?v=${hash}`;
 fs.writeFileSync(
   'src/site/_data/resourceCSS.json',
-  JSON.stringify({path: resourcePath}),
+  JSON.stringify({path: `/${resourceName}`}),
 );
 
-log(`Finished CSS! (app-${hash}.css)`);
+log(`Finished CSS! (${resourceName})`);

server.js

@@ -16,7 +16,6 @@
 
 const isGAEProd = Boolean(process.env.GAE_APPLICATION);
 
-const fs = require('fs');
 const compression = require('compression');
 const express = require('express');
 const cookieParser = require('cookie-parser');
@@ -62,51 +61,32 @@ const notFoundHandler = (req, res, next) => {
 // returns the current live asset. This applies to both "app.css" and "bootstrap.js".
 function buildSafetyAssetHandler() {
   const hashedAssetMatch = /^(\w+)(?:|-\w+)\.(\w+)(?:\?.*|)$/;
+
+  // Matches URLs like "/foo-hash.css" or "/blah.suffix", including an optional query param suffix.
+  // Just returns two groups: "foo" and "suffix".
   const runHashedAssetMatch = (cand) => {
     if (cand.startsWith('/')) {
       cand = cand.substr(1);
     }
     const m = hashedAssetMatch.exec(cand);
-    console.warn('got match', m, 'for', cand);
     if (!m) {
       return {base: null, ext: null};
     }
     const [base, ext] = m.slice(1, 3);
     return {base, ext};
   };
 
-  // On build, find the relevant bootstrap/app assets. We presume that in prod, only the correct
-  // files will exist. In dev, it's trivially possible for lots of files to be in /dist/, including
-  // older assets.
-  // TODO(samthor): does gcloud upload the files "src/site/_data/resourceCSS.json"? If so, we could
-  // just pull direct from there, or some other generated Eleventy file.
-  const files = fs.readdirSync('dist/');
-  const findHashedAsset = (base, ext) => {
-    for (const f of files) {
-      const {base: checkBase, ext: checkExt} = runHashedAssetMatch(f);
-      if (base === checkBase && ext === checkExt) {
-        return f;
-      }
-    }
-    return null;
-  };
-  const bootstrapJSAsset = findHashedAsset('bootstrap', 'js');
-  const appCSSAsset = findHashedAsset('app', 'css');
-  console.info(
-    `Server found bootstrap.js=${bootstrapJSAsset} app.css=${appCSSAsset}`,
-  );
-
-  // Matches requests like "/foo-hash.css" or "/blah.ext". Just returns two groups: "foo" and "ext".
   return (req, res, next) => {
     const {base, ext} = runHashedAssetMatch(req.url);
     if (!base) {
       return next();
     }
 
-    if (bootstrapJSAsset && base === 'bootstrap' && ext === 'js') {
-      req.url = '/' + bootstrapJSAsset;
-    } else if (appCSSAsset && base === 'app' && ext === 'css') {
-      req.url = '/' + appCSSAsset;
+    // We don't hash these assets in the upload, so just use them directly.
+    if (base === 'bootstrap' && ext === 'js') {
+      req.url = '/bootstrap.js';
+    } else if (base === 'app' && ext === 'css') {
+      req.url = '/app.css';
     }
 
     return next();

src/build/hash.js

@@ -3,26 +3,50 @@
  */
 
 const crypto = require('crypto');
+const fs = require('fs');
 
 const hashLength = 8;
 
+function generateAndValidateHash(c) {
+  const hash = c.digest('hex').substr(0, hashLength);
+  if (hash.length !== hashLength) {
+    throw new TypeError(`could not hash content`);
+  }
+  return hash;
+}
+
 /**
  * Hashes the passed content.
  *
- * @param {string} files to hash
+ * @param {string} contents to hash
  * @return {string}
  */
 function hashForContent(contents) {
   const c = crypto.createHash('sha1');
   c.update(contents);
+  return generateAndValidateHash(c);
+}
 
-  const hash = c.digest('hex').substr(0, hashLength);
-  if (hash.length !== hashLength) {
-    throw new TypeError(`could not hash content`);
+/**
+ * Hashes the passed files. Requires at least one.
+ *
+ * @param {string} file base file to hash
+ * @param {...string} rest additional files to hash
+ */
+function hashForFiles(file, ...rest) {
+  const files = [file].concat(rest);
+
+  const c = crypto.createHash('sha1');
+
+  for (const file of files) {
+    const b = fs.readFileSync(file);
+    c.update(b);
   }
-  return hash;
+
+  return generateAndValidateHash(c);
 }
 
 module.exports = {
   hashForContent,
+  hashForFiles,
 };

src/build/resource-path.js

@@ -0,0 +1,41 @@
+/**
+ * @fileoverview Helper that reads and checks the resource path
+ */
+
+const fs = require('fs');
+const path = require('path');
+
+/**
+ * Finds the resource path for the given name, and confirms that the file exists (throwing an
+ * Error otherwise). Returns the path including any query parameters, but without any leading "/",
+ * for use in caching.
+ *
+ * @param {string} name
+ * @return {string}
+ */
+function resourcePath(name) {
+  name = name.toUpperCase();
+
+  const f = path.join(
+    __dirname,
+    '../../src/site/_data',
+    `resource${name}.json`,
+  );
+
+  const raw = JSON.parse(fs.readFileSync(f), 'utf-8');
+
+  const {path: rawPath} = raw;
+  if (rawPath === undefined) {
+    throw new Error(`could not find 'path' key in: ${f}`);
+  }
+
+  const cleanPath = rawPath.split('?')[0]; // remove trailing query params
+  const check = path.join(__dirname, '../../dist', cleanPath);
+  if (!fs.existsSync(check)) {
+    throw new Error(`path did not exist: ${check}`);
+  }
+
+  return rawPath[0] === '/' ? rawPath.substr(1) : rawPath;
+}
+
+module.exports = resourcePath;

src/lib/sw.js

@@ -16,12 +16,16 @@ import {matchSameOriginRegExp} from './utils/sw-match.js';
 const cacheNames = {webdevCore: 'webdev-core', ...workboxCacheNames};
 
 /**
- * Configure default cache for some common web.dev assets: images, CSS, JS, partial template.
+ * Configure default cache for some common web.dev assets: images, CSS, JS, partial template. This
+ * doesn't match general HTML or other files, so we disable directoryIndex and cleanURLs.
  *
  * This must occur first, as we cache images that are also matched by runtime handlers below. See
  * this workbox issue for updates: https://github.com/GoogleChrome/workbox/issues/2402
  */
-precacheAndRoute(manifest);
+precacheAndRoute(manifest, {
+  cleanURLs: false,
+  directoryIndex: '',
+});
 
 // Architecture revision of the Service Worker. If the previously saved revision doesn't match,
 // then this will cause clients to be aggressively claimed and reloaded on install/activate.

src/site/_filters/strip-query-params-dev.js

@@ -0,0 +1,23 @@
+/*
+ * Copyright 2020 Google LLC
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+const isProd = process.env.ELEVENTY_ENV === 'prod';
+const noop = (url) => url;
+const strip = (url) => url.split('?')[0];
+
+// Strip query params from URLs in dev only. Used for lazy cache-busting
+// e.g. in non-prod, app.css?blah => app.css
+module.exports = isProd ? noop : strip;

src/site/_includes/layout.njk

@@ -36,7 +36,7 @@ tags: []
       rel="stylesheet"
       href="//fonts.googleapis.com/css?family=Google+Sans:400,500|Roboto:400,400italic,500,500italic|Roboto+Condensed:400,700|Roboto+Mono:400,500|Material+Icons"
     />
-    <link rel="stylesheet" href="{{resourceCSS.path}}" />
+    <link rel="stylesheet" href="{{ resourceCSS.path | stripQueryParamsDev }}" />
     <link rel="manifest" href="/manifest.webmanifest" />
     {# Include default icon even though we have a manifest #}
     <link rel="shortcut icon" href="/images/favicon.ico">
@@ -45,7 +45,7 @@ tags: []
     <link rel="apple-touch-icon" sizes="180x180" href="/images/apple-touch-icon.png">
     <link rel="mask-icon" color="#0054ff" href="/images/safari-pinned-tab.svg">
     <script async src="//www.google-analytics.com/analytics.js"></script>
-    <script defer src="{{resourceJS.path}}"></script>
+    <script defer src="{{ resourceJS.path | stripQueryParamsDev }}"></script>
   </head>
   <body class="unresolved">
     {#