From 76fa1edb69af12853743f04488b2b925c058c328 Mon Sep 17 00:00:00 2001 From: Devon Govett Date: Tue, 20 Mar 2018 19:46:15 -0700 Subject: [PATCH] Content hash output file names (#1025) --- src/Asset.js | 26 ++---------- src/Bundle.js | 54 ++++++++++++++++++++++++ src/Bundler.js | 22 +++++++--- src/Logger.js | 6 ++- src/Server.js | 5 ++- src/assets/HTMLAsset.js | 26 +++++++++--- src/assets/RawAsset.js | 5 +++ src/packagers/CSSPackager.js | 2 +- src/packagers/HTMLPackager.js | 2 +- src/packagers/JSPackager.js | 8 ++-- src/packagers/Packager.js | 23 +++++++++- src/packagers/RawPackager.js | 13 +----- src/packagers/SourceMapPackager.js | 2 +- src/utils/md5.js | 13 ++++++ src/visitors/dependencies.js | 16 +++---- test/contentHashing.js | 67 ++++++++++++++++++++++++++++++ test/css.js | 8 ++-- test/html.js | 10 +++-- test/javascript.js | 2 +- test/less.js | 4 +- test/logger.js | 2 +- test/sass.js | 4 +- test/stylus.js | 4 +- test/typescript.js | 2 +- 24 files changed, 244 insertions(+), 82 deletions(-) create mode 100644 test/contentHashing.js diff --git a/src/Asset.js b/src/Asset.js index f88d6906235..96de2a78d70 100644 --- a/src/Asset.js +++ b/src/Asset.js @@ -148,7 +148,7 @@ class Asset { await this.getDependencies(); await this.transform(); this.generated = await this.generate(); - this.hash = this.generateHash(); + this.hash = await this.generateHash(); } return this.generated; @@ -175,27 +175,9 @@ class Asset { } generateBundleName() { - const ext = '.' + this.type; - - const isEntryPoint = this.name === this.options.mainFile; - - // If this is the entry point of the root bundle, use outFile filename if provided - if (isEntryPoint && this.options.outFile) { - return ( - path.basename( - this.options.outFile, - path.extname(this.options.outFile) - ) + ext - ); - } - - // If this is the entry point of the root bundle, use the original filename - if (isEntryPoint) { - return path.basename(this.name, path.extname(this.name)) + ext; - } - - // Otherwise generate a unique name - return md5(this.name) + ext; + // Generate a unique name. This will be replaced with a nicer + // name later as part of content hashing. + return md5(this.name) + '.' + this.type; } generateErrorMessage(err) { diff --git a/src/Bundle.js b/src/Bundle.js index 7073b3f94c9..a14a91bd0af 100644 --- a/src/Bundle.js +++ b/src/Bundle.js @@ -91,6 +91,59 @@ class Bundle { return this.assets.size === 0; } + getBundleNameMap(contentHash, hashes = new Map()) { + let hashedName = this.getHashedBundleName(contentHash); + hashes.set(Path.basename(this.name), hashedName); + this.name = Path.join(Path.dirname(this.name), hashedName); + + for (let child of this.childBundles.values()) { + child.getBundleNameMap(contentHash, hashes); + } + + return hashes; + } + + getHashedBundleName(contentHash) { + // If content hashing is enabled, generate a hash from all assets in the bundle. + // Otherwise, use a hash of the filename so it remains consistent across builds. + let ext = Path.extname(this.name); + let hash = (contentHash + ? this.getHash() + : Path.basename(this.name, ext) + ).slice(-8); + let entryAsset = this.entryAsset || this.parentBundle.entryAsset; + let name = Path.basename(entryAsset.name, Path.extname(entryAsset.name)); + let isMainEntry = entryAsset.name === entryAsset.options.mainFile; + let isEntry = + isMainEntry || Array.from(entryAsset.parentDeps).some(dep => dep.entry); + + // If this is the main entry file, use the output file option as the name if provided. + if (isMainEntry && entryAsset.options.outFile) { + name = entryAsset.options.outFile; + } + + // If this is an entry asset, don't hash. Return a relative path + // from the main file so we keep the original file paths. + if (isEntry) { + return Path.join( + Path.relative( + Path.dirname(entryAsset.options.mainFile), + Path.dirname(entryAsset.name) + ), + name + ext + ); + } + + // If this is an index file, use the parent directory name instead + // which is probably more descriptive. + if (name === 'index') { + name = Path.basename(Path.dirname(entryAsset.name)); + } + + // Add the content hash and extension. + return name + '.' + hash + ext; + } + async package(bundler, oldHashes, newHashes = new Map()) { if (this.isEmpty) { return newHashes; @@ -125,6 +178,7 @@ class Bundle { let packager = new Packager(this, bundler); let startTime = Date.now(); + await packager.setup(); await packager.start(); let included = new Set(); diff --git a/src/Bundler.js b/src/Bundler.js index 578851cba3c..c9e4f900bd5 100644 --- a/src/Bundler.js +++ b/src/Bundler.js @@ -98,7 +98,11 @@ class Bundler extends EventEmitter { options.hmrHostname || (options.target === 'electron' ? 'localhost' : ''), detailedReport: options.detailedReport || false, - autoinstall: (options.autoinstall || false) && !isProduction + autoinstall: (options.autoinstall || false) && !isProduction, + contentHash: + typeof options.contentHash === 'boolean' + ? options.contentHash + : isProduction }; } @@ -198,8 +202,14 @@ class Bundler extends EventEmitter { } // Create a new bundle tree and package everything up. - let bundle = this.createBundleTree(this.mainAsset); - this.bundleHashes = await bundle.package(this, this.bundleHashes); + this.mainBundle = this.createBundleTree(this.mainAsset); + this.bundleNameMap = this.mainBundle.getBundleNameMap( + this.options.contentHash + ); + this.bundleHashes = await this.mainBundle.package( + this, + this.bundleHashes + ); // Unload any orphaned assets this.unloadOrphanedAssets(); @@ -208,11 +218,11 @@ class Bundler extends EventEmitter { let time = prettifyTime(buildTime); logger.status(emoji.success, `Built in ${time}.`, 'green'); if (!this.watcher) { - bundleReport(bundle, this.options.detailedReport); + bundleReport(this.mainBundle, this.options.detailedReport); } - this.emit('bundled', bundle); - return bundle; + this.emit('bundled', this.mainBundle); + return this.mainBundle; } catch (err) { this.errored = true; logger.error(err); diff --git a/src/Logger.js b/src/Logger.js index 3e8aa5d65a6..b01ca4871c6 100644 --- a/src/Logger.js +++ b/src/Logger.js @@ -20,6 +20,10 @@ class Logger { ? options.color : chalk.supportsColor; this.chalk = new chalk.constructor({enabled: this.color}); + this.isTest = + options && typeof options.isTest === 'boolean' + ? options.isTest + : process.env.NODE_ENV === 'test'; } countLines(message) { @@ -87,7 +91,7 @@ class Logger { } clear() { - if (!this.color) { + if (!this.color || this.isTest) { return; } diff --git a/src/Server.js b/src/Server.js index b6230a78421..bfa03ad50ab 100644 --- a/src/Server.js +++ b/src/Server.js @@ -6,6 +6,7 @@ const serverErrors = require('./utils/customErrors').serverErrors; const generateCertificate = require('./utils/generateCertificate'); const getCertificate = require('./utils/getCertificate'); const logger = require('./Logger'); +const path = require('path'); serveStatic.mime.define({ 'application/wasm': ['wasm'] @@ -56,8 +57,8 @@ function middleware(bundler) { function sendIndex() { // If the main asset is an HTML file, serve it - if (bundler.mainAsset.type === 'html') { - req.url = `/${bundler.mainAsset.generateBundleName()}`; + if (bundler.mainBundle.type === 'html') { + req.url = `/${path.basename(bundler.mainBundle.name)}`; serve(req, res, send404); } else { send404(); diff --git a/src/assets/HTMLAsset.js b/src/assets/HTMLAsset.js index d6ff6e8f0c9..73c5233ba24 100644 --- a/src/assets/HTMLAsset.js +++ b/src/assets/HTMLAsset.js @@ -61,6 +61,16 @@ const META = { ] }; +// Options to be passed to `addURLDependency` for certain tags + attributes +const OPTIONS = { + a: { + href: {entry: true} + }, + iframe: { + src: {entry: true} + } +}; + class HTMLAsset extends Asset { constructor(name, pkg, options) { super(name, pkg, options); @@ -75,20 +85,20 @@ class HTMLAsset extends Asset { return res; } - processSingleDependency(path) { - let assetPath = this.addURLDependency(decodeURIComponent(path)); + processSingleDependency(path, opts) { + let assetPath = this.addURLDependency(decodeURIComponent(path), opts); if (!isURL(assetPath)) { assetPath = urlJoin(this.options.publicURL, assetPath); } return assetPath; } - collectSrcSetDependencies(srcset) { + collectSrcSetDependencies(srcset, opts) { const newSources = []; for (const source of srcset.split(',')) { const pair = source.trim().split(' '); if (pair.length === 0) continue; - pair[0] = this.processSingleDependency(pair[0]); + pair[0] = this.processSingleDependency(pair[0], opts); newSources.push(pair.join(' ')); } return newSources.join(','); @@ -121,9 +131,15 @@ class HTMLAsset extends Asset { if (node.tag === 'a' && node.attrs[attr].lastIndexOf('.') < 1) { continue; } + if (elements && elements.includes(node.tag)) { let depHandler = this.getAttrDepHandler(attr); - node.attrs[attr] = depHandler.call(this, node.attrs[attr]); + let options = OPTIONS[node.tag]; + node.attrs[attr] = depHandler.call( + this, + node.attrs[attr], + options && options[attr] + ); this.isAstDirty = true; } } diff --git a/src/assets/RawAsset.js b/src/assets/RawAsset.js index 310fa2d98fe..4a1be0cdda5 100644 --- a/src/assets/RawAsset.js +++ b/src/assets/RawAsset.js @@ -1,5 +1,6 @@ const Asset = require('../Asset'); const urlJoin = require('../utils/urlJoin'); +const md5 = require('../utils/md5'); class RawAsset extends Asset { // Don't load raw assets. They will be copied by the RawPackager directly. @@ -21,6 +22,10 @@ class RawAsset extends Asset { js: `module.exports=${JSON.stringify(pathToAsset)};` }; } + + async generateHash() { + return await md5.file(this.name); + } } module.exports = RawAsset; diff --git a/src/packagers/CSSPackager.js b/src/packagers/CSSPackager.js index 43e55f2edeb..a188fb1a24b 100644 --- a/src/packagers/CSSPackager.js +++ b/src/packagers/CSSPackager.js @@ -22,7 +22,7 @@ class CSSPackager extends Packager { css = `@media ${media.join(', ')} {\n${css.trim()}\n}\n`; } - await this.dest.write(css); + await this.write(css); } } diff --git a/src/packagers/HTMLPackager.js b/src/packagers/HTMLPackager.js index 511385d02a3..498ba7dda9d 100644 --- a/src/packagers/HTMLPackager.js +++ b/src/packagers/HTMLPackager.js @@ -31,7 +31,7 @@ class HTMLPackager extends Packager { ).process(html, {sync: true}).html; } - await this.dest.write(html); + await this.write(html); } addBundlesToTree(bundles, tree) { diff --git a/src/packagers/JSPackager.js b/src/packagers/JSPackager.js index c6e9de8d20e..d5271c09344 100644 --- a/src/packagers/JSPackager.js +++ b/src/packagers/JSPackager.js @@ -30,7 +30,7 @@ class JSPackager extends Packager { this.options.hmrHostname )};` + preludeCode; } - await this.dest.write(preludeCode + '({'); + await this.write(preludeCode + '({'); this.lineOffset = lineCounter(preludeCode); } @@ -101,7 +101,7 @@ class JSPackager extends Packager { wrapped += ']'; this.first = false; - await this.dest.write(wrapped); + await this.write(wrapped); // Use the pre-computed line count from the source map if possible let lineCount = map && map.lineCount ? map.lineCount : lineCounter(code); @@ -198,10 +198,10 @@ class JSPackager extends Packager { entry.push(this.bundle.entryAsset.id); } - await this.dest.write('},{},' + JSON.stringify(entry) + ')'); + await this.write('},{},' + JSON.stringify(entry) + ')'); if (this.options.sourceMaps) { // Add source map url - await this.dest.write( + await this.write( `\n//# sourceMappingURL=${urlJoin( this.options.publicURL, path.basename(this.bundle.name, '.js') + '.map' diff --git a/src/packagers/Packager.js b/src/packagers/Packager.js index 9e495b55fe1..e993f721cec 100644 --- a/src/packagers/Packager.js +++ b/src/packagers/Packager.js @@ -1,20 +1,39 @@ const fs = require('fs'); const promisify = require('../utils/promisify'); +const path = require('path'); +const {mkdirp} = require('../utils/fs'); class Packager { constructor(bundle, bundler) { this.bundle = bundle; this.bundler = bundler; this.options = bundler.options; - this.setup(); } - setup() { + async setup() { + // Create sub-directories if needed + if (this.bundle.name.includes(path.sep)) { + await mkdirp(path.dirname(this.bundle.name)); + } + this.dest = fs.createWriteStream(this.bundle.name); this.dest.write = promisify(this.dest.write.bind(this.dest)); this.dest.end = promisify(this.dest.end.bind(this.dest)); } + async write(string) { + await this.dest.write(this.replaceBundleNames(string)); + } + + replaceBundleNames(string) { + // Replace temporary bundle names in the output with the final content-hashed names. + for (let [name, map] of this.bundler.bundleNameMap) { + string = string.split(name).join(map); + } + + return string; + } + async start() {} // eslint-disable-next-line no-unused-vars diff --git a/src/packagers/RawPackager.js b/src/packagers/RawPackager.js index a0f14f540f2..d45d349dfd7 100644 --- a/src/packagers/RawPackager.js +++ b/src/packagers/RawPackager.js @@ -1,7 +1,5 @@ const Packager = require('./Packager'); const fs = require('../utils/fs'); -const path = require('path'); -const url = require('url'); class RawPackager extends Packager { // Override so we don't create a file for this bundle. @@ -9,22 +7,13 @@ class RawPackager extends Packager { setup() {} async addAsset(asset) { - // Use the bundle name if this is the entry asset, otherwise generate one. - let name = this.bundle.name; - if (asset !== this.bundle.entryAsset) { - name = url.resolve( - path.join(path.dirname(this.bundle.name), asset.generateBundleName()), - '' - ); - } - let contents = asset.generated[asset.type]; if (!contents || (contents && contents.path)) { contents = await fs.readFile(contents ? contents.path : asset.name); } this.size = contents.length; - await fs.writeFile(name, contents); + await fs.writeFile(this.bundle.name, contents); } getSize() { diff --git a/src/packagers/SourceMapPackager.js b/src/packagers/SourceMapPackager.js index 6b00700ac29..925e55dddc1 100644 --- a/src/packagers/SourceMapPackager.js +++ b/src/packagers/SourceMapPackager.js @@ -16,7 +16,7 @@ class SourceMapPackager extends Packager { async end() { let file = path.basename(this.bundle.name); - await this.dest.write(this.sourceMap.stringify(file)); + await this.write(this.sourceMap.stringify(file)); await super.end(); } } diff --git a/src/utils/md5.js b/src/utils/md5.js index 9733a64c25d..bf111f62865 100644 --- a/src/utils/md5.js +++ b/src/utils/md5.js @@ -1,4 +1,5 @@ const crypto = require('crypto'); +const fs = require('fs'); function md5(string) { return crypto @@ -7,4 +8,16 @@ function md5(string) { .digest('hex'); } +md5.file = function(filename) { + return new Promise((resolve, reject) => { + fs + .createReadStream(filename) + .pipe(crypto.createHash('md5').setEncoding('hex')) + .on('finish', function() { + resolve(this.read()); + }) + .on('error', reject); + }); +}; + module.exports = md5; diff --git a/src/visitors/dependencies.js b/src/visitors/dependencies.js index 181f55338e5..ee49ae88abc 100644 --- a/src/visitors/dependencies.js +++ b/src/visitors/dependencies.js @@ -65,7 +65,9 @@ module.exports = { matchesPattern(callee, serviceWorkerPattern); if (isRegisterServiceWorker) { - addURLDependency(asset, args[0]); + // Treat service workers as an entry point so filenames remain consistent across builds. + // https://developers.google.com/web/fundamentals/primers/service-workers/lifecycle#avoid_changing_the_url_of_your_service_worker_script + addURLDependency(asset, args[0], {entry: true}); return; } }, @@ -88,11 +90,7 @@ module.exports = { function addDependency(asset, node, opts = {}) { if (asset.options.target !== 'browser') { - const isRelativeImport = - node.value.startsWith('/') || - node.value.startsWith('./') || - node.value.startsWith('../'); - + const isRelativeImport = /^[/~.]/.test(node.value); if (!isRelativeImport) return; } @@ -100,8 +98,10 @@ function addDependency(asset, node, opts = {}) { asset.addDependency(node.value, opts); } -function addURLDependency(asset, node) { - let assetPath = asset.addURLDependency(node.value); +function addURLDependency(asset, node, opts = {}) { + opts.loc = node.loc && node.loc.start; + + let assetPath = asset.addURLDependency(node.value, opts); if (!isURL(assetPath)) { assetPath = urlJoin(asset.options.publicURL, assetPath); } diff --git a/test/contentHashing.js b/test/contentHashing.js new file mode 100644 index 00000000000..1f2800c902a --- /dev/null +++ b/test/contentHashing.js @@ -0,0 +1,67 @@ +const assert = require('assert'); +const fs = require('fs'); +const {bundle} = require('./utils'); +const rimraf = require('rimraf'); +const promisify = require('../src/utils/promisify'); +const ncp = promisify(require('ncp')); + +describe('content hashing', function() { + beforeEach(function() { + rimraf.sync(__dirname + '/input'); + }); + + it('should update content hash when content changes', async function() { + await ncp(__dirname + '/integration/html-css', __dirname + '/input'); + + await bundle(__dirname + '/input/index.html', { + production: true + }); + + let html = fs.readFileSync(__dirname + '/dist/index.html', 'utf8'); + let filename = html.match( + // + )[1]; + assert(fs.existsSync(__dirname + '/dist/' + filename)); + + fs.writeFileSync( + __dirname + '/input/index.css', + 'body { background: green }' + ); + + await bundle(__dirname + '/input/index.html', { + production: true + }); + + html = fs.readFileSync(__dirname + '/dist/index.html', 'utf8'); + let newFilename = html.match( + // + )[1]; + assert(fs.existsSync(__dirname + '/dist/' + newFilename)); + + assert.notEqual(filename, newFilename); + }); + + it('should update content hash when raw asset changes', async function() { + await ncp(__dirname + '/integration/import-raw', __dirname + '/input'); + + await bundle(__dirname + '/input/index.js', { + production: true + }); + + let js = fs.readFileSync(__dirname + '/dist/index.js', 'utf8'); + let filename = js.match(/\/dist\/(test\.[0-9a-f]+\.txt)/)[1]; + assert(fs.existsSync(__dirname + '/dist/' + filename)); + + fs.writeFileSync(__dirname + '/input/test.txt', 'hello world'); + + await bundle(__dirname + '/input/index.js', { + production: true + }); + + js = fs.readFileSync(__dirname + '/dist/index.js', 'utf8'); + let newFilename = js.match(/\/dist\/(test\.[0-9a-f]+\.txt)/)[1]; + assert(fs.existsSync(__dirname + '/dist/' + newFilename)); + + assert.notEqual(filename, newFilename); + }); +}); diff --git a/test/css.js b/test/css.js index 57e39686d8f..5c1f71cf5bb 100644 --- a/test/css.js +++ b/test/css.js @@ -131,7 +131,7 @@ describe('css', function() { assert.equal(output(), 2); let css = fs.readFileSync(__dirname + '/dist/index.css', 'utf8'); - assert(/url\("[0-9a-f]+\.woff2"\)/.test(css)); + assert(/url\("test\.[0-9a-f]+\.woff2"\)/.test(css)); assert(css.includes('url("http://google.com")')); assert(css.includes('.index')); assert(css.includes('url("data:image/gif;base64,quotes")')); @@ -141,7 +141,7 @@ describe('css', function() { assert( fs.existsSync( - __dirname + '/dist/' + css.match(/url\("([0-9a-f]+\.woff2)"\)/)[1] + __dirname + '/dist/' + css.match(/url\("(test\.[0-9a-f]+\.woff2)"\)/)[1] ) ); }); @@ -176,7 +176,7 @@ describe('css', function() { assert.equal(output(), 2); let css = fs.readFileSync(__dirname + '/dist/index.css', 'utf8'); - assert(/url\([0-9a-f]+\.woff2\)/.test(css), 'woff ext found in css'); + assert(/url\(test\.[0-9a-f]+\.woff2\)/.test(css), 'woff ext found in css'); assert(css.includes('url(http://google.com)'), 'url() found'); assert(css.includes('.index'), '.index found'); assert(css.includes('url("data:image/gif;base64,quotes")')); @@ -186,7 +186,7 @@ describe('css', function() { assert( fs.existsSync( - __dirname + '/dist/' + css.match(/url\(([0-9a-f]+\.woff2)\)/)[1] + __dirname + '/dist/' + css.match(/url\((test\.[0-9a-f]+\.woff2)\)/)[1] ) ); }); diff --git a/test/html.js b/test/html.js index 839219e17d7..954f6b653fe 100644 --- a/test/html.js +++ b/test/html.js @@ -142,7 +142,7 @@ describe('html', function() { let html = fs.readFileSync(__dirname + '/dist/index.html'); assert( - //.test( + //.test( html ) ); @@ -174,7 +174,7 @@ describe('html', function() { let html = fs.readFileSync(__dirname + '/dist/index.html'); assert( - /\s*\s*/.test( + /\s*\s*/.test( html ) ); @@ -212,7 +212,9 @@ describe('html', function() { }); let html = fs.readFileSync(__dirname + '/dist/index.html'); - assert(/