From bddfca4e784ad58d16dabe64cea82a4a49ab55e8 Mon Sep 17 00:00:00 2001
From: Zoltan Kochan <z@kochan.io>
Date: Mon, 7 May 2018 22:23:21 +0300
Subject: [PATCH] feat: change the format of skipped optional dep log

---
 src/api/install.ts                   |  2 +-
 src/api/rebuild.ts                   |  2 +-
 src/install/getIsInstallable.ts      | 20 ++++++++++++++------
 src/loggers.ts                       |  6 ++++--
 src/resolveDependencies.ts           |  2 +-
 test/install/optionalDependencies.ts |  8 ++++----
 6 files changed, 25 insertions(+), 15 deletions(-)

diff --git a/src/api/install.ts b/src/api/install.ts
index 7e19ca3cd4..98bc0aa0da 100644
--- a/src/api/install.ts
+++ b/src/api/install.ts
@@ -689,7 +689,7 @@ async function installInContext (
               if (installCtx.pkgByPkgId[pkg.id].optional) {
                 // TODO: add parents field to the log
                 skippedOptionalDependencyLogger.debug({
-                  details: err,
+                  details: err.toString(),
                   package: {
                     id: pkg.id,
                     name: pkg.name,
diff --git a/src/api/rebuild.ts b/src/api/rebuild.ts
index 0aef867d73..65597ab362 100644
--- a/src/api/rebuild.ts
+++ b/src/api/rebuild.ts
@@ -219,7 +219,7 @@ async function _rebuild (
             if (pkgSnapshot.optional) {
               // TODO: add parents field to the log
               skippedOptionalDependencyLogger.debug({
-                details: err,
+                details: err.toString(),
                 package: {
                   id: pkgSnapshot.id || depAbsolutePath,
                   name: pkgInfo.name,
diff --git a/src/install/getIsInstallable.ts b/src/install/getIsInstallable.ts
index 5e5a55d425..9acd333c0d 100644
--- a/src/install/getIsInstallable.ts
+++ b/src/install/getIsInstallable.ts
@@ -21,10 +21,18 @@ export default async function getIsInstallable (
     pnpmVersion: string,
   },
 ): Promise<boolean> {
-  const warn = await installChecks.checkPlatform(pkg) || await installChecks.checkEngine(pkg, {
-    nodeVersion: options.nodeVersion,
-    pnpmVersion: options.pnpmVersion,
-  })
+  const warn = await installChecks.checkPlatform({
+      _id: pkgId,
+      cpu: pkg.cpu,
+      os: pkg.os,
+    }) ||
+    await installChecks.checkEngine({
+      _id: pkgId,
+      engines: pkg.engines,
+    }, {
+      nodeVersion: options.nodeVersion,
+      pnpmVersion: options.pnpmVersion,
+    })
 
   if (!warn) return true
 
@@ -32,14 +40,14 @@ export default async function getIsInstallable (
 
   if (options.optional) {
     skippedOptionalDependencyLogger.debug({
-      details: warn,
+      details: warn.toString(),
       package: {
         id: pkgId,
         name: pkg.name,
         version: pkg.version,
       },
       parents: nodeIdToParents(options.nodeId, options.pkgByPkgId),
-      reason: 'incompatible_engine',
+      reason: warn.code === 'ENOTSUP' ? 'unsupported_engine' : 'unsupported_platform',
     })
 
     return false
diff --git a/src/loggers.ts b/src/loggers.ts
index 923b0868d3..513ee01b2d 100644
--- a/src/loggers.ts
+++ b/src/loggers.ts
@@ -89,7 +89,7 @@ export type StatsMessage = {
 })
 
 export type SkippedOptionalDependencyMessage = {
-  details?: object,
+  details?: string,
   parents?: Array<{id: string, name: string, version: string}>,
 } & ({
   package: {
@@ -97,7 +97,9 @@ export type SkippedOptionalDependencyMessage = {
     name: string,
     version: string,
   },
-  reason: 'incompatible_engine' | 'build_failure',
+  reason: 'unsupported_engine'
+    | 'unsupported_platform'
+    | 'build_failure',
 } | {
   package: {
     name: string | undefined,
diff --git a/src/resolveDependencies.ts b/src/resolveDependencies.ts
index 39fa45d049..1f761130ba 100644
--- a/src/resolveDependencies.ts
+++ b/src/resolveDependencies.ts
@@ -274,7 +274,7 @@ async function install (
   } catch (err) {
     if (wantedDependency.optional) {
       skippedOptionalDependencyLogger.debug({
-        details: err,
+        details: err.toString(),
         package: {
           name: wantedDependency.alias,
           pref: wantedDependency.pref,
diff --git a/test/install/optionalDependencies.ts b/test/install/optionalDependencies.ts
index 3db074984f..1bc03224d6 100644
--- a/test/install/optionalDependencies.ts
+++ b/test/install/optionalDependencies.ts
@@ -87,7 +87,7 @@ test('skip optional dependency that does not support the current OS', async (t:
       version: '1.0.0',
     },
     parents: [],
-    reason: 'incompatible_engine',
+    reason: 'unsupported_platform',
   })
   const reportedTimes = reporter.withArgs(logMatcher).callCount
   t.equal(reportedTimes, 1, 'skipping optional dependency is logged')
@@ -113,7 +113,7 @@ test('skip optional dependency that does not support the current Node version',
       version: '1.0.0',
     },
     parents: [],
-    reason: 'incompatible_engine',
+    reason: 'unsupported_engine',
   })
   const reportedTimes = reporter.withArgs(logMatcher).callCount
   t.equal(reportedTimes, 1, 'skipping optional dependency is logged')
@@ -139,7 +139,7 @@ test('skip optional dependency that does not support the current pnpm version',
       version: '1.0.0',
     },
     parents: [],
-    reason: 'incompatible_engine',
+    reason: 'unsupported_engine',
   })
   const reportedTimes = reporter.withArgs(logMatcher).callCount
   t.equal(reportedTimes, 1, 'skipping optional dependency is logged')
@@ -183,7 +183,7 @@ test('optional subdependency is skipped', async (t: tape.Test) => {
         version: '1.0.0',
       },
     ],
-    reason: 'incompatible_engine',
+    reason: 'unsupported_platform',
   })
   const reportedTimes = reporter.withArgs(logMatcher).callCount
   t.equal(reportedTimes, 1, 'skipping optional dependency is logged')