Skip to content

Commit

Permalink
Recompile all parent assets that have an includedInParent dep
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
devongovett committed Jan 9, 2018
1 parent 727d810 commit eceebeb
Show file tree
Hide file tree
Showing 9 changed files with 97 additions and 29 deletions.
75 changes: 54 additions & 21 deletions src/Bundler.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
}
});

Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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 () => {
Expand Down
5 changes: 2 additions & 3 deletions src/HMRServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 1 addition & 3 deletions src/packagers/JSPackager.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)];
Expand Down
2 changes: 0 additions & 2 deletions src/utils/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
7 changes: 7 additions & 0 deletions test/integration/babel/.babelrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"presets": [["env", {
"targets": {
"browsers": ["last 2 Chrome versions"]
}
}]]
}
6 changes: 6 additions & 0 deletions test/integration/babel/.eslintrc.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"extends": "../.eslintrc.json",
"parserOptions": {
"sourceType": "module"
}
}
1 change: 1 addition & 0 deletions test/integration/babel/foo.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export default class Foo {}
4 changes: 4 additions & 0 deletions test/integration/babel/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import Foo from './foo';

export {Foo};
export class Bar {}
22 changes: 22 additions & 0 deletions test/watcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {}'));
});
});

0 comments on commit eceebeb

Please sign in to comment.