From 622caa057a2ca1428eb398b01f0d2387ce37ee8d Mon Sep 17 00:00:00 2001 From: Sam Thorogood Date: Tue, 7 Jul 2020 11:39:09 +1000 Subject: [PATCH] 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 | 21 +++-------- build-sw.js | 19 ++++++++-- build.js | 9 ++++- compile-css.js | 14 +++---- server.js | 36 ++++-------------- src/build/hash.js | 34 ++++++++++++++--- src/build/resource-path.js | 41 +++++++++++++++++++++ src/lib/sw.js | 8 +++- src/site/_filters/strip-query-params-dev.js | 23 ++++++++++++ src/site/_includes/layout.njk | 4 +- test/integration/build-test.js | 25 +++++-------- 11 files changed, 152 insertions(+), 82 deletions(-) create mode 100644 src/build/resource-path.js create mode 100644 src/site/_filters/strip-query-params-dev.js diff --git a/.eleventy.js b/.eleventy.js index 02d24f41b95..8c19bc94b6b 100644 --- a/.eleventy.js +++ b/.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'); + }); } // ---------------------------------------------------------------------------- diff --git a/build-sw.js b/build-sw.js index 825bbbf0709..fd568a66310 100644 --- a/build-sw.js +++ b/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,7 +41,7 @@ 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', @@ -48,15 +49,25 @@ async function buildCacheManifest() { // 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}`); } diff --git a/build.js b/build.js index b6322473e6a..8f49d6f5bd0 100644 --- a/build.js +++ b/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; } diff --git a/compile-css.js b/compile-css.js index eb77ccce999..dbf9f962479 100644 --- a/compile-css.js +++ b/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})`); diff --git a/server.js b/server.js index 4212d8e6724..b2bcc7ca7ed 100644 --- a/server.js +++ b/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,12 +61,14 @@ 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}; } @@ -75,38 +76,17 @@ function buildSafetyAssetHandler() { 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(); diff --git a/src/build/hash.js b/src/build/hash.js index 16d678a13c7..92513f54808 100644 --- a/src/build/hash.js +++ b/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, }; diff --git a/src/build/resource-path.js b/src/build/resource-path.js new file mode 100644 index 00000000000..34e32821060 --- /dev/null +++ b/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; diff --git a/src/lib/sw.js b/src/lib/sw.js index 62fbe5e58d8..219d154758e 100644 --- a/src/lib/sw.js +++ b/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. diff --git a/src/site/_filters/strip-query-params-dev.js b/src/site/_filters/strip-query-params-dev.js new file mode 100644 index 00000000000..fb16fe8b13d --- /dev/null +++ b/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; diff --git a/src/site/_includes/layout.njk b/src/site/_includes/layout.njk index a2c45b46659..05f377e909d 100644 --- a/src/site/_includes/layout.njk +++ b/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" /> - + {# Include default icon even though we have a manifest #} @@ -45,7 +45,7 @@ tags: [] - + {# diff --git a/test/integration/build-test.js b/test/integration/build-test.js index e27f8ba3755..e00c0c47914 100644 --- a/test/integration/build-test.js +++ b/test/integration/build-test.js @@ -30,6 +30,8 @@ describe('Build test', function () { path.join('images', 'favicon.ico'), path.join('images', 'lockup.svg'), '_redirects.yaml', + 'app.css', + 'bootstrap.js', 'manifest.webmanifest', 'nuke-sw.js', 'robots.txt', @@ -47,22 +49,13 @@ describe('Build test', function () { // Check that there's a Rollup-generated file with the given name that looks // like `[name]-[hash].js`. - ['bootstrap', 'app', 'measure', 'newsletter', 'default'].forEach( - (chunked) => { - const re = new RegExp(`^${chunked}-\\w+\\.js$`); - assert( - contents.find((file) => re.test(file)), - `Could not find Rollup output: ${chunked}`, - ); - }, - ); - - // Check that there's an "app-[hash].css" file. - const re = new RegExp(`^app-\\w+\\.css$`); - assert( - contents.find((file) => re.test(file)), - `Could not find Sass output: app`, - ); + ['app', 'measure', 'newsletter', 'default'].forEach((chunked) => { + const re = new RegExp(`^${chunked}-\\w+\\.js$`); + assert( + contents.find((file) => re.test(file)), + `Could not find Rollup output: ${chunked}`, + ); + }); // Check that there's NOT a web.dev/LIVE partial. We confirm that partials // are generally created above, in the list of common checks.