From a803be4849d4aa89e39c499d2a0c3f3c16eb57cf Mon Sep 17 00:00:00 2001 From: nlf Date: Thu, 18 Feb 2021 14:22:50 -0800 Subject: [PATCH] fixup! fix: skip optional deps with mismatched platform or engine --- lib/arborist/reify.js | 10 ++-- .../test-arborist-reify.js-TAP.test.js | 54 +++++++------------ test/arborist/reify.js | 2 +- 3 files changed, 23 insertions(+), 43 deletions(-) diff --git a/lib/arborist/reify.js b/lib/arborist/reify.js index fb0ead55c..7fd0728b0 100644 --- a/lib/arborist/reify.js +++ b/lib/arborist/reify.js @@ -84,7 +84,6 @@ const _explicitRequests = Symbol.for('explicitRequests') const _resolvedAdd = Symbol.for('resolvedAdd') const _usePackageLock = Symbol.for('usePackageLock') const _formatPackageLock = Symbol.for('formatPackageLock') -const _force = Symbol.for('force') module.exports = cls => class Reifier extends cls { constructor (options) { @@ -452,14 +451,13 @@ module.exports = cls => class Reifier extends cls { const p = Promise.resolve() .then(() => { // when we reify an optional node, check the engine and platform - // first. be sure to ignore the --engine-strict flag, since we - // always want to skip any optional packages. - // respect --force however, to match npm@6 behavior. + // first. be sure to ignore the --force and --engine-strict flags, + // since we always want to skip any optional packages we can't install. // these checks throwing will result in a rollback and removal // of the mismatches if (node.optional) { - checkEngine(node.package, npmVersion, nodeVersion, this[_force]) - checkPlatform(node.package, this[_force]) + checkEngine(node.package, npmVersion, nodeVersion, false) + checkPlatform(node.package, false) } }) .then(() => this[_checkBins](node)) diff --git a/tap-snapshots/test-arborist-reify.js-TAP.test.js b/tap-snapshots/test-arborist-reify.js-TAP.test.js index f7922a2a7..8e94a08c6 100644 --- a/tap-snapshots/test-arborist-reify.js-TAP.test.js +++ b/tap-snapshots/test-arborist-reify.js-TAP.test.js @@ -658,42 +658,6 @@ ArboristNode { } ` -exports[`test/arborist/reify.js TAP do install optional deps with mismatched platform specifications when forced > expect resolving Promise 1`] = ` -ArboristNode { - "children": Map { - "platform-specifying-test-package" => ArboristNode { - "edgesIn": Set { - EdgeIn { - "from": "", - "name": "platform-specifying-test-package", - "spec": "1.0.0", - "type": "optional", - }, - }, - "location": "node_modules/platform-specifying-test-package", - "name": "platform-specifying-test-package", - "optional": true, - "path": "{CWD}/test/arborist/reify-do-install-optional-deps-with-mismatched-platform-specifications-when-forced/node_modules/platform-specifying-test-package", - "resolved": "https://registry.npmjs.org/platform-specifying-test-package/-/platform-specifying-test-package-1.0.0.tgz", - "version": "1.0.0", - }, - }, - "edgesOut": Map { - "platform-specifying-test-package" => EdgeOut { - "name": "platform-specifying-test-package", - "spec": "1.0.0", - "to": "node_modules/platform-specifying-test-package", - "type": "optional", - }, - }, - "location": "", - "name": "reify-do-install-optional-deps-with-mismatched-platform-specifications-when-forced", - "packageName": "platform-test", - "path": "{CWD}/test/arborist/reify-do-install-optional-deps-with-mismatched-platform-specifications-when-forced", - "version": "1.0.0", -} -` - exports[`test/arborist/reify.js TAP do not add shrinkwrapped deps > expect resolving Promise 1`] = ` ArboristNode { "children": Map { @@ -29464,6 +29428,24 @@ exports[`test/arborist/reify.js TAP scoped registries > should preserve original @ruyadorno/theoretically-private-pkg@https://npm.pkg.github.com/@ruyadorno/theoretically-private-pkg/-/theoretically-private-pkg-1.2.3.tgz ` +exports[`test/arborist/reify.js TAP still do not install optional deps with mismatched platform specifications even when forced > expect resolving Promise 1`] = ` +ArboristNode { + "edgesOut": Map { + "platform-specifying-test-package" => EdgeOut { + "name": "platform-specifying-test-package", + "spec": "1.0.0", + "to": null, + "type": "optional", + }, + }, + "location": "", + "name": "reify-still-do-not-install-optional-deps-with-mismatched-platform-specifications-even-when-forced", + "packageName": "platform-test", + "path": "{CWD}/test/arborist/reify-still-do-not-install-optional-deps-with-mismatched-platform-specifications-even-when-forced", + "version": "1.0.0", +} +` + exports[`test/arborist/reify.js TAP store files with a custom indenting > must match snapshot 1`] = ` { "name": "tab-indented-package-json", diff --git a/test/arborist/reify.js b/test/arborist/reify.js index 501380aad..f5ef05641 100644 --- a/test/arborist/reify.js +++ b/test/arborist/reify.js @@ -367,7 +367,7 @@ t.test('do not install optional deps with mismatched platform specifications', t t.resolveMatchSnapshot(printReified( fixture(t, 'optional-platform-specification')))) -t.test('do install optional deps with mismatched platform specifications when forced', t => +t.test('still do not install optional deps with mismatched platform specifications even when forced', t => t.resolveMatchSnapshot(printReified( fixture(t, 'optional-platform-specification'), { force: true })))