Ensure npm scripts (via Gulp) emit errors

[colinrotherham]

This PR ensure npm scripts (via Gulp) emit errors

We currently ignore npm script crashes even when they write to stderr (which wasn't intentional)

I've also prevented the npm run --silent flag being logged to reduce Gulp CLI "log spam"

Before

[15:04:48] Starting 'npm run build:jsdoc --silent'...
[15:04:49] Finished 'npm run build:jsdoc --silent' after 1.22 s
[15:04:49] Starting 'npm run build:sassdoc --silent'...
[15:04:51] Finished 'npm run build:sassdoc --silent' after 1.51 s

After

[15:04:48] Starting 'npm run build:jsdoc'...
[15:04:49] Finished 'npm run build:jsdoc' after 1.22 s
[15:04:49] Starting 'npm run build:sassdoc'...
[15:04:51] Finished 'npm run build:sassdoc' after 1.51 s

[colinrotherham]

We still add --silent behind the scenes because we don't want both npm AND Gulp logging progress to the console—one CLI tool is enough

Note: Output from the npm task (linting for example) is not silenced

[colinrotherham]

By emitting an error here it ensures we hit reject() (below) via the error listener

[romaricpascal]

question: Won't that trigger multiple times if a script outputs to stderr multiple times? Furthermore, I'm worried:

• it'll catch false errors for a script that just outputs to stderr as a convenient second buffer. I thought the actual marker of a script erroring was a non-zero exit (covered by the close)
• it'll double up with the close handling later

question: Why are we emitting an error rather than directly rejecting? That feels an unnecessary extra step there.

[colinrotherham]

Yeah it's confusing, probably why we wanted to remove Gulp 😭

Gulp sources are Node.js Stream instances (or early "stream" implementations that our Gulp plugins use)

They extend Node.js EventEmitter so we're going to follow the approach of expecting our tasks to emit:

task.emit('end') // Gulp piped task has finished processing
stream.emit('error') // Gulp stream has encountered an error

If we only reject() the promise we miss all the lovely error messages (lint output, line numbers etc)

By emitting events we can ensure the Gulp runner stays in control (logs the error and exits) but it also lets us listen via .on('error') and alternatively NOT exit (log only) like during gulp dev 🎉

• Allow Sass compilation to restart after gulp dev errors #3034
• Allow JavaScript compilation to restart after gulp dev errors #3035

[colinrotherham]

Won't that trigger multiple times if a script outputs to stderr multiple times?

The first error written will hit script.on('error', reject) so you'll only see the first one

[colinrotherham]

Also helps reduce complexity logged in #2243

[romaricpascal]

It all makes sense now, cheers for the explanation 😄