Skip to content

Commit

Permalink
fix(bundle): ensure that dependencies are added in order
Browse files Browse the repository at this point in the history
fixes #378
  • Loading branch information
JeroenVinke committed Mar 30, 2017
1 parent 2cd3c56 commit 51a2cce
Show file tree
Hide file tree
Showing 4 changed files with 132 additions and 9 deletions.
22 changes: 13 additions & 9 deletions lib/build/bundle.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const Convert = require('./convert-source-map');
const fs = require('../file-system');
const SourceInclusion = require('./source-inclusion').SourceInclusion;
const DependencyInclusion = require('./dependency-inclusion').DependencyInclusion;
const Utils = require('./utils');

exports.Bundle = class {
constructor(bundler, config) {
Expand Down Expand Up @@ -32,15 +33,18 @@ exports.Bundle = class {
static create(bundler, config) {
let bundle = new exports.Bundle(bundler, config);
let dependencies = config.dependencies || [];

return Promise.all(
dependencies
.filter(x => bundler.itemIncludedInBuild(x))
.map(dependency => bundler.configureDependency(dependency))
)
// Add dependencies in the same order as they were entered
// to prevent a wrong module load order.
.then(descriptions => Promise.all(descriptions.map(dep => bundle.addDependency(dep))))
let dependenciesToBuild = dependencies
.filter(x => bundler.itemIncludedInBuild(x));

return Utils.runSequentially(
dependenciesToBuild,
dep => bundler.configureDependency(dep))
.then(descriptions => {
return Utils.runSequentially(
descriptions,
description => bundle.addDependency(description)
);
})
.then(() => bundle);
}

Expand Down
17 changes: 17 additions & 0 deletions lib/build/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,23 @@ exports.generateBundleName = function(contents, fileName, rev) {
return rev ? exports.generateHashedPath(fileName, hash) : fileName;
};

exports.runSequentially = function(tasks, cb) {
let index = -1;
let result = [];

function exec() {
index ++;

if (index < tasks.length) {
return cb(tasks[index]).then(r => result.push(r)).then(exec);
}

return Promise.resolve();
}

return exec().then(() => result);
};

exports.generateHashedPath = function(pth, hash) {
if (arguments.length !== 2) {
throw new Error('`path` and `hash` required');
Expand Down
53 changes: 53 additions & 0 deletions spec/lib/build/bundle.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,59 @@ describe('the Bundle module', () => {
expect(sut.getBundledFiles()).toEqual(['a.js', 'b.js', 'c.js']);
});

it('configures dependencies in the same order as they were entered to prevent a wrong module load order', done => {
let bundler = new BundlerMock();
let configuredDependencies = [];
bundler.configureDependency.and.callFake(dep => {
let depName = dep.name || dep;
let description = {
loaderConfig: {
name: depName
}
};

return new Promise(resolve => {
if (depName === 'my-large-plugin') {
setTimeout(() => {
configuredDependencies.push(depName);
resolve(description);
}, 100);
} else {
configuredDependencies.push(depName);
resolve(description);
}
});
});
bundler.itemIncludedInBuild.and.callFake((dep) => true);

let config = {
name: 'app-bundle.js',
dependencies: [
'foo',
{
name: 'my-large-plugin',
main: 'index',
path: '../node_modules/my-plugin/'
},
{
name: 'my-other-plugin',
main: 'index',
path: '../node_modules/my-plugin/'
}
]
};

Bundle.create(bundler, config)
.then((bundle) => {
expect(configuredDependencies[0]).toBe('foo');
expect(configuredDependencies[1]).toBe('my-large-plugin');
expect(configuredDependencies[2]).toBe('my-other-plugin');
done();
}).catch(e => {
done.fail(e);
});
});

it('add dependencies in the same order as they were entered to prevent a wrong module load order', done => {
let bundler = new BundlerMock();
bundler.configureDependency.and.callFake(dep => {
Expand Down
49 changes: 49 additions & 0 deletions spec/lib/build/util.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
'use strict';

const Utils = require('../../../lib/build/utils');

describe('the Utils.runSequentially function', () => {
it('calls the callback function for all items', (d) => {
let items = [{ name: 'first' }, { name: 'second' }];
let cb = jasmine.createSpy('cb').and.returnValue(Promise.resolve());
Utils.runSequentially(items, cb)
.then(() => {
expect(cb.calls.count()).toBe(2);
expect(cb.calls.argsFor(0)[0].name).toBe('first');
expect(cb.calls.argsFor(1)[0].name).toBe('second');
d();
});
});

it('runs in sequence', (d) => {
let items = [{ name: 'first' }, { name: 'second' }, { name: 'third' }];
let cb = jasmine.createSpy('cb').and.callFake((item) => {
return new Promise(resolve => {
if (item.name === 'first' || item.name === 'second') {
setTimeout(() => resolve(), 200);
} else {
resolve();
}
});
});
Utils.runSequentially(items, cb)
.then(() => {
expect(cb.calls.argsFor(0)[0].name).toBe('first');
expect(cb.calls.argsFor(1)[0].name).toBe('second');
expect(cb.calls.argsFor(2)[0].name).toBe('third');
d();
});
});

it('handles empty items array', (done) => {
let items = [];
Utils.runSequentially(items, () => {})
.catch(e => {
done.fail(e, '', 'expected no error');
throw e;
})
.then(() => {
done();
});
});
});

0 comments on commit 51a2cce

Please sign in to comment.