Skip to content

Commit

Permalink
fix(bundler): fix a regression on missing yaml file for some users
Browse files Browse the repository at this point in the history 
Additional files like yaml were not processed by any gulp tasks. Previously amodro-trace was able to pull them in directly from file system. This fix mimics the same behaviour.

closes #930
  • Loading branch information
@3cp
3cp committed Oct 10, 2018
1 parent 23be17f commit 4387bff
Show file tree
Hide file tree
Showing 7 changed files with 122 additions and 29 deletions.
12 changes: 9 additions & 3 deletions lib/build/bundled-source.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -253,9 +253,15 @@ exports.BundledSource = class {

let moduleIds = Array.from(new Set(deps)) // unique
.map(stripPluginPrefixOrSubfix)
// don't bother with local dependency in src,
// as we bundled all of local js/html/css files.
.filter(d => this.dependencyInclusion || d[0] !== '.')
.filter(d => {
// any dep requested by a npm package file
if (this.dependencyInclusion) return true;
// For local src, pick up all absolute dep.
if (d[0] !== '.') return true;
// For relative dep, as we bundled all of local js/html/css files,
// only pick up unknown ext that might be missed by gulp tasks.
return Utils.couldMissGulpPreprocess(d);
})
.map(d => absoluteModuleId(moduleId, d))
.filter(d => {
// ignore false replacment
Expand Down
22 changes: 20 additions & 2 deletions lib/build/bundler.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const CLIOptions = require('../cli-options').CLIOptions;
const LoaderPlugin = require('./loader-plugin').LoaderPlugin;
const Configuration = require('../configuration').Configuration;
const path = require('path');
const fs = require('../file-system');
const Utils = require('./utils');
const logger = require('aurelia-logging').getLogger('Bundler');
const stubCoreNodejsModule = require('./stub-core-nodejs-module');
Expand Down Expand Up @@ -205,12 +206,12 @@ exports.Bundler = class {
}

// process normally if result is not recognizable
return this.addNpmResource(d);
return this.addMissingDep(d);
},
// proceed normally after error
err => {
logger.error(err);
return this.addNpmResource(d);
return this.addMissingDep(d);
}
);
}
Expand Down Expand Up @@ -239,6 +240,23 @@ exports.Bundler = class {
return this.bundles.reduce((a, b) => a.concat(b.getDependencyInclusions()), []);
}

addMissingDep(id) {
const localFilePath = path.resolve(this.project.paths.root, id);

// load additional local file missed by gulp tasks,
// this could be json/yaml file that user wanted in
// aurelia.json 'text!' plugin
if (Utils.couldMissGulpPreprocess(id) && fs.existsSync(localFilePath)) {
this.addFile({
path: localFilePath,
contents: fs.readFileSync(localFilePath)
});
return Promise.resolve();
}

return this.addNpmResource(id);
}

addNpmResource(id) {
// match scoped npm module and normal npm module
const match = id.match(/^((?:@[^/]+\/[^/]+)|(?:[^@][^/]*))(\/.+)?$/);
Expand Down
5 changes: 5 additions & 0 deletions lib/build/utils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@ const tmpDir = require('os').tmpdir();

exports.knownExtensions = ['.js', '.json', '.css', '.svg', '.html'];

exports.couldMissGulpPreprocess = function(id) {
const ext = path.extname(id).toLowerCase();
return ext && ext !== '.js' && ext !== '.html' && ext !== '.css';
};

// require.resolve(packageName) cannot resolve package has no main.
// for instance: font-awesome v4.7.0
// manually try resolve paths
Expand Down
2 changes: 1 addition & 1 deletion package-lock.json
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions spec/lib/build/bundled-source.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ describe('the BundledSource module', () => {
it('transforms local js file', () => {
let file = {
path: path.resolve(cwd, 'src/foo/bar/loo.js'),
contents: "define(['./a', 'lo/rem', 'text!./loo.html', 'text!foo.html'], function(a,r){});"
contents: "define(['./a', 'lo/rem', 'text!./loo.yaml', 'text!foo.html'], function(a,r){});"
};

let bs = new BundledSource(bundler, file);
Expand All @@ -120,9 +120,9 @@ describe('the BundledSource module', () => {
bs._getUseCache = () => undefined;

let deps = bs.transform();
expect(deps).toEqual(['lo/rem', 'foo.html']); // relative dep is ignored
expect(deps).toEqual(['lo/rem', 'foo/bar/loo.yaml', 'foo.html']); // relative dep is ignored
expect(bs.requiresTransform).toBe(false);
expect(bs.contents).toBe("define('foo/bar/loo',['./a', 'lo/rem', 'text!./loo.html', 'text!foo.html'], function(a,r){});");
expect(bs.contents).toBe("define('foo/bar/loo',['./a', 'lo/rem', 'text!./loo.yaml', 'text!foo.html'], function(a,r){});");
expect(bundler.configTargetBundle.addAlias).toHaveBeenCalledWith('b8/loo', 'foo/bar/loo');
});

Expand Down
91 changes: 71 additions & 20 deletions spec/lib/build/bundler.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,19 @@ const CLIOptionsMock = require('../../mocks/cli-options');
describe('the Bundler module', () => {
let analyzer;
let cliOptionsMock;
let mockfs;

beforeEach(() => {
mockfs = require('mock-fs');
analyzer = new PackageAnalyzer();
cliOptionsMock = new CLIOptionsMock();
cliOptionsMock.attach();
});

afterEach(() => {
mockfs.restore();
});

it('uses paths.root from aurelia.json in the loaderConfig as baseUrl', () => {
let project = {
paths: {
Expand Down Expand Up @@ -171,7 +177,52 @@ describe('the Bundler module', () => {
});
});

it('addNpmResource traces npm package', done => {
it('addMissingDep traces local missing file', done => {
let project = {
paths: {
root: 'src',
foo: 'bar'
},
build: { loader: {} }
};

const fsConfig = {};
fsConfig['src/lorem/lo.yaml'] = 'a: 1';
mockfs(fsConfig);

analyzer.analyze = nodeId => Promise.resolve({
name: nodeId,
loaderConfig: {
name: nodeId,
path: `../node_modules/${nodeId}`,
main: 'index'
}
});

let bundler = new Bundler(project, analyzer);
bundler.addFile = jasmine.createSpy('addFile').and.returnValue(null);

let depInclusion = {};

bundler.configTargetBundle = {
addDependency: jasmine.createSpy('addDependency')
.and.returnValue(Promise.resolve(depInclusion))
};

bundler.addMissingDep('lorem/lo.yaml')
.then(() => {
expect(bundler.configTargetBundle.addDependency)
.not.toHaveBeenCalled();
expect(bundler.addFile).toHaveBeenCalledWith({
path: path.resolve('src/lorem/lo.yaml'),
contents: 'a: 1'
});
done();
})
.catch(e => done.fail(e));
});

it('addMissingDep traces npm package', done => {
let project = {
paths: {
root: 'src',
Expand All @@ -198,7 +249,7 @@ describe('the Bundler module', () => {
.and.returnValue(Promise.resolve(depInclusion))
};

bundler.addNpmResource('lorem')
bundler.addMissingDep('lorem')
.then(() => {
expect(bundler.configTargetBundle.addDependency)
.toHaveBeenCalled();
Expand All @@ -216,7 +267,7 @@ describe('the Bundler module', () => {
.catch(e => done.fail(e));
});

it('addNpmResource traces npm package and additional js', done => {
it('addMissingDep traces npm package and additional js', done => {
let project = {
paths: {
root: 'src',
Expand Down Expand Up @@ -245,7 +296,7 @@ describe('the Bundler module', () => {
.and.returnValue(Promise.resolve(depInclusion))
};

bundler.addNpmResource('lorem/foo/bar')
bundler.addMissingDep('lorem/foo/bar')
.then(() => {
expect(bundler.configTargetBundle.addDependency)
.toHaveBeenCalled();
Expand All @@ -267,7 +318,7 @@ describe('the Bundler module', () => {
.catch(e => done.fail(e));
});

it('addNpmResource traces npm package and additional other resource', done => {
it('addMissingDep traces npm package and additional other resource', done => {
let project = {
paths: {
root: 'src',
Expand Down Expand Up @@ -296,7 +347,7 @@ describe('the Bundler module', () => {
.and.returnValue(Promise.resolve(depInclusion))
};

bundler.addNpmResource('lorem/foo/bar.css')
bundler.addMissingDep('lorem/foo/bar.css')
.then(() => {
expect(bundler.configTargetBundle.addDependency)
.toHaveBeenCalled();
Expand All @@ -318,7 +369,7 @@ describe('the Bundler module', () => {
.catch(e => done.fail(e));
});

it('addNpmResource traces scoped npm package', done => {
it('addMissingDep traces scoped npm package', done => {
let project = {
paths: {
root: 'src',
Expand All @@ -345,7 +396,7 @@ describe('the Bundler module', () => {
.and.returnValue(Promise.resolve(depInclusion))
};

bundler.addNpmResource('@scope/lorem')
bundler.addMissingDep('@scope/lorem')
.then(() => {
expect(bundler.configTargetBundle.addDependency)
.toHaveBeenCalled();
Expand All @@ -363,7 +414,7 @@ describe('the Bundler module', () => {
.catch(e => done.fail(e));
});

it('addNpmResource traces scoped npm package and additional js', done => {
it('addMissingDep traces scoped npm package and additional js', done => {
let project = {
paths: {
root: 'src',
Expand Down Expand Up @@ -392,7 +443,7 @@ describe('the Bundler module', () => {
.and.returnValue(Promise.resolve(depInclusion))
};

bundler.addNpmResource('@scope/lorem/foo/bar')
bundler.addMissingDep('@scope/lorem/foo/bar')
.then(() => {
expect(bundler.configTargetBundle.addDependency)
.toHaveBeenCalled();
Expand All @@ -414,7 +465,7 @@ describe('the Bundler module', () => {
.catch(e => done.fail(e));
});

it('addNpmResource traces scoped npm package and additional other resource', done => {
it('addMissingDep traces scoped npm package and additional other resource', done => {
let project = {
paths: {
root: 'src',
Expand Down Expand Up @@ -443,7 +494,7 @@ describe('the Bundler module', () => {
.and.returnValue(Promise.resolve(depInclusion))
};

bundler.addNpmResource('@scope/lorem/foo/bar.css')
bundler.addMissingDep('@scope/lorem/foo/bar.css')
.then(() => {
expect(bundler.configTargetBundle.addDependency)
.toHaveBeenCalled();
Expand All @@ -465,7 +516,7 @@ describe('the Bundler module', () => {
.catch(e => done.fail(e));
});

it('addNpmResource trace main of npm package', done => {
it('addMissingDep trace main of npm package', done => {
let project = {
paths: {
root: 'src',
Expand All @@ -484,15 +535,15 @@ describe('the Bundler module', () => {

bundler.getDependencyInclusions = () => [depInclusion];

bundler.addNpmResource('lorem')
bundler.addMissingDep('lorem')
.then(() => {
expect(depInclusion.traceMain).toHaveBeenCalled();
done();
})
.catch(e => done.fail(e));
});

it('addNpmResource trace resource of npm package', done => {
it('addMissingDep trace resource of npm package', done => {
let project = {
paths: {
root: 'src',
Expand All @@ -511,7 +562,7 @@ describe('the Bundler module', () => {

bundler.getDependencyInclusions = () => [depInclusion];

bundler.addNpmResource('lorem/foo/bar')
bundler.addMissingDep('lorem/foo/bar')
.then(() => {
expect(depInclusion.traceResource).toHaveBeenCalled();
expect(depInclusion.traceResource).toHaveBeenCalledWith('foo/bar');
Expand All @@ -520,7 +571,7 @@ describe('the Bundler module', () => {
.catch(e => done.fail(e));
});

it('addNpmResource trace main of scoped npm package', done => {
it('addMissingDep trace main of scoped npm package', done => {
let project = {
paths: {
root: 'src',
Expand All @@ -539,15 +590,15 @@ describe('the Bundler module', () => {

bundler.getDependencyInclusions = () => [depInclusion];

bundler.addNpmResource('@scope/lorem')
bundler.addMissingDep('@scope/lorem')
.then(() => {
expect(depInclusion.traceMain).toHaveBeenCalled();
done();
})
.catch(e => done.fail(e));
});

it('addNpmResource trace resource of scoped npm package', done => {
it('addMissingDep trace resource of scoped npm package', done => {
let project = {
paths: {
root: 'src',
Expand All @@ -566,7 +617,7 @@ describe('the Bundler module', () => {

bundler.getDependencyInclusions = () => [depInclusion];

bundler.addNpmResource('@scope/lorem/foo/bar')
bundler.addMissingDep('@scope/lorem/foo/bar')
.then(() => {
expect(depInclusion.traceResource).toHaveBeenCalled();
expect(depInclusion.traceResource).toHaveBeenCalledWith('foo/bar');
Expand Down
13 changes: 13 additions & 0 deletions spec/lib/build/util.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,3 +75,16 @@ describe('the Utils.moduleIdWithPlugin function', () => {
});
});

describe('the Utils.couldMissGulpPreprocess function', () => {
it('returns false for js/html/css files', () => {
expect(Utils.couldMissGulpPreprocess('foo/bar')).toBeFalsy();
expect(Utils.couldMissGulpPreprocess('foo/bar.js')).toBeFalsy();
expect(Utils.couldMissGulpPreprocess('foo/bar.html')).toBeFalsy();
expect(Utils.couldMissGulpPreprocess('bar.css')).toBeFalsy();
});

it('returns true for unknown file extension', () => {
expect(Utils.couldMissGulpPreprocess('foo/bar.json')).toBeTruthy();
expect(Utils.couldMissGulpPreprocess('foo/bar.yaml')).toBeTruthy();
});
});

0 comments on commit 4387bff

Please sign in to comment.