From e34a4d097ef0624890589f471e4a977367da568a Mon Sep 17 00:00:00 2001 From: Devon Govett Date: Wed, 11 Jul 2018 10:12:54 -0700 Subject: [PATCH] Deterministic asset ids (#1694) --- src/Asset.js | 15 +++++++++--- src/Bundler.js | 3 +-- src/Pipeline.js | 5 ++-- src/builtins/hmr-runtime.js | 2 +- src/packagers/JSConcatPackager.js | 17 +++++++------- src/packagers/JSPackager.js | 13 +++++++---- src/scope-hoisting/concat.js | 21 +++++++++-------- src/scope-hoisting/hoist.js | 38 +++++++------------------------ src/scope-hoisting/shake.js | 2 +- src/scope-hoisting/utils.js | 28 +++++++++++++++++++++++ src/utils/md5.js | 4 ++-- src/worker.js | 4 ++-- 12 files changed, 86 insertions(+), 66 deletions(-) create mode 100644 src/scope-hoisting/utils.js diff --git a/src/Asset.js b/src/Asset.js index 2191f15a2c5..79812e3eab9 100644 --- a/src/Asset.js +++ b/src/Asset.js @@ -10,8 +10,6 @@ const syncPromise = require('./utils/syncPromise'); const logger = require('./Logger'); const Resolver = require('./Resolver'); -let ASSET_ID = 1; - /** * An Asset represents a file in the dependency tree. Assets can have multiple * parents that depend on it, and can be added to multiple output bundles. @@ -20,7 +18,7 @@ let ASSET_ID = 1; */ class Asset { constructor(name, options) { - this.id = ASSET_ID++; + this.id = null; this.name = name; this.basename = path.basename(this.name); this.relativeName = path.relative(options.rootDir, this.name); @@ -188,6 +186,17 @@ class Asset { } async process() { + // Generate the id for this asset, unless it has already been set. + // We do this here rather than in the constructor to avoid unnecessary work in the main process. + // In development, the id is just the relative path to the file, for easy debugging and performance. + // In production, we use a short hash of the relative path. + if (!this.id) { + this.id = + this.options.production || this.options.scopeHoist + ? md5(this.relativeName, 'base64').slice(0, 4) + : this.relativeName; + } + if (!this.generated) { await this.loadIfNeeded(); await this.pretransform(); diff --git a/src/Bundler.js b/src/Bundler.js index f1901b41cd8..66d53dece25 100644 --- a/src/Bundler.js +++ b/src/Bundler.js @@ -525,8 +525,7 @@ class Bundler extends EventEmitter { let processed = this.cache && (await this.cache.read(asset.name)); let cacheMiss = false; if (!processed || asset.shouldInvalidate(processed.cacheData)) { - processed = await this.farm.run(asset.name, asset.id); - processed.id = asset.id; + processed = await this.farm.run(asset.name); cacheMiss = true; } diff --git a/src/Pipeline.js b/src/Pipeline.js index 6483bd9ed1a..16c9a9c5b02 100644 --- a/src/Pipeline.js +++ b/src/Pipeline.js @@ -11,15 +11,13 @@ class Pipeline { this.parser = new Parser(options); } - async process(path, id, isWarmUp) { + async process(path, isWarmUp) { let options = this.options; if (isWarmUp) { options = Object.assign({isWarmUp}, options); } let asset = this.parser.getAsset(path, options); - asset.id = id; - let generated = await this.processAsset(asset); let generatedMap = {}; for (let rendition of generated) { @@ -27,6 +25,7 @@ class Pipeline { } return { + id: asset.id, dependencies: Array.from(asset.dependencies.values()), generated: generatedMap, hash: asset.hash, diff --git a/src/builtins/hmr-runtime.js b/src/builtins/hmr-runtime.js index f1645e92a0d..74efb86d4d6 100644 --- a/src/builtins/hmr-runtime.js +++ b/src/builtins/hmr-runtime.js @@ -110,7 +110,7 @@ function getParents(bundle, id) { for (d in modules[k][1]) { dep = modules[k][1][d]; if (dep === id || (Array.isArray(dep) && dep[dep.length - 1] === id)) { - parents.push(+k); + parents.push(k); } } } diff --git a/src/packagers/JSConcatPackager.js b/src/packagers/JSConcatPackager.js index 715049e8a76..f3579e13dce 100644 --- a/src/packagers/JSConcatPackager.js +++ b/src/packagers/JSConcatPackager.js @@ -6,6 +6,7 @@ const urlJoin = require('../utils/urlJoin'); const walk = require('babylon-walk'); const babylon = require('babylon'); const t = require('babel-types'); +const {getName, getIdentifier} = require('../scope-hoisting/utils'); const prelude = { source: fs @@ -125,9 +126,9 @@ class JSConcatPackager extends Packager { } getExportIdentifier(asset) { - let id = '$' + asset.id + '$exports'; + let id = getName(asset, 'exports'); if (this.shouldWrap(asset)) { - return `($${asset.id}$init(), ${id})`; + return `(${getName(asset, 'init')}(), ${id})`; } return id; @@ -317,13 +318,13 @@ class JSConcatPackager extends Packager { } } - let executed = `$${asset.id}$executed`; + let executed = getName(asset, 'executed'); decls.push( t.variableDeclarator(t.identifier(executed), t.booleanLiteral(false)) ); let init = t.functionDeclaration( - t.identifier(`$${asset.id}$init`), + getIdentifier(asset, 'init'), [], t.blockStatement([ t.ifStatement(t.identifier(executed), t.returnStatement()), @@ -504,7 +505,7 @@ class JSConcatPackager extends Packager { ); } - exposed.push(`${m.id}: ${this.getExportIdentifier(m)}`); + exposed.push(`"${m.id}": ${this.getExportIdentifier(m)}`); } this.write(` @@ -545,12 +546,12 @@ class JSConcatPackager extends Packager { } resolveModule(id, name) { - let module = this.assets.get(+id); + let module = this.assets.get(id); return module.depAssets.get(module.dependencies.get(name)); } findExportModule(id, name, replacements) { - let asset = this.assets.get(+id); + let asset = this.assets.get(id); let exp = asset && Object.prototype.hasOwnProperty.call(asset.cacheData.exports, name) @@ -578,7 +579,7 @@ class JSConcatPackager extends Packager { // If this is a wildcard import, resolve to the exports object. if (asset && name === '*') { - exp = `$${id}$exports`; + exp = getName(asset, 'exports'); } if (replacements && replacements.has(exp)) { diff --git a/src/packagers/JSPackager.js b/src/packagers/JSPackager.js index 0fe1c823eed..ddff44d48de 100644 --- a/src/packagers/JSPackager.js +++ b/src/packagers/JSPackager.js @@ -106,7 +106,10 @@ class JSPackager extends Packager { async writeModule(id, code, deps = {}, map) { let wrapped = this.first ? '' : ','; wrapped += - id + ':[function(require,module,exports) {\n' + (code || '') + '\n},'; + JSON.stringify(id) + + ':[function(require,module,exports) {\n' + + (code || '') + + '\n},'; wrapped += JSON.stringify(deps); wrapped += ']'; @@ -154,7 +157,7 @@ class JSPackager extends Packager { } // Generate a module to register the bundle loaders that are needed - let loads = 'var b=require(' + bundleLoader.id + ');'; + let loads = 'var b=require(' + JSON.stringify(bundleLoader.id) + ');'; for (let bundleType of this.bundleLoaders) { let loader = this.options.bundleLoaders[bundleType]; if (loader) { @@ -165,7 +168,7 @@ class JSPackager extends Packager { 'b.register(' + JSON.stringify(bundleType) + ',require(' + - asset.id + + JSON.stringify(asset.id) + '));'; } } @@ -183,7 +186,9 @@ class JSPackager extends Packager { loads += 'b.load(' + JSON.stringify(preload) + ')'; if (this.bundle.entryAsset) { - loads += `.then(function(){require(${this.bundle.entryAsset.id});})`; + loads += `.then(function(){require(${JSON.stringify( + this.bundle.entryAsset.id + )});})`; } loads += ';'; diff --git a/src/scope-hoisting/concat.js b/src/scope-hoisting/concat.js index b7c48087c30..5e3f61f71bf 100644 --- a/src/scope-hoisting/concat.js +++ b/src/scope-hoisting/concat.js @@ -5,8 +5,9 @@ const traverse = require('babel-traverse').default; const generate = require('babel-generator').default; const treeShake = require('./shake'); const mangleScope = require('./mangler'); +const {getName, getIdentifier} = require('./utils'); -const EXPORTS_RE = /^\$([\d]+)\$exports$/; +const EXPORTS_RE = /^\$(.+?)\$exports$/; const DEFAULT_INTEROP_TEMPLATE = template( 'var NAME = $parcel$interopDefault(MODULE)' @@ -43,7 +44,7 @@ module.exports = (packager, ast) => { // If the module is not in this bundle, create a `require` call for it. if (!node && !mod) { - node = REQUIRE_TEMPLATE({ID: t.numericLiteral(id)}).expression; + node = REQUIRE_TEMPLATE({ID: t.stringLiteral(id)}).expression; return interop(module, name, path, node); } @@ -55,7 +56,7 @@ module.exports = (packager, ast) => { // If it is CommonJS, look for an exports object. if (!node && mod.cacheData.isCommonJS) { - node = findSymbol(path, `$${id}$exports`); + node = findSymbol(path, getName(mod, 'exports')); if (!node) { return null; } @@ -82,7 +83,7 @@ module.exports = (packager, ast) => { function interop(mod, originalName, path, node) { // Handle interop for default imports of CommonJS modules. if (mod.cacheData.isCommonJS && originalName === 'default') { - let name = `$${mod.id}$interop$default`; + let name = getName(mod, '$interop$default'); if (!path.scope.getBinding(name)) { let [decl] = path.getStatementParent().insertBefore( DEFAULT_INTEROP_TEMPLATE({ @@ -91,7 +92,7 @@ module.exports = (packager, ast) => { }) ); - let binding = path.scope.getBinding(`$${mod.id}$exports`); + let binding = path.scope.getBinding(getName(mod, 'exports')); if (binding) { binding.reference(decl.get('declarations.0.init')); } @@ -133,7 +134,7 @@ module.exports = (packager, ast) => { if ( args.length !== 2 || - !t.isNumericLiteral(id) || + !t.isStringLiteral(id) || !t.isStringLiteral(source) ) { throw new Error( @@ -158,7 +159,7 @@ module.exports = (packager, ast) => { if (assets.get(mod.id)) { // Replace with nothing if the require call's result is not used. if (!isUnusedValue(path)) { - let name = `$${mod.id}$exports`; + let name = getName(mod, 'exports'); node = t.identifier(replacements.get(name) || name); } @@ -166,11 +167,11 @@ module.exports = (packager, ast) => { // call happens inside a non top-level scope, e.g. in a // function, if statement, or conditional expression. if (mod.cacheData.shouldWrap) { - let call = t.callExpression(t.identifier(`$${mod.id}$init`), []); + let call = t.callExpression(getIdentifier(mod, 'init'), []); node = node ? t.sequenceExpression([call, node]) : call; } } else { - node = REQUIRE_TEMPLATE({ID: t.numericLiteral(mod.id)}).expression; + node = REQUIRE_TEMPLATE({ID: t.stringLiteral(mod.id)}).expression; } if (node) { @@ -184,7 +185,7 @@ module.exports = (packager, ast) => { if ( args.length !== 2 || - !t.isNumericLiteral(id) || + !t.isStringLiteral(id) || !t.isStringLiteral(source) ) { throw new Error( diff --git a/src/scope-hoisting/hoist.js b/src/scope-hoisting/hoist.js index 3a0412eb6bf..b1e34ddcef3 100644 --- a/src/scope-hoisting/hoist.js +++ b/src/scope-hoisting/hoist.js @@ -2,6 +2,7 @@ const matchesPattern = require('../visitors/matches-pattern'); const t = require('babel-types'); const template = require('babel-template'); const rename = require('./renamer'); +const {getName, getIdentifier, getExportIdentifier} = require('./utils'); const WRAPPER_TEMPLATE = template(` var NAME = (function () { @@ -119,8 +120,8 @@ module.exports = { // Rename each binding in the top-level scope to something unique. for (let name in scope.bindings) { - if (!name.startsWith('$' + asset.id)) { - let newName = '$' + asset.id + '$var$' + name; + if (!name.startsWith('$' + t.toIdentifier(asset.id))) { + let newName = getName(asset, 'var', name); rename(scope, name, newName); } } @@ -160,7 +161,7 @@ module.exports = { } if (matchesPattern(path.node, 'module.id')) { - path.replaceWith(t.numericLiteral(asset.id)); + path.replaceWith(t.stringLiteral(asset.id)); } if (matchesPattern(path.node, 'module.hot')) { @@ -302,7 +303,7 @@ module.exports = { // This will be replaced by the final variable name of the resolved asset in the packager. path.replaceWith( REQUIRE_CALL_TEMPLATE({ - ID: t.numericLiteral(asset.id), + ID: t.stringLiteral(asset.id), SOURCE: t.stringLiteral(args[0].value) }) ); @@ -311,7 +312,7 @@ module.exports = { if (matchesPattern(callee, 'require.resolve')) { path.replaceWith( REQUIRE_RESOLVE_CALL_TEMPLATE({ - ID: t.numericLiteral(asset.id), + ID: t.stringLiteral(asset.id), SOURCE: args[0] }) ); @@ -455,7 +456,7 @@ module.exports = { EXPORT_ALL_TEMPLATE({ OLD_NAME: getExportsIdentifier(asset), SOURCE: t.stringLiteral(path.node.source.value), - ID: t.numericLiteral(asset.id) + ID: t.stringLiteral(asset.id) }) ); } @@ -464,7 +465,7 @@ module.exports = { function addImport(asset, path) { // Replace with a $parcel$require call so we know where to insert side effects. let requireCall = REQUIRE_CALL_TEMPLATE({ - ID: t.numericLiteral(asset.id), + ID: t.stringLiteral(asset.id), SOURCE: t.stringLiteral(path.node.source.value) }); @@ -539,29 +540,6 @@ function safeRename(path, asset, from, to) { } } -function getName(asset, type, ...rest) { - return ( - '$' + - asset.id + - '$' + - type + - (rest.length - ? '$' + - rest - .map(name => (name === 'default' ? name : t.toIdentifier(name))) - .join('$') - : '') - ); -} - -function getIdentifier(asset, type, ...rest) { - return t.identifier(getName(asset, type, ...rest)); -} - -function getExportIdentifier(asset, name) { - return getIdentifier(asset, 'export', name); -} - function getExportsIdentifier(asset, scope) { if (scope && scope.getData('shouldWrap')) { return t.identifier('exports'); diff --git a/src/scope-hoisting/shake.js b/src/scope-hoisting/shake.js index 701469c3dbd..40f6964b0f2 100644 --- a/src/scope-hoisting/shake.js +++ b/src/scope-hoisting/shake.js @@ -1,6 +1,6 @@ const t = require('babel-types'); -const EXPORTS_RE = /^\$([\d]+)\$exports$/; +const EXPORTS_RE = /^\$(.+?)\$exports$/; /** * This is a small small implementation of dead code removal specialized to handle diff --git a/src/scope-hoisting/utils.js b/src/scope-hoisting/utils.js new file mode 100644 index 00000000000..fb80d0480a8 --- /dev/null +++ b/src/scope-hoisting/utils.js @@ -0,0 +1,28 @@ +const t = require('babel-types'); + +function getName(asset, type, ...rest) { + return ( + '$' + + t.toIdentifier(asset.id) + + '$' + + type + + (rest.length + ? '$' + + rest + .map(name => (name === 'default' ? name : t.toIdentifier(name))) + .join('$') + : '') + ); +} + +function getIdentifier(asset, type, ...rest) { + return t.identifier(getName(asset, type, ...rest)); +} + +function getExportIdentifier(asset, name) { + return getIdentifier(asset, 'export', name); +} + +exports.getName = getName; +exports.getIdentifier = getIdentifier; +exports.getExportIdentifier = getExportIdentifier; diff --git a/src/utils/md5.js b/src/utils/md5.js index 124fc3ebbea..5d53698d51e 100644 --- a/src/utils/md5.js +++ b/src/utils/md5.js @@ -1,11 +1,11 @@ const crypto = require('crypto'); const fs = require('fs'); -function md5(string) { +function md5(string, encoding = 'hex') { return crypto .createHash('md5') .update(string) - .digest('hex'); + .digest(encoding); } md5.file = function(filename) { diff --git a/src/worker.js b/src/worker.js index c33d51cb63a..6a0a6180f72 100644 --- a/src/worker.js +++ b/src/worker.js @@ -10,9 +10,9 @@ function init(options) { process.env.HMR_HOSTNAME = options.hmrHostname; } -async function run(path, id, isWarmUp) { +async function run(path, isWarmUp) { try { - return await pipeline.process(path, id, isWarmUp); + return await pipeline.process(path, isWarmUp); } catch (e) { e.fileName = path; throw e;