From 6fc40ad4f4026f4296d3eb537af5637a8f04c1e6 Mon Sep 17 00:00:00 2001
From: Rhys Arkins <rhys@arkins.net>
Date: Fri, 12 Oct 2018 20:00:49 +0200
Subject: [PATCH] refactor: default skipInstalls to null

This allows users to control true/false explicitly via config.

Related: #2647
---
 lib/config/definitions.js                      |  1 +
 lib/manager/npm/extract/index.js               | 18 +++++++++++++++---
 .../updates/__snapshots__/flatten.spec.js.snap | 16 ++++++++--------
 3 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/lib/config/definitions.js b/lib/config/definitions.js
index 42868154a0..c58be3d18e 100644
--- a/lib/config/definitions.js
+++ b/lib/config/definitions.js
@@ -251,6 +251,7 @@ const options = [
     description:
       'Skip installing modules/dependencies if lock file updating is possible alone',
     type: 'boolean',
+    default: null,
     admin: true,
   },
   {
diff --git a/lib/manager/npm/extract/index.js b/lib/manager/npm/extract/index.js
index 5cf44ee96b..f082153d2e 100644
--- a/lib/manager/npm/extract/index.js
+++ b/lib/manager/npm/extract/index.js
@@ -94,7 +94,7 @@ async function extractDependencies(content, fileName, config) {
   let lernaDir;
   let lernaPackages;
   let lernaClient;
-  let skipInstalls = true;
+  let hasFileRefs = false;
   const lernaJson = JSON.parse(
     await platform.getFile(upath.join(path.dirname(fileName), 'lerna.json'))
   );
@@ -134,7 +134,7 @@ async function extractDependencies(content, fileName, config) {
     }
     if (dep.currentValue.startsWith('file:')) {
       dep.skipReason = 'file';
-      skipInstalls = false;
+      hasFileRefs = true;
       return dep;
     }
     if (semver.isValid(dep.currentValue)) {
@@ -237,6 +237,18 @@ async function extractDependencies(content, fileName, config) {
       return null;
     }
   }
+  let skipInstalls = config.skipInstalls;
+  if (skipInstalls === null) {
+    if (hasFileRefs) {
+      // https://npm.community/t/npm-i-package-lock-only-changes-lock-file-incorrectly-when-file-references-used-in-dependencies/1412
+      // Explanation:
+      //  - npm install --package-lock-only is buggy for transitive deps in file: references
+      //  - So we set skipInstalls to false if file: refs are found *and* the user hasn't explicitly set the value already
+      skipInstalls = false;
+    } else {
+      skipInstalls = true;
+    }
+  }
   return {
     deps,
     packageJsonName,
@@ -248,7 +260,7 @@ async function extractDependencies(content, fileName, config) {
     lernaDir,
     lernaClient,
     lernaPackages,
-    skipInstalls: config.skipInstalls && skipInstalls,
+    skipInstalls,
     yarnWorkspacesPackages,
   };
 }
diff --git a/test/workers/repository/updates/__snapshots__/flatten.spec.js.snap b/test/workers/repository/updates/__snapshots__/flatten.spec.js.snap
index e4a1ead9a9..629cef0242 100644
--- a/test/workers/repository/updates/__snapshots__/flatten.spec.js.snap
+++ b/test/workers/repository/updates/__snapshots__/flatten.spec.js.snap
@@ -77,7 +77,7 @@ Array [
     "semanticCommits": null,
     "separateMajorMinor": true,
     "separateMinorPatch": false,
-    "skipInstalls": true,
+    "skipInstalls": null,
     "statusCheckVerify": false,
     "timezone": null,
     "unpublishSafe": false,
@@ -168,7 +168,7 @@ Array [
     "semanticCommits": null,
     "separateMajorMinor": true,
     "separateMinorPatch": false,
-    "skipInstalls": true,
+    "skipInstalls": null,
     "statusCheckVerify": false,
     "timezone": null,
     "unpublishSafe": false,
@@ -258,7 +258,7 @@ Array [
     "semanticCommits": null,
     "separateMajorMinor": true,
     "separateMinorPatch": false,
-    "skipInstalls": true,
+    "skipInstalls": null,
     "statusCheckVerify": false,
     "timezone": null,
     "unpublishSafe": false,
@@ -350,7 +350,7 @@ Array [
     "semanticCommits": null,
     "separateMajorMinor": true,
     "separateMinorPatch": false,
-    "skipInstalls": true,
+    "skipInstalls": null,
     "statusCheckVerify": false,
     "timezone": null,
     "unpublishSafe": false,
@@ -440,7 +440,7 @@ Array [
     "semanticCommits": null,
     "separateMajorMinor": true,
     "separateMinorPatch": false,
-    "skipInstalls": true,
+    "skipInstalls": null,
     "statusCheckVerify": false,
     "timezone": null,
     "unpublishSafe": false,
@@ -532,7 +532,7 @@ Array [
     "semanticCommits": null,
     "separateMajorMinor": true,
     "separateMinorPatch": false,
-    "skipInstalls": true,
+    "skipInstalls": null,
     "statusCheckVerify": false,
     "timezone": null,
     "unpublishSafe": false,
@@ -623,7 +623,7 @@ Array [
     "semanticCommits": null,
     "separateMajorMinor": true,
     "separateMinorPatch": false,
-    "skipInstalls": true,
+    "skipInstalls": null,
     "statusCheckVerify": false,
     "timezone": null,
     "unpublishSafe": false,
@@ -714,7 +714,7 @@ Array [
     "semanticCommits": null,
     "separateMajorMinor": true,
     "separateMinorPatch": false,
-    "skipInstalls": true,
+    "skipInstalls": null,
     "statusCheckVerify": false,
     "timezone": null,
     "unpublishSafe": false,
-- 
GitLab