diff --git a/lib/plugins/package/lib/zipService.js b/lib/plugins/package/lib/zipService.js index 6837ff66d..582f683cc 100644 --- a/lib/plugins/package/lib/zipService.js +++ b/lib/plugins/package/lib/zipService.js @@ -35,6 +35,8 @@ module.exports = { } if (excludeDevDependencies) { + this.serverless.cli.log('Excluding development dependencies...'); + return BbPromise.bind(this) .then(() => excludeNodeDevDependencies(servicePath)) .then((exAndInNode) => { @@ -125,11 +127,11 @@ function excludeNodeDevDependencies(servicePath) { exclude: [], }; - // the file where we'll write the dependencies into - const nodeDevDepFile = path.join( - os.tmpdir(), - `node-dev-dependencies-${crypto.randomBytes(8).toString('hex')}` - ); + // the files where we'll write the dependencies into + const tmpDir = os.tmpdir(); + const randHash = crypto.randomBytes(8).toString('hex'); + const nodeDevDepFile = path.join(tmpDir, `node-dependencies-${randHash}-dev`); + const nodeProdDepFile = path.join(tmpDir, `node-dependencies-${randHash}-prod`); try { const packageJsonFilePaths = globby.sync([ @@ -161,17 +163,29 @@ function excludeNodeDevDependencies(servicePath) { // we added a catch which resolves so that npm commands with an exit code of 1 // (e.g. if the package.json is invalid) won't crash the dev dependency exclusion process - return childProcess.execAsync( - `npm ls --dev=true --parseable=true --silent >> ${nodeDevDepFile}`, - { cwd: dirWithPackageJson } - ).catch(() => BbPromise.resolve()); + return BbPromise.map(['dev', 'prod'], (env) => { + const depFile = env === 'dev' ? nodeDevDepFile : nodeProdDepFile; + return childProcess.execAsync( + `npm ls --${env}=true --parseable=true --silent >> ${depFile}`, + { cwd: dirWithPackageJson } + ).catch(() => BbPromise.resolve()); + }); }) - .then(() => fs.readFileAsync(nodeDevDepFile)) - .then((fileContent) => { - const dependencies = _.compact( - (_.uniq(_.split(fileContent.toString('utf8'), '\n'))), - elem => elem.length > 0 - ); + // NOTE: using mapSeries here for a sequential computation (w/o race conditions) + .then(() => BbPromise.mapSeries(['dev', 'prod'], (env) => { + const depFile = env === 'dev' ? nodeDevDepFile : nodeProdDepFile; + return fs.readFileAsync(depFile) + .then((fileContent) => _.compact( + (_.uniq(_.split(fileContent.toString('utf8'), '\n'))), + elem => elem.length > 0 + )).catch(() => BbPromise.resolve()); + })) + .then((devAndProDependencies) => { + const devDependencies = devAndProDependencies[0]; + const prodDependencies = devAndProDependencies[1]; + + // NOTE: the order for _.difference is important + const dependencies = _.difference(devDependencies, prodDependencies); const nodeModulesRegex = new RegExp(`${path.join('node_modules', path.sep)}.*`, 'g'); if (!_.isEmpty(dependencies)) { diff --git a/lib/plugins/package/lib/zipService.test.js b/lib/plugins/package/lib/zipService.test.js index a304bf861..8727ad496 100644 --- a/lib/plugins/package/lib/zipService.test.js +++ b/lib/plugins/package/lib/zipService.test.js @@ -136,8 +136,8 @@ describe('zipService', () => { return expect(packagePlugin.excludeDevDependencies(params)).to.be .fulfilled.then((updatedParams) => { expect(globbySyncStub).to.have.been.calledOnce; - expect(execAsyncStub).to.have.been.calledOnce; - expect(readFileAsyncStub).to.have.been.calledOnce; + expect(execAsyncStub).to.have.been.calledTwice; + expect(readFileAsyncStub).to.have.been.calledTwice; expect(globbySyncStub).to.have.been .calledWithExactly(['**/package.json'], { cwd: packagePlugin.serverless.config.servicePath, @@ -150,6 +150,10 @@ describe('zipService', () => { .match(/npm ls --dev=true --parseable=true --silent >> .+/); expect(execAsyncStub.args[0][1].cwd).to .match(/.+/); + expect(execAsyncStub.args[1][0]).to + .match(/npm ls --prod=true --parseable=true --silent >> .+/); + expect(execAsyncStub.args[1][1].cwd).to + .match(/.+/); expect(updatedParams.exclude).to.deep.equal([ 'user-defined-exclude-me', ]); @@ -178,18 +182,42 @@ describe('zipService', () => { }); }); - it('should return excludes and includes if a Promise is rejected in the chain', () => { + it('should return excludes and includes if a exec Promise is rejected', () => { const filePaths = ['package.json', 'node_modules']; globbySyncStub.returns(filePaths); - execAsyncStub.resolves(); - readFileAsyncStub.rejects(); + execAsyncStub.onCall(0).resolves(); + execAsyncStub.onCall(1).rejects(); + readFileAsyncStub.resolves(); return expect(packagePlugin.excludeDevDependencies(params)).to.be .fulfilled.then((updatedParams) => { expect(globbySyncStub).to.been.calledOnce; - expect(execAsyncStub).to.have.been.calledOnce; - expect(readFileAsyncStub).to.have.been.calledOnce; + expect(execAsyncStub).to.have.been.calledTwice; + expect(readFileAsyncStub).to.have.been.calledTwice; + expect(updatedParams.exclude).to.deep.equal([ + 'user-defined-exclude-me', + ]); + expect(updatedParams.include).to.deep.equal([ + 'user-defined-include-me', + ]); + expect(updatedParams.zipFileName).to.equal(params.zipFileName); + }); + }); + + it('should return excludes and includes if a readFile Promise is rejected', () => { + const filePaths = ['package.json', 'node_modules']; + + globbySyncStub.returns(filePaths); + execAsyncStub.resolves(); + readFileAsyncStub.onCall(0).resolves(); + readFileAsyncStub.onCall(1).rejects(); + + return expect(packagePlugin.excludeDevDependencies(params)).to.be + .fulfilled.then((updatedParams) => { + expect(globbySyncStub).to.been.calledOnce; + expect(execAsyncStub).to.have.been.calledTwice; + expect(readFileAsyncStub).to.have.been.calledTwice; expect(updatedParams.exclude).to.deep.equal([ 'user-defined-exclude-me', ]); @@ -215,21 +243,25 @@ describe('zipService', () => { globbySyncStub.returns(filePaths); execAsyncStub.onCall(0).resolves(); - execAsyncStub.onCall(1).rejects(); - execAsyncStub.onCall(2).resolves(); + execAsyncStub.onCall(1).resolves(); + execAsyncStub.onCall(2).rejects(); + execAsyncStub.onCall(3).rejects(); + execAsyncStub.onCall(4).resolves(); + execAsyncStub.onCall(5).resolves(); const depPaths = [ `${servicePath}/node_modules/module-1`, `${servicePath}/node_modules/module-2`, `${servicePath}/1st/2nd/node_modules/module-1`, `${servicePath}/1st/2nd/node_modules/module-2`, ].join('\n'); - readFileAsyncStub.resolves(depPaths); + readFileAsyncStub.withArgs(sinon.match(/dev$/)).resolves(depPaths); + readFileAsyncStub.withArgs(sinon.match(/prod$/)).resolves([]); return expect(packagePlugin.excludeDevDependencies(params)).to.be .fulfilled.then((updatedParams) => { expect(globbySyncStub).to.have.been.calledOnce; - expect(execAsyncStub.callCount).to.equal(3); - expect(readFileAsyncStub).to.have.been.calledOnce; + expect(execAsyncStub.callCount).to.equal(6); + expect(readFileAsyncStub).to.have.been.calledTwice; expect(globbySyncStub).to.have.been .calledWithExactly(['**/package.json'], { cwd: packagePlugin.serverless.config.servicePath, @@ -243,13 +275,25 @@ describe('zipService', () => { expect(execAsyncStub.args[0][1].cwd).to .match(/.+/); expect(execAsyncStub.args[1][0]).to - .match(/npm ls --dev=true --parseable=true --silent >> .+/); + .match(/npm ls --prod=true --parseable=true --silent >> .+/); expect(execAsyncStub.args[1][1].cwd).to .match(/.+/); expect(execAsyncStub.args[2][0]).to .match(/npm ls --dev=true --parseable=true --silent >> .+/); expect(execAsyncStub.args[2][1].cwd).to .match(/.+/); + expect(execAsyncStub.args[3][0]).to + .match(/npm ls --prod=true --parseable=true --silent >> .+/); + expect(execAsyncStub.args[3][1].cwd).to + .match(/.+/); + expect(execAsyncStub.args[4][0]).to + .match(/npm ls --dev=true --parseable=true --silent >> .+/); + expect(execAsyncStub.args[4][1].cwd).to + .match(/.+/); + expect(execAsyncStub.args[5][0]).to + .match(/npm ls --prod=true --parseable=true --silent >> .+/); + expect(execAsyncStub.args[5][1].cwd).to + .match(/.+/); expect(updatedParams.exclude).to.deep.equal([ 'user-defined-exclude-me', 'node_modules/module-1/**', @@ -274,13 +318,14 @@ describe('zipService', () => { `${servicePath}/node_modules/module-1`, `${servicePath}/node_modules/module-2`, ].join('\n'); - readFileAsyncStub.resolves(depPaths); + readFileAsyncStub.withArgs(sinon.match(/dev$/)).resolves(depPaths); + readFileAsyncStub.withArgs(sinon.match(/prod$/)).resolves([]); return expect(packagePlugin.excludeDevDependencies(params)).to.be .fulfilled.then((updatedParams) => { expect(globbySyncStub).to.have.been.calledOnce; - expect(execAsyncStub).to.have.been.calledOnce; - expect(readFileAsyncStub).to.have.been.calledOnce; + expect(execAsyncStub).to.have.been.calledTwice; + expect(readFileAsyncStub).to.have.been.calledTwice; expect(globbySyncStub).to.have.been .calledWithExactly(['**/package.json'], { cwd: packagePlugin.serverless.config.servicePath, @@ -293,6 +338,10 @@ describe('zipService', () => { .match(/npm ls --dev=true --parseable=true --silent >> .+/); expect(execAsyncStub.args[0][1].cwd).to .match(/.+/); + expect(execAsyncStub.args[1][0]).to + .match(/npm ls --prod=true --parseable=true --silent >> .+/); + expect(execAsyncStub.args[1][1].cwd).to + .match(/.+/); expect(updatedParams.exclude).to.deep.equal([ 'user-defined-exclude-me', 'node_modules/module-1/**', @@ -328,12 +377,14 @@ describe('zipService', () => { `${servicePath}/1st/2nd/node_modules/module-2`, ].join('\n'); readFileAsyncStub.resolves(depPaths); + readFileAsyncStub.withArgs(sinon.match(/dev$/)).resolves(depPaths); + readFileAsyncStub.withArgs(sinon.match(/prod$/)).resolves([]); return expect(packagePlugin.excludeDevDependencies(params)).to.be .fulfilled.then((updatedParams) => { expect(globbySyncStub).to.have.been.calledOnce; - expect(execAsyncStub.callCount).to.equal(3); - expect(readFileAsyncStub).to.have.been.calledOnce; + expect(execAsyncStub.callCount).to.equal(6); + expect(readFileAsyncStub).to.have.been.calledTwice; expect(globbySyncStub).to.have.been .calledWithExactly(['**/package.json'], { cwd: packagePlugin.serverless.config.servicePath, @@ -347,13 +398,25 @@ describe('zipService', () => { expect(execAsyncStub.args[0][1].cwd).to .match(/.+/); expect(execAsyncStub.args[1][0]).to - .match(/npm ls --dev=true --parseable=true --silent >> .+/); + .match(/npm ls --prod=true --parseable=true --silent >> .+/); expect(execAsyncStub.args[1][1].cwd).to .match(/.+/); expect(execAsyncStub.args[2][0]).to .match(/npm ls --dev=true --parseable=true --silent >> .+/); expect(execAsyncStub.args[2][1].cwd).to .match(/.+/); + expect(execAsyncStub.args[3][0]).to + .match(/npm ls --prod=true --parseable=true --silent >> .+/); + expect(execAsyncStub.args[3][1].cwd).to + .match(/.+/); + expect(execAsyncStub.args[4][0]).to + .match(/npm ls --dev=true --parseable=true --silent >> .+/); + expect(execAsyncStub.args[4][1].cwd).to + .match(/.+/); + expect(execAsyncStub.args[5][0]).to + .match(/npm ls --prod=true --parseable=true --silent >> .+/); + expect(execAsyncStub.args[5][1].cwd).to + .match(/.+/); expect(updatedParams.exclude).to.deep.equal([ 'user-defined-exclude-me', 'node_modules/module-1/**', @@ -369,6 +432,55 @@ describe('zipService', () => { expect(updatedParams.zipFileName).to.equal(params.zipFileName); }); }); + + it('should not include packages if in both dependencies and devDependencies', () => { + const filePaths = ['package.json', 'node_modules']; + + globbySyncStub.returns(filePaths); + execAsyncStub.resolves(); + + const devDepPaths = [ + `${servicePath}/node_modules/module-1`, + `${servicePath}/node_modules/module-2`, + ].join('\n'); + readFileAsyncStub.withArgs(sinon.match(/dev$/)).resolves(devDepPaths); + + const prodDepPaths = [ + `${servicePath}/node_modules/module-2`, + ]; + readFileAsyncStub.withArgs(sinon.match(/prod$/)).resolves(prodDepPaths); + + return expect(packagePlugin.excludeDevDependencies(params)).to.be + .fulfilled.then((updatedParams) => { + expect(globbySyncStub).to.have.been.calledOnce; + expect(execAsyncStub).to.have.been.calledTwice; + expect(readFileAsyncStub).to.have.been.calledTwice; + expect(globbySyncStub).to.have.been + .calledWithExactly(['**/package.json'], { + cwd: packagePlugin.serverless.config.servicePath, + dot: true, + silent: true, + follow: true, + nosort: true, + }); + expect(execAsyncStub.args[0][0]).to + .match(/npm ls --dev=true --parseable=true --silent >> .+/); + expect(execAsyncStub.args[0][1].cwd).to + .match(/.+/); + expect(execAsyncStub.args[1][0]).to + .match(/npm ls --prod=true --parseable=true --silent >> .+/); + expect(execAsyncStub.args[1][1].cwd).to + .match(/.+/); + expect(updatedParams.exclude).to.deep.equal([ + 'user-defined-exclude-me', + 'node_modules/module-1/**', + ]); + expect(updatedParams.include).to.deep.equal([ + 'user-defined-include-me', + ]); + expect(updatedParams.zipFileName).to.equal(params.zipFileName); + }); + }); }); });