Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 14, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions packages/build/src/core/commands.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ const runCommands = async function({ commands, configPath, buildDir, nodePath, c
index,
childEnv,
envChanges,
statuses,
commands,
errorMonitor,
error,
failedPlugins,
Expand Down Expand Up @@ -113,7 +113,7 @@ const runCommand = async function({
index,
childEnv,
envChanges,
statuses,
commands,
errorMonitor,
error,
failedPlugins,
Expand All @@ -138,7 +138,7 @@ const runCommand = async function({
nodePath,
childEnv,
envChanges,
statuses,
commands,
error,
})

Expand Down Expand Up @@ -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`
Expand Down Expand Up @@ -218,7 +218,7 @@ const firePluginCommand = async function({
packageJson,
local,
envChanges,
statuses,
commands,
error,
}) {
pipeOutput(childProcess)
Expand All @@ -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 }
Expand Down
51 changes: 34 additions & 17 deletions packages/build/src/core/status.js
Original file line number Diff line number Diff line change
@@ -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
}

Expand All @@ -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)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[[plugins]]
package = "./plugin.js"
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
module.exports = {
onBuild() {
throw new Error('onBuild')
},
onError() {
throw new Error('onError')
},
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,13 @@ module.exports = {
onBuild() {
throw new Error('onBuild')
},
onSuccess({
utils: {
status: { show },
},
}) {
show({ summary: 'summary' })
},
onError({
utils: {
status: { show },
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[[plugins]]
package = "./plugin.js"
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module.exports = {
onBuild() {},
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[[plugins]]
package = "./plugin.js"
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module.exports = {
onEnd() {},
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[[plugins]]
package = "./plugin.js"
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
module.exports = {
onBuild() {},
onSuccess() {},
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[[plugins]]
package = "./plugin.js"
10 changes: 10 additions & 0 deletions packages/build/tests/plugins/status/fixtures/no_implicit/plugin.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
module.exports = {
onBuild({
utils: {
status: { show },
},
}) {
show({ summary: 'summary' })
},
onSuccess() {},
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[[plugins]]
package = "./plugin.js"
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module.exports = {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[[plugins]]
package = "./plugin.js"
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module.exports = {
onError() {},
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[[plugins]]
package = "./plugin.js"
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
module.exports = {
onBuild({
utils: {
status: { show },
},
}) {
show({ summary: 'onBuild' })
},
onSuccess({
utils: {
status: { show },
},
}) {
show({ summary: 'onSuccess' })
},
}
Loading