From 24f0f37115f2eb913818292496659c1f0a3f31ad Mon Sep 17 00:00:00 2001 From: Jeremy Benoist Date: Wed, 16 Oct 2019 16:29:17 +0200 Subject: [PATCH 1/2] Remove optional peer dependencies When fetching peer dependencies we now remove those which are optional. And optional dependencies can be defined using Yarn (only at the moment) when using the `peerDependenciesMeta` field. --- lib/packExternalModules.js | 34 +++- tests/data/rp-package-optional.json | 94 +++++++++ tests/packExternalModules.test.js | 303 +++++++++++++++++++--------- 3 files changed, 329 insertions(+), 102 deletions(-) create mode 100644 tests/data/rp-package-optional.json diff --git a/lib/packExternalModules.js b/lib/packExternalModules.js index 3e787a874..cd762a304 100644 --- a/lib/packExternalModules.js +++ b/lib/packExternalModules.js @@ -94,14 +94,32 @@ function getProdModules(externalModules, packagePath, dependencyGraph, forceExcl 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, - forceExcludes - ); - Array.prototype.push.apply(prodModules, peerModules); + + const peerDependenciesMeta = require(modulePackagePath).peerDependenciesMeta; + + if (!_.isEmpty(peerDependenciesMeta)) { + _.forEach(peerDependencies, (value, key) => { + if (peerDependenciesMeta[key] && peerDependenciesMeta[key].optional === true) { + this.options.verbose && + this.serverless.cli.log( + `Skipping peers dependency ${key} for dependency ${module.external} because it's optional` + ); + + _.unset(peerDependencies, key); + } + }); + } + + if (!_.isEmpty(peerDependencies)) { + 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}`); diff --git a/tests/data/rp-package-optional.json b/tests/data/rp-package-optional.json new file mode 100644 index 000000000..f193d5c37 --- /dev/null +++ b/tests/data/rp-package-optional.json @@ -0,0 +1,94 @@ +{ + "_from": "request-promise", + "_id": "request-promise@4.2.1", + "_inBundle": false, + "_integrity": "sha1-fuxWyJMXqCLL/qmbA5zlQ8LhX2c=", + "_location": "/request-promise", + "_phantomChildren": {}, + "_requested": { + "type": "tag", + "registry": true, + "raw": "request-promise", + "name": "request-promise", + "escapedName": "request-promise", + "rawSpec": "", + "saveSpec": null, + "fetchSpec": "latest" + }, + "_requiredBy": [ + "#USER", + "/" + ], + "_resolved": "https://registry.npmjs.org/request-promise/-/request-promise-4.2.1.tgz", + "_shasum": "7eec56c89317a822cbfea99b039ce543c2e15f67", + "_spec": "request-promise", + "_where": "C:\\Projects\\serverless\\test\\babel-dynamically-entries", + "author": { + "name": "Nicolai Kamenzky", + "url": "https://github.com/analog-nico" + }, + "bugs": { + "url": "https://github.com/request/request-promise/issues" + }, + "bundleDependencies": false, + "dependencies": { + "bluebird": "^3.5.0", + "request-promise-core": "1.1.1", + "stealthy-require": "^1.1.0", + "tough-cookie": ">=2.3.0" + }, + "deprecated": false, + "description": "The simplified HTTP request client 'request' with Promise support. Powered by Bluebird.", + "devDependencies": { + "body-parser": "~1.15.2", + "chai": "~3.5.0", + "chalk": "~1.1.3", + "gulp": "~3.9.1", + "gulp-coveralls": "~0.1.4", + "gulp-eslint": "~2.1.0", + "gulp-istanbul": "~1.0.0", + "gulp-mocha": "~2.2.0", + "lodash": "~4.13.1", + "publish-please": "~2.1.4", + "request": "^2.34.0", + "rimraf": "~2.5.3", + "run-sequence": "~1.2.2" + }, + "engines": { + "node": ">=0.10.0" + }, + "homepage": "https://github.com/request/request-promise#readme", + "keywords": [ + "xhr", + "http", + "https", + "promise", + "request", + "then", + "thenable", + "bluebird" + ], + "license": "ISC", + "main": "./lib/rp.js", + "name": "request-promise", + "peerDependencies": { + "request": "^2.34", + "canvas": "^2.5.0" + }, + "peerDependenciesMeta": { + "canvas": { + "optional": true + } + }, + "repository": { + "type": "git", + "url": "git+https://github.com/request/request-promise.git" + }, + "scripts": { + "prepublish": "publish-please guard", + "publish-please": "publish-please", + "test": "gulp ci", + "test-publish": "gulp ci-no-cov" + }, + "version": "4.2.1" +} diff --git a/tests/packExternalModules.test.js b/tests/packExternalModules.test.js index 2fb9a88f3..b1abd4305 100644 --- a/tests/packExternalModules.test.js +++ b/tests/packExternalModules.test.js @@ -1114,107 +1114,222 @@ describe('packExternalModules', () => { }); describe('peer dependencies', () => { - before(() => { - const peerDepPackageJson = require('./data/package-peerdeps.json'); - mockery.deregisterMock(path.join(process.cwd(), 'package.json')); - mockery.registerMock(path.join(process.cwd(), 'package.json'), peerDepPackageJson); - // Mock request-promise package.json - const rpPackageJson = require('./data/rp-package.json'); - const rpPackagePath = path.join(process.cwd(), 'node_modules', 'request-promise', 'package.json'); - mockery.registerMock(rpPackagePath, rpPackageJson); - }); + /** + * Both "default" & "optinal" behaviors are mostly equal. + * The only difference between each scenario is they don't use the same package.json as mock + */ + describe('default behavior', () => { + before(() => { + const peerDepPackageJson = require('./data/package-peerdeps.json'); + mockery.deregisterMock(path.join(process.cwd(), 'package.json')); + mockery.registerMock(path.join(process.cwd(), 'package.json'), peerDepPackageJson); + // Mock request-promise package.json + const rpPackageJson = require('./data/rp-package.json'); + const rpPackagePath = path.join(process.cwd(), 'node_modules', 'request-promise', 'package.json'); + mockery.registerMock(rpPackagePath, rpPackageJson); + }); - after(() => { - mockery.deregisterMock(path.join(process.cwd(), 'package.json')); - mockery.registerMock(path.join(process.cwd(), 'package.json'), packageMock); - const rpPackagePath = path.join(process.cwd(), 'node_modules', 'request-promise', 'package.json'); - mockery.deregisterMock(rpPackagePath); - }); + after(() => { + mockery.deregisterMock(path.join(process.cwd(), 'package.json')); + mockery.registerMock(path.join(process.cwd(), 'package.json'), packageMock); + const rpPackagePath = path.join(process.cwd(), 'node_modules', 'request-promise', 'package.json'); + mockery.deregisterMock(rpPackagePath); + }); - it('should install external peer dependencies', () => { - const expectedCompositePackageJSON = { - name: 'test-service', - version: '1.0.0', - description: 'Packaged externals for test-service', - private: true, - scripts: {}, - dependencies: { - bluebird: '^3.5.0', - 'request-promise': '^4.2.1', - request: '^2.82.0' - } - }; - const expectedPackageJSON = { - name: 'test-service', - version: '1.0.0', - description: 'Packaged externals for test-service', - private: true, - scripts: {}, - dependencies: { - bluebird: '^3.5.0', - 'request-promise': '^4.2.1', - request: '^2.82.0' - } - }; + it('should install external peer dependencies', () => { + const expectedCompositePackageJSON = { + name: 'test-service', + version: '1.0.0', + description: 'Packaged externals for test-service', + private: true, + scripts: {}, + dependencies: { + bluebird: '^3.5.0', + 'request-promise': '^4.2.1', + request: '^2.82.0' + } + }; + const expectedPackageJSON = { + name: 'test-service', + version: '1.0.0', + description: 'Packaged externals for test-service', + private: true, + scripts: {}, + dependencies: { + bluebird: '^3.5.0', + 'request-promise': '^4.2.1', + request: '^2.82.0' + } + }; - const dependencyGraph = require('./data/npm-ls-peerdeps.json'); - const peerDepStats = { - stats: [ - { - compilation: { - chunks: [ - new ChunkMock([ - { - identifier: _.constant('"crypto"') - }, - { - identifier: _.constant('"uuid/v4"') - }, - { - identifier: _.constant('"mockery"') - }, - { - identifier: _.constant('"@scoped/vendor/module1"') - }, - { - identifier: _.constant('external "bluebird"') - }, - { - identifier: _.constant('external "request-promise"') - } - ]) - ], - compiler: { - outputPath: '/my/Service/Path/.webpack/service' + const dependencyGraph = require('./data/npm-ls-peerdeps.json'); + const peerDepStats = { + stats: [ + { + compilation: { + chunks: [ + new ChunkMock([ + { + identifier: _.constant('"crypto"') + }, + { + identifier: _.constant('"uuid/v4"') + }, + { + identifier: _.constant('"mockery"') + }, + { + identifier: _.constant('"@scoped/vendor/module1"') + }, + { + identifier: _.constant('external "bluebird"') + }, + { + identifier: _.constant('external "request-promise"') + } + ]) + ], + compiler: { + outputPath: '/my/Service/Path/.webpack/service' + } } } + ] + }; + + module.webpackOutputPath = 'outputPath'; + fsExtraMock.pathExists.yields(null, false); + fsExtraMock.copy.yields(); + packagerMock.getProdDependencies.returns(BbPromise.resolve(dependencyGraph)); + packagerMock.install.returns(BbPromise.resolve()); + packagerMock.prune.returns(BbPromise.resolve()); + packagerMock.runScripts.returns(BbPromise.resolve()); + module.compileStats = peerDepStats; + return expect(module.packExternalModules()).to.be.fulfilled.then(() => + BbPromise.all([ + // The module package JSON and the composite one should have been stored + expect(writeFileSyncStub).to.have.been.calledTwice, + expect(writeFileSyncStub.firstCall.args[1]).to.equal( + JSON.stringify(expectedCompositePackageJSON, null, 2) + ), + expect(writeFileSyncStub.secondCall.args[1]).to.equal(JSON.stringify(expectedPackageJSON, null, 2)), + // The modules should have been copied + expect(fsExtraMock.copy).to.have.been.calledOnce, + // npm ls and npm prune 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 + ]) + ); + }); + }); + + describe('optional behavior', () => { + before(() => { + const peerDepPackageJson = require('./data/package-peerdeps.json'); + mockery.deregisterMock(path.join(process.cwd(), 'package.json')); + mockery.registerMock(path.join(process.cwd(), 'package.json'), peerDepPackageJson); + // Mock request-promise package.json + const rpPackageJson = require('./data/rp-package-optional.json'); + const rpPackagePath = path.join(process.cwd(), 'node_modules', 'request-promise', 'package.json'); + mockery.registerMock(rpPackagePath, rpPackageJson); + }); + + after(() => { + mockery.deregisterMock(path.join(process.cwd(), 'package.json')); + mockery.registerMock(path.join(process.cwd(), 'package.json'), packageMock); + const rpPackagePath = path.join(process.cwd(), 'node_modules', 'request-promise', 'package.json'); + mockery.deregisterMock(rpPackagePath); + }); + + it('should skip optional peer dependencies', () => { + const expectedCompositePackageJSON = { + name: 'test-service', + version: '1.0.0', + description: 'Packaged externals for test-service', + private: true, + scripts: {}, + dependencies: { + bluebird: '^3.5.0', + 'request-promise': '^4.2.1', + request: '^2.82.0' + } + }; + const expectedPackageJSON = { + name: 'test-service', + version: '1.0.0', + description: 'Packaged externals for test-service', + private: true, + scripts: {}, + dependencies: { + bluebird: '^3.5.0', + 'request-promise': '^4.2.1', + request: '^2.82.0' } - ] - }; + }; - module.webpackOutputPath = 'outputPath'; - fsExtraMock.pathExists.yields(null, false); - fsExtraMock.copy.yields(); - packagerMock.getProdDependencies.returns(BbPromise.resolve(dependencyGraph)); - packagerMock.install.returns(BbPromise.resolve()); - packagerMock.prune.returns(BbPromise.resolve()); - packagerMock.runScripts.returns(BbPromise.resolve()); - module.compileStats = peerDepStats; - return expect(module.packExternalModules()).to.be.fulfilled.then(() => - BbPromise.all([ - // The module package JSON and the composite one should have been stored - expect(writeFileSyncStub).to.have.been.calledTwice, - expect(writeFileSyncStub.firstCall.args[1]).to.equal(JSON.stringify(expectedCompositePackageJSON, null, 2)), - expect(writeFileSyncStub.secondCall.args[1]).to.equal(JSON.stringify(expectedPackageJSON, null, 2)), - // The modules should have been copied - expect(fsExtraMock.copy).to.have.been.calledOnce, - // npm ls and npm prune 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 - ]) - ); + const dependencyGraph = require('./data/npm-ls-peerdeps.json'); + const peerDepStats = { + stats: [ + { + compilation: { + chunks: [ + new ChunkMock([ + { + identifier: _.constant('"crypto"') + }, + { + identifier: _.constant('"uuid/v4"') + }, + { + identifier: _.constant('"mockery"') + }, + { + identifier: _.constant('"@scoped/vendor/module1"') + }, + { + identifier: _.constant('external "bluebird"') + }, + { + identifier: _.constant('external "request-promise"') + } + ]) + ], + compiler: { + outputPath: '/my/Service/Path/.webpack/service' + } + } + } + ] + }; + + module.webpackOutputPath = 'outputPath'; + fsExtraMock.pathExists.yields(null, false); + fsExtraMock.copy.yields(); + packagerMock.getProdDependencies.returns(BbPromise.resolve(dependencyGraph)); + packagerMock.install.returns(BbPromise.resolve()); + packagerMock.prune.returns(BbPromise.resolve()); + packagerMock.runScripts.returns(BbPromise.resolve()); + module.compileStats = peerDepStats; + return expect(module.packExternalModules()).to.be.fulfilled.then(() => + BbPromise.all([ + // The module package JSON and the composite one should have been stored + expect(writeFileSyncStub).to.have.been.calledTwice, + expect(writeFileSyncStub.firstCall.args[1]).to.equal( + JSON.stringify(expectedCompositePackageJSON, null, 2) + ), + expect(writeFileSyncStub.secondCall.args[1]).to.equal(JSON.stringify(expectedPackageJSON, null, 2)), + // The modules should have been copied + expect(fsExtraMock.copy).to.have.been.calledOnce, + // npm ls and npm prune 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 + ]) + ); + }); }); }); }); From 0563222b02c4207f5cb6aba16c34ba810fc15f70 Mon Sep 17 00:00:00 2001 From: "Miguel A. Calles MBA" <44813512+miguel-a-calles-mba@users.noreply.github.com> Date: Mon, 1 Jun 2020 19:07:15 -0700 Subject: [PATCH 2/2] Update packExternalModules.js --- lib/packExternalModules.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/packExternalModules.js b/lib/packExternalModules.js index cd762a304..ac7b091dc 100644 --- a/lib/packExternalModules.js +++ b/lib/packExternalModules.js @@ -442,3 +442,4 @@ module.exports = { }); } }; +