Skip to content

Commit

Permalink
Merge a9e959b into de2b50d
Browse files Browse the repository at this point in the history
  • Loading branch information
@HyperBrain
HyperBrain committed May 3, 2018
2 parents de2b50d + a9e959b commit b57066f
Show file tree
Hide file tree
Showing 3 changed files with 215 additions and 15 deletions.
38 changes: 26 additions & 12 deletions lib/packExternalModules.js
View file
Expand Up @@ -58,10 +58,10 @@ function removeExcludedModules(modules, packageForceExcludes, log) {
}

/**
* Resolve the needed versions of production depenencies for external modules.
* Resolve the needed versions of production dependencies for external modules.
* @this - The active plugin instance
*/
function getProdModules(externalModules, packagePath, dependencyGraph) {
function getProdModules(externalModules, packagePath, dependencyGraph, forceExcludes) {
const packageJsonPath = path.join(process.cwd(), packagePath);
const packageJson = require(packageJsonPath);
const prodModules = [];
Expand Down Expand Up @@ -89,20 +89,34 @@ function getProdModules(externalModules, packagePath, dependencyGraph) {
const peerDependencies = require(modulePackagePath).peerDependencies;
if (!_.isEmpty(peerDependencies)) {
this.options.verbose && this.serverless.cli.log(`Adding explicit peers for dependency ${module.external}`);
const peerModules = getProdModules.call(this, _.map(peerDependencies, (value, key) => ({ external: key })), packagePath, dependencyGraph);
const peerModules = getProdModules.call(this, _.map(peerDependencies, (value, key) => ({ external: key })), packagePath, dependencyGraph, forceExcludes);
Array.prototype.push.apply(prodModules, peerModules);
}
} catch (e) {
this.serverless.cli.log(`WARNING: Could not check for peer dependencies of ${module.external}`);
}
} else if (!packageJson.devDependencies || !packageJson.devDependencies[module.external]) {
// Add transient dependencies if they appear not in the service's dev dependencies
const originInfo = _.get(dependencyGraph, 'dependencies', {})[module.origin] || {};
moduleVersion = _.get(_.get(originInfo, 'dependencies', {})[module.external], 'version');
if (!moduleVersion) {
this.serverless.cli.log(`WARNING: Could not determine version of module ${module.external}`);
} else {
if (!packageJson.devDependencies || !packageJson.devDependencies[module.external]) {
// Add transient dependencies if they appear not in the service's dev dependencies
const originInfo = _.get(dependencyGraph, 'dependencies', {})[module.origin] || {};
moduleVersion = _.get(_.get(originInfo, 'dependencies', {})[module.external], 'version');
if (!moduleVersion) {
this.serverless.cli.log(`WARNING: Could not determine version of module ${module.external}`);
}
prodModules.push(moduleVersion ? `${module.external}@${moduleVersion}` : module.external);
} else if (packageJson.devDependencies && packageJson.devDependencies[module.external] && !_.includes(forceExcludes, module.external)) {
// To minimize the chance of breaking setups we whitelist packages available on AWS here. These are due to the previously missing check
// most likely set in devDependencies and should not lead to an error now.
const ignoredDevDependencies = ['aws-sdk'];

if (!_.includes(ignoredDevDependencies, module.external)) {
// Runtime dependency found in devDependencies but not forcefully excluded
this.serverless.cli.log(`ERROR: Runtime dependency '${module.external}' found in devDependencies. Move it to dependencies or use forceExclude to explicitly exclude it.`);
throw new this.serverless.classes.Error(`Serverless-webpack dependency error: ${module.external}.`);
}

this.serverless.cli.log(`WARNING: Runtime dependency '${module.external}' found in devDependencies. You should use forceExclude to explicitly exclude it.`);
}
prodModules.push(moduleVersion ? `${module.external}@${moduleVersion}` : module.external);
}
});

Expand Down Expand Up @@ -220,7 +234,7 @@ module.exports = {
getExternalModules.call(this, compileStats),
_.map(packageForceIncludes, whitelistedPackage => ({ external: whitelistedPackage }))
);
return getProdModules.call(this, externalModules, packagePath, dependencyGraph);
return getProdModules.call(this, externalModules, packagePath, dependencyGraph, packageForceExcludes);
}));
removeExcludedModules.call(this, compositeModules, packageForceExcludes, true);

Expand Down Expand Up @@ -292,7 +306,7 @@ module.exports = {
_.concat(
getExternalModules.call(this, compileStats),
_.map(packageForceIncludes, whitelistedPackage => ({ external: whitelistedPackage }))
), packagePath, dependencyGraph);
), packagePath, dependencyGraph, packageForceExcludes);
removeExcludedModules.call(this, prodModules, packageForceExcludes);
const relPath = path.relative(modulePath, path.dirname(packageJsonPath));
addModulesToPackageJson(prodModules, modulePackage, relPath);
Expand Down
33 changes: 33 additions & 0 deletions tests/mocks/packageIgnoredDevDeps.mock.json
View file
@@ -0,0 +1,33 @@
{
"dependencies": {
"archiver": "^2.0.0",
"bluebird": "^3.4.0",
"fs-extra": "^0.26.7",
"glob": "^7.1.2",
"lodash": "^4.17.4",
"npm-programmatic": "0.0.5",
"uuid": "^5.4.1",
"ts-node": "^3.2.0",
"@scoped/vendor": "1.0.0",
"pg": "^4.3.5"
},
"devDependencies": {
"aws-sdk": "^2.10.1",
"babel-eslint": "^7.2.3",
"chai": "^4.1.0",
"chai-as-promised": "^7.1.1",
"eslint": "^4.3.0",
"eslint-plugin-import": "^2.7.0",
"eslint-plugin-lodash": "^2.4.4",
"eslint-plugin-promise": "^3.5.0",
"istanbul": "^0.4.5",
"mocha": "^3.4.2",
"mockery": "^2.1.0",
"serverless": "^1.17.0",
"sinon": "^2.3.8",
"sinon-chai": "^2.12.0"
},
"peerDependencies": {
"webpack": "*"
}
}
159 changes: 156 additions & 3 deletions tests/packExternalModules.test.js
View file
Expand Up @@ -13,6 +13,7 @@ const Configuration = require('../lib/Configuration');
const fsExtraMockFactory = require('./mocks/fs-extra.mock');
const packageMock = require('./mocks/package.mock.json');
const packageLocalRefMock = require('./mocks/packageLocalRef.mock.json');
const packageIgnoredDevDepsMock = require('./mocks/packageIgnoredDevDeps.mock.json');

chai.use(require('chai-as-promised'));
chai.use(require('sinon-chai'));
Expand Down Expand Up @@ -136,9 +137,6 @@ describe('packExternalModules', () => {
{
identifier: _.constant('"uuid/v4"')
},
{
identifier: _.constant('external "eslint"')
},
{
identifier: _.constant('"mockery"')
},
Expand Down Expand Up @@ -191,6 +189,45 @@ describe('packExternalModules', () => {
]
};
const statsWithFileRef = {
stats: [
{
compilation: {
chunks: [
new ChunkMock([
{
identifier: _.constant('"crypto"')
},
{
identifier: _.constant('"uuid/v4"')
},
{
identifier: _.constant('"mockery"')
},
{
identifier: _.constant('"@scoped/vendor/module1"')
},
{
identifier: _.constant('external "@scoped/vendor/module2"')
},
{
identifier: _.constant('external "uuid/v4"')
},
{
identifier: _.constant('external "localmodule"')
},
{
identifier: _.constant('external "bluebird"')
},
])
],
compiler: {
outputPath: '/my/Service/Path/.webpack/service'
}
}
}
]
};
const statsWithDevDependency = {
stats: [
{
compilation: {
Expand Down Expand Up @@ -232,6 +269,48 @@ describe('packExternalModules', () => {
}
]
};
const statsWithIgnoredDevDependency = {
stats: [
{
compilation: {
chunks: [
new ChunkMock([
{
identifier: _.constant('"crypto"')
},
{
identifier: _.constant('"uuid/v4"')
},
{
identifier: _.constant('"mockery"')
},
{
identifier: _.constant('"@scoped/vendor/module1"')
},
{
identifier: _.constant('external "@scoped/vendor/module2"')
},
{
identifier: _.constant('external "uuid/v4"')
},
{
identifier: _.constant('external "localmodule"')
},
{
identifier: _.constant('external "bluebird"')
},
{
identifier: _.constant('external "aws-sdk"')
},
])
],
compiler: {
outputPath: '/my/Service/Path/.webpack/service'
}
}
}
]
};

it('should do nothing if webpackIncludeModules is not set', () => {
module.configuration = new Configuration();
Expand Down Expand Up @@ -760,6 +839,80 @@ describe('packExternalModules', () => {
]));
});

it('should reject if devDependency is required at runtime', () => {
module.webpackOutputPath = 'outputPath';
fsExtraMock.pathExists.yields(null, false);
fsExtraMock.copy.yields();
packagerMock.getProdDependencies.returns(BbPromise.resolve({}));
packagerMock.install.returns(BbPromise.resolve());
packagerMock.prune.returns(BbPromise.resolve());
packagerMock.runScripts.returns(BbPromise.resolve());
module.compileStats = statsWithDevDependency;
return expect(module.packExternalModules()).to.be.rejectedWith('Serverless-webpack dependency error: eslint.')
.then(() => BbPromise.all([
expect(module.serverless.cli.log).to.have.been.calledWith(sinon.match(/ERROR: Runtime dependency 'eslint' found in devDependencies/)),
// npm ls and npm install should have been called
expect(packagerMock.getProdDependencies).to.have.been.calledOnce,
expect(packagerMock.install).to.not.have.been.called,
expect(packagerMock.prune).to.not.have.been.called,
expect(packagerMock.runScripts).to.not.have.been.called,
]));
});

it('should ignore aws-sdk if set only in devDependencies', () => {
module.configuration = new Configuration({
webpack: {
includeModules: {
packagePath: path.join('ignoreDevDeps', 'package.json')
}
}
});
module.webpackOutputPath = 'outputPath';
fsExtraMock.pathExists.yields(null, false);
fsExtraMock.copy.yields();
packagerMock.getProdDependencies.returns(BbPromise.resolve({}));
packagerMock.install.returns(BbPromise.resolve());
packagerMock.prune.returns(BbPromise.resolve());
packagerMock.runScripts.returns(BbPromise.resolve());
module.compileStats = statsWithIgnoredDevDependency;
mockery.registerMock(path.join(process.cwd(), 'ignoreDevDeps', 'package.json'), packageIgnoredDevDepsMock);
return expect(module.packExternalModules()).to.be.fulfilled
.then(() => BbPromise.all([
expect(module.serverless.cli.log).to.have.been.calledWith(sinon.match(/WARNING: Runtime dependency 'aws-sdk' found in devDependencies/)),
// npm ls and npm install should have been called
expect(packagerMock.getProdDependencies).to.have.been.calledOnce,
expect(packagerMock.install).to.have.been.calledOnce,
expect(packagerMock.prune).to.have.been.calledOnce,
expect(packagerMock.runScripts).to.have.been.calledOnce,
]));
});

it('should succeed if devDependency is required at runtime but forcefully excluded', () => {
module.configuration = new Configuration({
webpack: {
includeModules: {
forceExclude: ['eslint']
}
}
});
module.webpackOutputPath = 'outputPath';
fsExtraMock.pathExists.yields(null, false);
fsExtraMock.copy.yields();
packagerMock.getProdDependencies.returns(BbPromise.resolve({}));
packagerMock.install.returns(BbPromise.resolve());
packagerMock.prune.returns(BbPromise.resolve());
packagerMock.runScripts.returns(BbPromise.resolve());
module.compileStats = statsWithDevDependency;
return expect(module.packExternalModules()).to.be.fulfilled
.then(() => BbPromise.all([
// npm ls and npm install should have been called
expect(packagerMock.getProdDependencies).to.have.been.calledOnce,
expect(packagerMock.install).to.have.been.calledOnce,
expect(packagerMock.prune).to.have.been.calledOnce,
expect(packagerMock.runScripts).to.have.been.calledOnce,
]));
});

it('should read package-lock if found', () => {
const expectedCompositePackageJSON = {
name: 'test-service',
Expand Down

0 comments on commit b57066f

Please sign in to comment.