Skip to content

Commit

Permalink
Do not add .js extension to entries values (#167)
Browse files Browse the repository at this point in the history
* Do not add .js extension to entries values

* Find entries extension from filename

* Better filesearch performance

* Add warning for multiple files found

* Do not match directories

* Added unit tests. Throw error if handler is not found.

* Fixed examples

* Use preference sorting to find the most probable handler for multiple hits
  • Loading branch information
kikar authored and HyperBrain committed Jul 28, 2017
1 parent ceaf43c commit e7531dc
Show file tree
Hide file tree
Showing 5 changed files with 184 additions and 37 deletions.
2 changes: 1 addition & 1 deletion examples/babel-dynamically-entries/webpack.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,6 @@ module.exports = {
output: {
libraryTarget: 'commonjs',
path: path.join(__dirname, '.webpack'),
filename: '[name]'
filename: '[name].js'
}
};
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,6 @@ module.exports = {
output: {
libraryTarget: 'commonjs',
path: path.join(__dirname, '.webpack'),
filename: '[name]'
filename: '[name].js'
}
};
62 changes: 52 additions & 10 deletions lib/validate.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,21 +3,63 @@
const BbPromise = require('bluebird');
const path = require('path');
const fse = require('fs-extra');
const glob = require('glob');
const lib = require('./index');
const _ = require('lodash');

function getEntryForFunction(serverlessFunction) {
const handler = serverlessFunction.handler;
const handlerFile = /(.*)\..*?$/.exec(handler)[1] + '.js';

// Create a valid entry key
return {
[handlerFile]: `./${handlerFile}`
};
};
/**
* For automatic entry detection we sort the found files to solve ambiguities.
* This should cover most of the cases. For complex setups the user should
* build his own entries with help of the other exports.
*/
const preferredExtensions = [
'.js',
'.ts',
'.jsx'
];

module.exports = {
validate() {
const getEntryExtension = fileName => {
const files = glob.sync(`${fileName}.*`, {
cwd: this.serverless.config.servicePath,
nodir: true
});

if (_.isEmpty(files)) {
// If we cannot find any handler we should terminate with an error
throw new this.serverless.classes.Error(`No matching handler found for '${fileName}'. Check your service definition.`);
}

// Move preferred file extensions to the beginning
const sortedFiles = _.uniq(
_.concat(
_.sortBy(
_.filter(files, file => _.includes(preferredExtensions, path.extname(file))),
a => _.size(a)
),
files
)
);

if (_.size(sortedFiles) > 1) {
this.serverless.cli.log(`WARNING: More than one matching handlers found for '${fileName}'. Using '${_.first(sortedFiles)}'.`);
}
return path.extname(_.first(sortedFiles));
}

const getEntryForFunction = serverlessFunction => {
const handler = serverlessFunction.handler;
const handlerFile = /(.*)\..*?$/.exec(handler)[1];

const ext = getEntryExtension(handlerFile);

// Create a valid entry key
return {
[handlerFile]: `./${handlerFile}${ext}`
};
};

this.webpackConfig = (
this.serverless.service.custom &&
this.serverless.service.custom.webpack ||
Expand All @@ -38,7 +80,7 @@ module.exports = {
});
}
lib.entries = entries;

// Expose service file and options
lib.serverless = this.serverless;
lib.options = this.options;
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
"body-parser": "^1.15.2",
"express": "^4.14.0",
"fs-extra": "^0.26.7",
"glob": "^7.1.2",
"lodash": "^4.17.4",
"npm-programmatic": "0.0.5",
"ts-node": "^3.2.0"
Expand Down
154 changes: 129 additions & 25 deletions tests/validate.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,24 @@ const makeFsExtraMock = require('./fs-extra.mock');
chai.use(require('sinon-chai'));
const expect = chai.expect;

const globMock = {
sync() {}
};

describe('validate', () => {
let fsExtraMock;
let baseModule;
let module;
let serverless;
let sandbox;

before(() => {
sandbox = sinon.sandbox.create();

mockery.enable({ warnOnUnregistered: false });
fsExtraMock = makeFsExtraMock();
mockery.registerMock('fs-extra', fsExtraMock);
mockery.registerMock('glob', globMock);
baseModule = require('../lib/validate');
Object.freeze(baseModule);
});
Expand All @@ -31,13 +39,20 @@ describe('validate', () => {

beforeEach(() => {
serverless = new Serverless();
serverless.cli = {
log: sandbox.stub()
};
fsExtraMock._resetSpies();
module = Object.assign({
serverless,
options: {},
}, baseModule);
});

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

it('should expose a `validate` method', () => {
expect(module.validate).to.be.a('function');
});
Expand Down Expand Up @@ -266,6 +281,12 @@ describe('validate', () => {
});

describe('entries', () => {
let globSyncStub;

beforeEach(() => {
globSyncStub = sandbox.stub(globMock, 'sync');
});

const testFunctionsConfig = {
func1: {
handler: 'module1.func1handler',
Expand Down Expand Up @@ -305,7 +326,7 @@ describe('validate', () => {
},
};

it('should expose entries from serverless.yml if `options.function` is not defined', () => {
it('should expose all functions if `options.function` is not defined', () => {
const testOutPath = 'test';
const testConfig = {
entry: 'test',
Expand All @@ -316,22 +337,24 @@ describe('validate', () => {
};
module.serverless.service.custom.webpack = testConfig;
module.serverless.service.functions = testFunctionsConfig;
return module
.validate()
.then(() => {
const lib = require('../lib/index');
const expectedLibEntries = {
'module1.js': './module1.js',
'module2.js': './module2.js',
'handlers/func3/module2.js': './handlers/func3/module2.js',
'handlers/module2/func3/module2.js': './handlers/module2/func3/module2.js',
};

expect(lib.entries).to.deep.eq(expectedLibEntries)
});
globSyncStub.callsFake(filename => [ _.replace(filename, '*', 'js') ]);
return expect(module.validate()).to.be.fulfilled
.then(() => {
const lib = require('../lib/index');
const expectedLibEntries = {
'module1': './module1.js',
'module2': './module2.js',
'handlers/func3/module2': './handlers/func3/module2.js',
'handlers/module2/func3/module2': './handlers/module2/func3/module2.js',
};

expect(lib.entries).to.deep.equal(expectedLibEntries);
expect(globSyncStub).to.have.callCount(4);
expect(serverless.cli.log).to.not.have.been.called;
});
});

it('should expose entries with `options.function` value if `options.function` is defined and found in entries from serverless.yml', () => {
it('should expose the requested function if `options.function` is defined and the function is found', () => {
const testOutPath = 'test';
const testFunction = 'func1';
const testConfig = {
Expand All @@ -344,16 +367,97 @@ describe('validate', () => {
module.serverless.service.custom.webpack = testConfig;
module.serverless.service.functions = testFunctionsConfig;
module.options.function = testFunction;
return module
.validate()
.then(() => {
const lib = require('../lib/index');
const expectedLibEntries = {
'module1.js': './module1.js'
};

expect(lib.entries).to.deep.eq(expectedLibEntries)
});
globSyncStub.callsFake(filename => [ _.replace(filename, '*', 'js') ]);
return expect(module.validate()).to.be.fulfilled
.then(() => {
const lib = require('../lib/index');
const expectedLibEntries = {
'module1': './module1.js'
};

expect(lib.entries).to.deep.equal(expectedLibEntries)
expect(globSyncStub).to.have.been.calledOnce;
expect(serverless.cli.log).to.not.have.been.called;
});
});

it('should show a warning if more than one matching handler is found', () => {
const testOutPath = 'test';
const testFunction = 'func1';
const testConfig = {
entry: 'test',
context: 'testcontext',
output: {
path: testOutPath,
},
};
module.serverless.service.custom.webpack = testConfig;
module.serverless.service.functions = testFunctionsConfig;
module.options.function = testFunction;
globSyncStub.returns([ 'module1.ts', 'module1.js' ]);
return expect(module.validate()).to.be.fulfilled
.then(() => {
const lib = require('../lib/index');
const expectedLibEntries = {
'module1': './module1.ts'
};

expect(lib.entries).to.deep.equal(expectedLibEntries)
expect(globSyncStub).to.have.been.calledOnce;
expect(serverless.cli.log).to.have.been.calledOnce;
expect(serverless.cli.log).to.have.been.calledWith(
'WARNING: More than one matching handlers found for \'module1\'. Using \'module1.ts\'.'
);
});
});

it('should select the most probable handler if multiple hits are found', () => {
const testOutPath = 'test';
const testFunction = 'func1';
const testConfig = {
entry: 'test',
context: 'testcontext',
output: {
path: testOutPath,
},
};
module.serverless.service.custom.webpack = testConfig;
module.serverless.service.functions = testFunctionsConfig;
module.options.function = testFunction;
globSyncStub.returns([ 'module1.doc', 'module1.json', 'module1.test.js', 'module1.ts', 'module1.js' ]);
return expect(module.validate()).to.be.fulfilled
.then(() => {
const lib = require('../lib/index');
const expectedLibEntries = {
'module1': './module1.ts'
};

expect(lib.entries).to.deep.equal(expectedLibEntries)
expect(globSyncStub).to.have.been.calledOnce;
expect(serverless.cli.log).to.have.been.calledOnce;
expect(serverless.cli.log).to.have.been.calledWith(
'WARNING: More than one matching handlers found for \'module1\'. Using \'module1.ts\'.'
);
});
});

it('should throw an exception if no handler is found', () => {
const testOutPath = 'test';
const testFunction = 'func1';
const testConfig = {
entry: 'test',
context: 'testcontext',
output: {
path: testOutPath,
},
};
module.serverless.service.custom.webpack = testConfig;
module.serverless.service.functions = testFunctionsConfig;
module.options.function = testFunction;
globSyncStub.returns([]);
expect(() => {
module.validate();
}).to.throw(/No matching handler found for/);
});

it('should throw an exception if `options.function` is defined but not found in entries from serverless.yml', () => {
Expand Down

0 comments on commit e7531dc

Please sign in to comment.