From eccef72b520f02600839f513b4d0b8760e8a83b5 Mon Sep 17 00:00:00 2001
From: Rhys Arkins <rhys@arkins.net>
Date: Wed, 9 May 2018 18:04:37 +0200
Subject: [PATCH] fix(npm): restore autodetect pinVersions

---
 lib/manager/npm/extract/index.js              |  5 +++++
 lib/manager/npm/extract/type.js               | 18 +++++++++++++++++
 lib/workers/repository/process/fetch.js       | 14 ++++++++++++-
 .../extract/__snapshots__/index.spec.js.snap  |  3 +++
 test/manager/npm/extract/type.spec.js         | 20 +++++++++++++++++++
 .../process/__snapshots__/fetch.spec.js.snap  | 10 ++++++++++
 test/workers/repository/process/fetch.spec.js |  7 ++++++-
 7 files changed, 75 insertions(+), 2 deletions(-)
 create mode 100644 lib/manager/npm/extract/type.js
 create mode 100644 test/manager/npm/extract/type.spec.js

diff --git a/lib/manager/npm/extract/index.js b/lib/manager/npm/extract/index.js
index e0f9902dce..bf147185dd 100644
--- a/lib/manager/npm/extract/index.js
+++ b/lib/manager/npm/extract/index.js
@@ -2,6 +2,7 @@ const path = require('path');
 const upath = require('upath');
 const { getLockedVersions } = require('./locked-versions');
 const { detectMonorepos } = require('./monorepo');
+const { mightBeABrowserLibrary } = require('./type');
 
 module.exports = {
   extractDependencies,
@@ -22,6 +23,9 @@ async function extractDependencies(content, packageFile) {
   const packageJsonName = packageJson.name;
   const packageJsonVersion = packageJson.version;
   const yarnWorkspacesPackages = packageJson.workspaces;
+  const packageJsonType = mightBeABrowserLibrary(packageJson)
+    ? 'library'
+    : 'app';
 
   const lockFiles = {
     yarnLock: 'yarn.lock',
@@ -106,6 +110,7 @@ async function extractDependencies(content, packageFile) {
     deps,
     packageJsonName,
     packageJsonVersion,
+    packageJsonType,
     npmrc,
     ...lockFiles,
     lernaDir,
diff --git a/lib/manager/npm/extract/type.js b/lib/manager/npm/extract/type.js
new file mode 100644
index 0000000000..111498e1fd
--- /dev/null
+++ b/lib/manager/npm/extract/type.js
@@ -0,0 +1,18 @@
+module.exports = {
+  mightBeABrowserLibrary,
+};
+
+function mightBeABrowserLibrary(packageJson) {
+  // return true unless we're sure it's not a browser library
+  if (packageJson.private === true) {
+    // it's not published
+    return false;
+  }
+  if (packageJson.main === undefined) {
+    // it can't be required
+    return false;
+  }
+  // TODO: how can we know if it's a node.js library only, and not browser?
+  // Otherwise play it safe and return true
+  return true;
+}
diff --git a/lib/workers/repository/process/fetch.js b/lib/workers/repository/process/fetch.js
index ae7f5edc29..834f127b2f 100644
--- a/lib/workers/repository/process/fetch.js
+++ b/lib/workers/repository/process/fetch.js
@@ -12,7 +12,7 @@ module.exports = {
 async function fetchDepUpdates(packageFileConfig, dep) {
   /* eslint-disable no-param-reassign */
   const { manager, packageFile } = packageFileConfig;
-  const { depName, currentVersion } = dep;
+  const { depType, depName, currentVersion } = dep;
   let depConfig = mergeChildConfig(packageFileConfig, dep);
   depConfig = applyPackageRules(depConfig);
   dep.updates = [];
@@ -32,6 +32,18 @@ async function fetchDepUpdates(packageFileConfig, dep) {
     logger.debug({ depName: dep.depName }, 'Dependency is disabled');
     dep.skipReason = 'disabled';
   } else {
+    if (depConfig.pinVersions === null && !depConfig.upgradeInRange) {
+      if (depType === 'devDependencies') {
+        // Always pin devDependencies
+        logger.debug({ depName }, 'Pinning devDependency');
+        depConfig.pinVersions = true;
+      }
+      if (depType === 'dependencies' && depConfig.packageJsonType === 'app') {
+        // Pin dependencies if we're pretty sure it's not a browser library
+        logger.debug('Pinning app dependency');
+        depConfig.pinVersions = true;
+      }
+    }
     dep.updates = await getPackageUpdates(manager, depConfig);
     logger.debug({
       packageFile,
diff --git a/test/manager/npm/extract/__snapshots__/index.spec.js.snap b/test/manager/npm/extract/__snapshots__/index.spec.js.snap
index 3323d1bbcb..0bf7ed26cf 100644
--- a/test/manager/npm/extract/__snapshots__/index.spec.js.snap
+++ b/test/manager/npm/extract/__snapshots__/index.spec.js.snap
@@ -60,6 +60,7 @@ Object {
   "npmLock": undefined,
   "npmrc": undefined,
   "packageJsonName": "renovate",
+  "packageJsonType": "app",
   "packageJsonVersion": "1.0.0",
   "pnpmShrinkwrap": undefined,
   "yarnLock": "yarn.lock",
@@ -127,6 +128,7 @@ Object {
   "npmLock": undefined,
   "npmrc": undefined,
   "packageJsonName": "renovate",
+  "packageJsonType": "app",
   "packageJsonVersion": "1.0.0",
   "pnpmShrinkwrap": undefined,
   "yarnLock": undefined,
@@ -194,6 +196,7 @@ Object {
   "npmLock": undefined,
   "npmrc": undefined,
   "packageJsonName": "renovate",
+  "packageJsonType": "app",
   "packageJsonVersion": "1.0.0",
   "pnpmShrinkwrap": undefined,
   "yarnLock": undefined,
diff --git a/test/manager/npm/extract/type.spec.js b/test/manager/npm/extract/type.spec.js
new file mode 100644
index 0000000000..881eba2557
--- /dev/null
+++ b/test/manager/npm/extract/type.spec.js
@@ -0,0 +1,20 @@
+const {
+  mightBeABrowserLibrary,
+} = require('../../../../lib/manager/npm/extract/type');
+
+describe('manager/npm/extract/type', () => {
+  describe('.mightBeABrowserLibrary()', () => {
+    it('is not a library if private', () => {
+      const isLibrary = mightBeABrowserLibrary({ private: true });
+      expect(isLibrary).toBe(false);
+    });
+    it('is not a library if no main', () => {
+      const isLibrary = mightBeABrowserLibrary({});
+      expect(isLibrary).toBe(false);
+    });
+    it('is a library if has a main', () => {
+      const isLibrary = mightBeABrowserLibrary({ main: 'index.js ' });
+      expect(isLibrary).toBe(true);
+    });
+  });
+});
diff --git a/test/workers/repository/process/__snapshots__/fetch.spec.js.snap b/test/workers/repository/process/__snapshots__/fetch.spec.js.snap
index 063de32ae4..5a267eaa22 100644
--- a/test/workers/repository/process/__snapshots__/fetch.spec.js.snap
+++ b/test/workers/repository/process/__snapshots__/fetch.spec.js.snap
@@ -7,6 +7,15 @@ Object {
       "deps": Array [
         Object {
           "depName": "aaa",
+          "depType": "devDependencies",
+          "updates": Array [
+            "a",
+            "b",
+          ],
+        },
+        Object {
+          "depName": "bbb",
+          "depType": "dependencies",
           "updates": Array [
             "a",
             "b",
@@ -14,6 +23,7 @@ Object {
         },
       ],
       "packageFile": "package.json",
+      "packageJsonType": "app",
     },
   ],
 }
diff --git a/test/workers/repository/process/fetch.spec.js b/test/workers/repository/process/fetch.spec.js
index 87796ab801..fed32a4432 100644
--- a/test/workers/repository/process/fetch.spec.js
+++ b/test/workers/repository/process/fetch.spec.js
@@ -49,11 +49,16 @@ describe('workers/repository/process/fetch', () => {
       expect(packageFiles.npm[0].deps[2].updates).toHaveLength(0);
     });
     it('fetches updates', async () => {
+      config.pinVersions = null;
       const packageFiles = {
         npm: [
           {
             packageFile: 'package.json',
-            deps: [{ depName: 'aaa' }],
+            packageJsonType: 'app',
+            deps: [
+              { depName: 'aaa', depType: 'devDependencies' },
+              { depName: 'bbb', depType: 'dependencies' },
+            ],
           },
         ],
       };
-- 
GitLab