From 3541e3efcd6533e1b8041971c1d5136a1644a6de Mon Sep 17 00:00:00 2001 From: Devon Govett Date: Mon, 8 Jan 2018 21:50:18 -0800 Subject: [PATCH 1/3] Add config files as dependencies so they are watched and invalidate the cache --- src/Asset.js | 14 ++++++++++++++ src/assets/JSAsset.js | 3 +-- src/assets/LESSAsset.js | 3 +-- src/assets/SASSAsset.js | 3 +-- src/assets/StylusAsset.js | 3 +-- src/assets/TypeScriptAsset.js | 3 +-- src/transforms/babel.js | 3 +-- src/transforms/uglify.js | 3 +-- 8 files changed, 21 insertions(+), 14 deletions(-) diff --git a/src/Asset.js b/src/Asset.js index a636764e32a..244d88eb1dd 100644 --- a/src/Asset.js +++ b/src/Asset.js @@ -4,6 +4,7 @@ const objectHash = require('./utils/objectHash'); const md5 = require('./utils/md5'); const isURL = require('./utils/is-url'); const sanitizeFilename = require('sanitize-filename'); +const config = require('./utils/config'); let ASSET_ID = 1; @@ -82,6 +83,19 @@ class Asset { .generateBundleName(); } + async getConfig(filenames) { + // Resolve the config file + let conf = await config.resolve(this.name, filenames); + if (conf) { + // Add as a dependency so it is added to the watcher and invalidates + // this asset when the config changes. + this.addDependency(conf, {includedInParent: true}); + return await config.load(this.name, filenames); + } + + return null; + } + mightHaveDependencies() { return true; } diff --git a/src/assets/JSAsset.js b/src/assets/JSAsset.js index 3ec9b956d25..63f0d1f5537 100644 --- a/src/assets/JSAsset.js +++ b/src/assets/JSAsset.js @@ -10,7 +10,6 @@ const fsVisitor = require('../visitors/fs'); const babel = require('../transforms/babel'); const generate = require('babel-generator').default; const uglify = require('../transforms/uglify'); -const config = require('../utils/config'); const IMPORT_RE = /\b(?:import\b|export\b|require\s*\()/; const GLOBAL_RE = /\b(?:process|__dirname|__filename|global|Buffer)\b/; @@ -55,7 +54,7 @@ class JSAsset extends Asset { // Check if there is a babel config file. If so, determine which parser plugins to enable this.babelConfig = (this.package && this.package.babel) || - (await config.load(this.name, ['.babelrc', '.babelrc.js'])); + (await this.getConfig(['.babelrc', '.babelrc.js'])); if (this.babelConfig) { const file = new BabelFile({filename: this.name}); options.plugins.push(...file.parserOpts.plugins); diff --git a/src/assets/LESSAsset.js b/src/assets/LESSAsset.js index 88e06d1209c..a2eb4a97042 100644 --- a/src/assets/LESSAsset.js +++ b/src/assets/LESSAsset.js @@ -1,5 +1,4 @@ const CSSAsset = require('./CSSAsset'); -const config = require('../utils/config'); const localRequire = require('../utils/localRequire'); const promisify = require('../utils/promisify'); @@ -11,7 +10,7 @@ class LESSAsset extends CSSAsset { let opts = this.package.less || - (await config.load(this.name, ['.lessrc', '.lessrc.js'])) || + (await this.getConfig(['.lessrc', '.lessrc.js'])) || {}; opts.filename = this.name; opts.plugins = (opts.plugins || []).concat(urlPlugin(this)); diff --git a/src/assets/SASSAsset.js b/src/assets/SASSAsset.js index 3f961ae065a..203769f2dd2 100644 --- a/src/assets/SASSAsset.js +++ b/src/assets/SASSAsset.js @@ -1,5 +1,4 @@ const CSSAsset = require('./CSSAsset'); -const config = require('../utils/config'); const localRequire = require('../utils/localRequire'); const promisify = require('../utils/promisify'); const path = require('path'); @@ -12,7 +11,7 @@ class SASSAsset extends CSSAsset { let opts = this.package.sass || - (await config.load(this.name, ['.sassrc', '.sassrc.js'])) || + (await this.getConfig(['.sassrc', '.sassrc.js'])) || {}; opts.includePaths = (opts.includePaths || []).concat( path.dirname(this.name) diff --git a/src/assets/StylusAsset.js b/src/assets/StylusAsset.js index bdce6802b02..fae87e26b50 100644 --- a/src/assets/StylusAsset.js +++ b/src/assets/StylusAsset.js @@ -1,5 +1,4 @@ const CSSAsset = require('./CSSAsset'); -const config = require('../utils/config'); const localRequire = require('../utils/localRequire'); const Resolver = require('../Resolver'); @@ -11,7 +10,7 @@ class StylusAsset extends CSSAsset { let stylus = await localRequire('stylus', this.name); let opts = this.package.stylus || - (await config.load(this.name, ['.stylusrc', '.stylusrc.js'])); + (await this.getConfig(['.stylusrc', '.stylusrc.js'])); let style = stylus(code, opts); style.set('filename', this.name); style.set('include css', true); diff --git a/src/assets/TypeScriptAsset.js b/src/assets/TypeScriptAsset.js index 450e6e3799f..e61aaafc018 100644 --- a/src/assets/TypeScriptAsset.js +++ b/src/assets/TypeScriptAsset.js @@ -1,5 +1,4 @@ const JSAsset = require('./JSAsset'); -const config = require('../utils/config'); const localRequire = require('../utils/localRequire'); class TypeScriptAsset extends JSAsset { @@ -14,7 +13,7 @@ class TypeScriptAsset extends JSAsset { fileName: this.basename }; - let tsconfig = await config.load(this.name, ['tsconfig.json']); + let tsconfig = await this.getConfig(['tsconfig.json']); // Overwrite default if config is found if (tsconfig) { diff --git a/src/transforms/babel.js b/src/transforms/babel.js index 03b2f5fb75a..6c9b99d4a43 100644 --- a/src/transforms/babel.js +++ b/src/transforms/babel.js @@ -1,5 +1,4 @@ const babel = require('babel-core'); -const config = require('../utils/config'); module.exports = async function(asset) { if (!await shouldTransform(asset)) { @@ -40,6 +39,6 @@ async function shouldTransform(asset) { return true; } - let babelrc = await config.resolve(asset.name, ['.babelrc', '.babelrc.js']); + let babelrc = await asset.getConfig(['.babelrc', '.babelrc.js']); return !!babelrc; } diff --git a/src/transforms/uglify.js b/src/transforms/uglify.js index 6e19850ab2f..7377ba3bffb 100644 --- a/src/transforms/uglify.js +++ b/src/transforms/uglify.js @@ -1,5 +1,4 @@ const {minify} = require('uglify-es'); -const config = require('../utils/config'); module.exports = async function(asset) { await asset.parseIfNeeded(); @@ -7,7 +6,7 @@ module.exports = async function(asset) { // Convert AST into JS let code = asset.generate().js; - let customConfig = await config.load(asset.name, ['.uglifyrc']); + let customConfig = await asset.getConfig(['.uglifyrc']); let options = { warnings: true, mangle: { From 727d8107f69815f51f2cd4c5f57f431dbaac3491 Mon Sep 17 00:00:00 2001 From: Devon Govett Date: Mon, 8 Jan 2018 23:05:45 -0800 Subject: [PATCH 2/3] Invalidate cache when environment variables change This adds a cacheData object to Assets, and a shouldInvalidate method which accepts a cacheData object to compare. If it returns false, the asset is re-generated. JSAsset uses this to implement environment variable cache invalidation. --- src/Asset.js | 5 +++++ src/Bundler.js | 2 +- src/assets/JSAsset.js | 11 +++++++++++ src/visitors/globals.js | 1 + src/worker.js | 3 ++- 5 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/Asset.js b/src/Asset.js index 244d88eb1dd..9a890c824a7 100644 --- a/src/Asset.js +++ b/src/Asset.js @@ -34,6 +34,11 @@ class Asset { this.depAssets = new Map(); this.parentBundle = null; this.bundles = new Set(); + this.cacheData = {}; + } + + shouldInvalidate() { + return false; } async loadIfNeeded() { diff --git a/src/Bundler.js b/src/Bundler.js index b6126a127c4..bd2aebfa26d 100644 --- a/src/Bundler.js +++ b/src/Bundler.js @@ -317,7 +317,7 @@ class Bundler extends EventEmitter { // First try the cache, otherwise load and compile in the background let processed = this.cache && (await this.cache.read(asset.name)); - if (!processed) { + if (!processed || asset.shouldInvalidate(processed.cacheData)) { processed = await this.farm.run(asset.name, asset.package, this.options); if (this.cache) { this.cache.write(asset.name, processed); diff --git a/src/assets/JSAsset.js b/src/assets/JSAsset.js index 63f0d1f5537..808d71d4bf7 100644 --- a/src/assets/JSAsset.js +++ b/src/assets/JSAsset.js @@ -25,6 +25,17 @@ class JSAsset extends Asset { this.isAstDirty = false; this.isES6Module = false; this.outputCode = null; + this.cacheData.env = {}; + } + + shouldInvalidate(cacheData) { + for (let key in cacheData.env) { + if (cacheData.env[key] !== process.env[key]) { + return true; + } + } + + return false; } mightHaveDependencies() { diff --git a/src/visitors/globals.js b/src/visitors/globals.js index 2bb8ca701fa..e2107912d99 100644 --- a/src/visitors/globals.js +++ b/src/visitors/globals.js @@ -26,6 +26,7 @@ module.exports = { let val = types.valueToNode(process.env[key.value]); morph(node, val); asset.isAstDirty = true; + asset.cacheData.env[key.value] = process.env[key.value]; } } }, diff --git a/src/worker.js b/src/worker.js index f6ed40eb7ed..c1ded1407f1 100644 --- a/src/worker.js +++ b/src/worker.js @@ -17,7 +17,8 @@ exports.run = async function(path, pkg, options, callback) { callback(null, { dependencies: Array.from(asset.dependencies.values()), generated: asset.generated, - hash: asset.hash + hash: asset.hash, + cacheData: asset.cacheData }); } catch (err) { let returned = err; From eceebebe8eef19eeaf6ac49e9f80cd6e78fe470d Mon Sep 17 00:00:00 2001 From: Devon Govett Date: Tue, 9 Jan 2018 09:27:01 -0800 Subject: [PATCH 3/3] Recompile all parent assets that have an includedInParent dep For example, if you had a stylus file that was @imported by several parents, only one of them would get recompiled when that file changed. This also affected .babelrc and other config changes, etc. --- src/Bundler.js | 75 +++++++++++++++++++-------- src/HMRServer.js | 5 +- src/packagers/JSPackager.js | 4 +- src/utils/config.js | 2 - test/integration/babel/.babelrc | 7 +++ test/integration/babel/.eslintrc.json | 6 +++ test/integration/babel/foo.js | 1 + test/integration/babel/index.js | 4 ++ test/watcher.js | 22 ++++++++ 9 files changed, 97 insertions(+), 29 deletions(-) create mode 100644 test/integration/babel/.babelrc create mode 100644 test/integration/babel/.eslintrc.json create mode 100644 test/integration/babel/foo.js create mode 100644 test/integration/babel/index.js diff --git a/src/Bundler.js b/src/Bundler.js index bd2aebfa26d..1734932d4b9 100644 --- a/src/Bundler.js +++ b/src/Bundler.js @@ -35,6 +35,7 @@ class Bundler extends EventEmitter { this.pending = false; this.loadedAssets = new Map(); + this.watchedAssets = new Map(); this.farm = null; this.watcher = null; this.hmr = null; @@ -267,11 +268,35 @@ class Bundler extends EventEmitter { let asset = this.parser.getAsset(path, pkg, this.options); this.loadedAssets.set(path, asset); - if (this.watcher) { + this.watch(path, asset); + return asset; + } + + watch(path, asset) { + if (!this.watcher) { + return; + } + + if (!this.watchedAssets.has(path)) { this.watcher.add(path); + this.watchedAssets.set(path, new Set()); } - return asset; + this.watchedAssets.get(path).add(asset); + } + + unwatch(path, asset) { + if (!this.watchedAssets.has(path)) { + return; + } + + let watched = this.watchedAssets.get(path); + watched.delete(asset); + + if (watched.size === 0) { + this.watchedAssets.delete(path); + this.watcher.unwatch(path); + } } async resolveDep(asset, dep) { @@ -339,26 +364,25 @@ class Bundler extends EventEmitter { // Resolve and load asset dependencies let assetDeps = await Promise.all( dependencies.map(async dep => { - let assetDep = await this.resolveDep(asset, dep); - if (!dep.includedInParent) { + if (dep.includedInParent) { + // This dependency is already included in the parent's generated output, + // so no need to load it. We map the name back to the parent asset so + // that changing it triggers a recompile of the parent. + this.watch(dep.name, asset); + } else { + let assetDep = await this.resolveDep(asset, dep); await this.loadAsset(assetDep); + return assetDep; } - - return assetDep; }) ); // Store resolved assets in their original order dependencies.forEach((dep, i) => { + asset.dependencies.set(dep.name, dep); let assetDep = assetDeps[i]; - if (dep.includedInParent) { - // This dependency is already included in the parent's generated output, - // so no need to load it. We map the name back to the parent asset so - // that changing it triggers a recompile of the parent. - this.loadedAssets.set(dep.name, asset); - } else { - asset.dependencies.set(dep.name, dep); - asset.depAssets.set(dep.name, assetDep); + if (assetDep) { + asset.depAssets.set(dep, assetDep); } }); @@ -421,8 +445,7 @@ class Bundler extends EventEmitter { asset.parentBundle = bundle; - for (let dep of asset.dependencies.values()) { - let assetDep = asset.depAssets.get(dep.name); + for (let [dep, assetDep] of asset.depAssets) { this.createBundleTree(assetDep, dep, bundle); } @@ -463,21 +486,31 @@ class Bundler extends EventEmitter { unloadAsset(asset) { this.loadedAssets.delete(asset.name); if (this.watcher) { - this.watcher.unwatch(asset.name); + this.unwatch(asset.name, asset); + + // Unwatch all included dependencies that map to this asset + for (let dep of asset.dependencies.values()) { + if (dep.includedInParent) { + this.unwatch(dep.name, asset); + } + } } } async onChange(path) { - let asset = this.loadedAssets.get(path); - if (!asset) { + let assets = this.watchedAssets.get(path); + if (!assets || !assets.size) { return; } this.logger.clear(); - this.logger.status(emoji.progress, `Building ${asset.basename}...`); + this.logger.status(emoji.progress, `Building ${Path.basename(path)}...`); // Add the asset to the rebuild queue, and reset the timeout. - this.buildQueue.add(asset); + for (let asset of assets) { + this.buildQueue.add(asset); + } + clearTimeout(this.rebuildTimeout); this.rebuildTimeout = setTimeout(async () => { diff --git a/src/HMRServer.js b/src/HMRServer.js index 620d34eabd5..516374c1a72 100644 --- a/src/HMRServer.js +++ b/src/HMRServer.js @@ -57,9 +57,8 @@ class HMRServer { type: 'update', assets: assets.map(asset => { let deps = {}; - for (let dep of asset.dependencies.values()) { - let mod = asset.depAssets.get(dep.name); - deps[dep.name] = mod.id; + for (let [dep, depAsset] of asset.depAssets) { + deps[dep.name] = depAsset.id; } return { diff --git a/src/packagers/JSPackager.js b/src/packagers/JSPackager.js index 1776807fe37..1fabad6103a 100644 --- a/src/packagers/JSPackager.js +++ b/src/packagers/JSPackager.js @@ -36,9 +36,7 @@ class JSPackager extends Packager { } let deps = {}; - for (let dep of asset.dependencies.values()) { - let mod = asset.depAssets.get(dep.name); - + for (let [dep, mod] of asset.depAssets) { // For dynamic dependencies, list the child bundles to load along with the module id if (dep.dynamic && this.bundle.childBundles.has(mod.parentBundle)) { let bundles = [path.basename(mod.parentBundle.name)]; diff --git a/src/utils/config.js b/src/utils/config.js index 720c1e351db..eb8eed4618d 100644 --- a/src/utils/config.js +++ b/src/utils/config.js @@ -21,8 +21,6 @@ async function resolve(filepath, filenames, root = path.parse(filepath).root) { existsCache.set(file, true); return file; } - - existsCache.set(file, false); } return resolve(filepath, filenames, root); diff --git a/test/integration/babel/.babelrc b/test/integration/babel/.babelrc new file mode 100644 index 00000000000..15edf6beb98 --- /dev/null +++ b/test/integration/babel/.babelrc @@ -0,0 +1,7 @@ +{ + "presets": [["env", { + "targets": { + "browsers": ["last 2 Chrome versions"] + } + }]] +} diff --git a/test/integration/babel/.eslintrc.json b/test/integration/babel/.eslintrc.json new file mode 100644 index 00000000000..f89e231fa93 --- /dev/null +++ b/test/integration/babel/.eslintrc.json @@ -0,0 +1,6 @@ +{ + "extends": "../.eslintrc.json", + "parserOptions": { + "sourceType": "module" + } +} \ No newline at end of file diff --git a/test/integration/babel/foo.js b/test/integration/babel/foo.js new file mode 100644 index 00000000000..7804111002d --- /dev/null +++ b/test/integration/babel/foo.js @@ -0,0 +1 @@ +export default class Foo {} diff --git a/test/integration/babel/index.js b/test/integration/babel/index.js new file mode 100644 index 00000000000..37a72cf028f --- /dev/null +++ b/test/integration/babel/index.js @@ -0,0 +1,4 @@ +import Foo from './foo'; + +export {Foo}; +export class Bar {} diff --git a/test/watcher.js b/test/watcher.js index 25f1724f631..8f7b1898af0 100644 --- a/test/watcher.js +++ b/test/watcher.js @@ -172,4 +172,26 @@ describe('watcher', function() { assert(!b.loadedAssets.has(path.join(__dirname, '/input/common-dep.js'))); }); + + it('should recompile all assets when a config file changes', async function() { + await ncp(__dirname + '/integration/babel', __dirname + '/input'); + b = bundler(__dirname + '/input/index.js', {watch: true}); + + await b.bundle(); + let file = fs.readFileSync(__dirname + '/dist/index.js', 'utf8'); + assert(file.includes('class Foo {}')); + assert(file.includes('class Bar {}')); + + // Change babelrc, should recompile both files + let babelrc = JSON.parse( + fs.readFileSync(__dirname + '/input/.babelrc', 'utf8') + ); + babelrc.presets[0][1].targets.browsers.push('IE >= 11'); + fs.writeFileSync(__dirname + '/input/.babelrc', JSON.stringify(babelrc)); + + await nextBundle(b); + file = fs.readFileSync(__dirname + '/dist/index.js', 'utf8'); + assert(!file.includes('class Foo {}')); + assert(!file.includes('class Bar {}')); + }); });