fixup! fix: skip optional deps with mismatched platform or engine

npm · Feb 18, 2021 · a803be4 · a803be4
1 parent 2aaeff6
commit a803be4
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 43 deletions.
diff --git 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
@@ -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
@@ -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 })))