Assign a default status for plugins that do not use utils.status.show() (#1275)

Author: ehmicky

packages/build/src/core/commands.js

@@ -79,7 +79,7 @@ const runCommands = async function({ commands, configPath, buildDir, nodePath, c
           index,
           childEnv,
           envChanges,
-          statuses,
+          commands,
           errorMonitor,
           error,
           failedPlugins,
@@ -113,7 +113,7 @@ const runCommand = async function({
   index,
   childEnv,
   envChanges,
-  statuses,
+  commands,
   errorMonitor,
   error,
   failedPlugins,
@@ -138,7 +138,7 @@ const runCommand = async function({
     nodePath,
     childEnv,
     envChanges,
-    statuses,
+    commands,
     error,
   })
 
@@ -173,14 +173,14 @@ const fireCommand = function({
   nodePath,
   childEnv,
   envChanges,
-  statuses,
+  commands,
   error,
 }) {
   if (buildCommand !== undefined) {
     return fireBuildCommand({ buildCommand, configPath, buildDir, nodePath, childEnv, envChanges })
   }
 
-  return firePluginCommand({ event, childProcess, package, packageJson, local, envChanges, statuses, error })
+  return firePluginCommand({ event, childProcess, package, packageJson, local, envChanges, commands, error })
 }
 
 // Fire `build.command`
@@ -218,7 +218,7 @@ const firePluginCommand = async function({
   packageJson,
   local,
   envChanges,
-  statuses,
+  commands,
   error,
 }) {
   pipeOutput(childProcess)
@@ -230,7 +230,7 @@ const firePluginCommand = async function({
       { event, error, envChanges },
       { plugin: { packageJson, package }, location: { event, package, local } },
     )
-    const newStatus = getSuccessStatus({ status, statuses, package })
+    const newStatus = getSuccessStatus(status, { commands, event, package })
     return { newEnvChanges, newStatus }
   } catch (newError) {
     return { newError }

packages/build/src/core/status.js

@@ -1,35 +1,37 @@
 const { logStatuses } = require('../log/main')
 
-// Assign default value for successful `newStatus`
-// Retrieved from `utils.status.show()`, which is optional
-const getSuccessStatus = function({ status, statuses, package }) {
-  // `utils.status.show()` called in the current event
-  if (status !== undefined) {
-    return status
+// The last event handler of a plugin (except for `onError` and `onEnd`)
+// defaults to `utils.status.show({ state: 'success' })` without any `summary`.
+const getSuccessStatus = function(newStatus, { commands, event, package }) {
+  if (newStatus === undefined && isLastMainCommand({ commands, event, package })) {
+    return IMPLICIT_STATUS
   }
 
-  // `utils.status.show()` not called, but set in a previous event
-  const hasStatus = statuses.some(pluginStatus => pluginStatus.package === package)
-  if (hasStatus) {
-    return
-  }
+  return newStatus
+}
 
-  // `utils.status.show()` not called, but this is the first event, so we assign a default
-  return { state: 'success' }
+const isLastMainCommand = function({ commands, event, package }) {
+  const mainCommands = commands.filter(command => command.package === package && isMainCommand(command))
+  return mainCommands.length === 0 || mainCommands[mainCommands.length - 1].event === event
 }
 
-// Merge `success` status to the list of plugin statuses.
+const isMainCommand = function({ event }) {
+  return event !== 'onEnd' && event !== 'onError'
+}
+
+const IMPLICIT_STATUS = { state: 'success', implicit: true }
+
+// Merge plugin status to the list of plugin statuses.
 const addStatus = function({ newStatus, statuses, event, package, packageJson: { version } = {} }) {
   // Either:
   //  - `build.command`
-  //  - `utils.status.show()` not called but set in a previous event
+  //  - no status was set
   if (newStatus === undefined) {
     return statuses
   }
 
-  // Error statuses cannot be overwritten
   const formerStatus = statuses.find(status => status.package === package)
-  if (formerStatus !== undefined && formerStatus.state !== 'success') {
+  if (!canOverrideStatus(formerStatus, newStatus)) {
     return statuses
   }
 
@@ -38,6 +40,21 @@ const addStatus = function({ newStatus, statuses, event, package, packageJson: {
   return [...newStatuses, { ...newStatus, event, package, version }]
 }
 
+const canOverrideStatus = function(formerStatus, newStatus) {
+  // No previous status
+  if (formerStatus === undefined) {
+    return true
+  }
+
+  // Implicit statuses can never override
+  if (newStatus.implicit) {
+    return false
+  }
+
+  // Error statuses can only be overwritten by other error statuses
+  return formerStatus.state === 'success' || newStatus.state !== 'success'
+}
+
 const reportStatuses = async function(statuses, api, mode) {
   printStatuses(statuses, mode)
 }

packages/build/tests/plugins/status/fixtures/error_status_error_override/netlify.toml

@@ -0,0 +1,2 @@
+[[plugins]]
+package = "./plugin.js"

packages/build/tests/plugins/status/fixtures/error_status_error_override/plugin.js

@@ -0,0 +1,8 @@
+module.exports = {
+  onBuild() {
+    throw new Error('onBuild')
+  },
+  onError() {
+    throw new Error('onError')
+  },
+}

packages/build/tests/plugins/status/fixtures/error_status_override/plugin.js

@@ -2,6 +2,13 @@ module.exports = {
   onBuild() {
     throw new Error('onBuild')
   },
+  onSuccess({
+    utils: {
+      status: { show },
+    },
+  }) {
+    show({ summary: 'summary' })
+  },
   onError({
     utils: {
       status: { show },

packages/build/tests/plugins/status/fixtures/implicit_one/netlify.toml

@@ -0,0 +1,2 @@
+[[plugins]]
+package = "./plugin.js"

packages/build/tests/plugins/status/fixtures/implicit_one/plugin.js

@@ -0,0 +1,3 @@
+module.exports = {
+  onBuild() {},
+}

packages/build/tests/plugins/status/fixtures/implicit_onend/netlify.toml

@@ -0,0 +1,2 @@
+[[plugins]]
+package = "./plugin.js"

packages/build/tests/plugins/status/fixtures/implicit_onend/plugin.js

@@ -0,0 +1,3 @@
+module.exports = {
+  onEnd() {},
+}

packages/build/tests/plugins/status/fixtures/implicit_several/netlify.toml

@@ -0,0 +1,2 @@
+[[plugins]]
+package = "./plugin.js"

packages/build/tests/plugins/status/fixtures/implicit_several/plugin.js

@@ -0,0 +1,4 @@
+module.exports = {
+  onBuild() {},
+  onSuccess() {},
+}

packages/build/tests/plugins/status/fixtures/no_implicit/netlify.toml

@@ -0,0 +1,2 @@
+[[plugins]]
+package = "./plugin.js"

packages/build/tests/plugins/status/fixtures/no_implicit/plugin.js

@@ -0,0 +1,10 @@
+module.exports = {
+  onBuild({
+    utils: {
+      status: { show },
+    },
+  }) {
+    show({ summary: 'summary' })
+  },
+  onSuccess() {},
+}

packages/build/tests/plugins/status/fixtures/no_implicit_none/netlify.toml

@@ -0,0 +1,2 @@
+[[plugins]]
+package = "./plugin.js"

packages/build/tests/plugins/status/fixtures/no_implicit_none/plugin.js

@@ -0,0 +1 @@
+module.exports = {}

packages/build/tests/plugins/status/fixtures/no_implicit_onerror/netlify.toml

@@ -0,0 +1,2 @@
+[[plugins]]
+package = "./plugin.js"

packages/build/tests/plugins/status/fixtures/no_implicit_onerror/plugin.js

@@ -0,0 +1,3 @@
+module.exports = {
+  onError() {},
+}

packages/build/tests/plugins/status/fixtures/success_status_override/netlify.toml

@@ -0,0 +1,2 @@
+[[plugins]]
+package = "./plugin.js"

packages/build/tests/plugins/status/fixtures/success_status_override/plugin.js

@@ -0,0 +1,16 @@
+module.exports = {
+  onBuild({
+    utils: {
+      status: { show },
+    },
+  }) {
+    show({ summary: 'onBuild' })
+  },
+  onSuccess({
+    utils: {
+      status: { show },
+    },
+  }) {
+    show({ summary: 'onSuccess' })
+  },
+}