Increase parity between "pitch" & "normal" loaders

[jugglinmike]

Webpack's documentation describes the default behavior of "pitching" loaders as follows [1]:

[...] If a loader delivers a result in the pitch method the process turns around and skips the remaining loaders, continuing with the calls to the more left loaders. [...]

This implicitly suggests that the "pitching" behavior will not take place when a value is not returned. The loader will be "skipped", and traversal through the chain of loaders will continue.

In synchronous "pitching" loaders, the skipping behavior can be triggered by explicitly returning the undefined value or by omitting a return statement altogether.

In asynchronous "pitching" loaders which signal completion via a callback function, the skipping behavior can only be triggered by invoking the callback function with zero or 1 arguments. If a second argument value is specified as undefined, that value is interpreted as invalid source code (rather than a signal to continue traversing the chain of loaders). This argument-length-driven behavior is inconvenient because it requires consumers maintain a dedicated code path for the described condition (which may explain why such APIs are relatively rare in idiomatic Node.js code).

In asynchronous "pitching" loaders which signal completion via a Promise value, this same implementation detail makes it impossible to trigger the skipping behavior.

Consistently interpret the value undefined as a signal to skip the loader, whether specified as return value from synchronous loaders, an asynchronous callback value, or an asynchronous Promise resolution value.

[1] https://webpack.js.org/api/loaders/#pitching-loader

This is intended to resolve https://github.com/webpack/webpack/issues/5782

[jsf-clabot]

All committers have signed the CLA.

[jugglinmike]

The CI failure is due to a linting error that also occurs in master.

[jugglinmike]

Sure. Since this is getting a little involved, I've added a comment to explain the intention. Let me know if that text makes sense.

The build continues to fail because of that pre-existing problem in master. It's a linting error in the following code:

context.async = function async() {

I was inclined to fix that mysel, but it looks like this project uses multiple validation steps that are configured with incompatible parameters. Currently, the beautify-lint script reports:

> eslint lib test

> loader-runner@2.3.0 beautify-lint /home/mike/projects/oss/loader-runner
> beautify-lint lib/**.js test/*.js

Index: lib/LoaderRunner.js
===================================================================
--- lib/LoaderRunner.js
+++ lib/LoaderRunner.js
@@ -91,9 +91,9 @@
  var isSync = true;
  var isDone = false;
  var isError = false; // internal error
  var reportedError = false;
- context.async = function async() {
+ context.async = function async () {
      if(isDone) {
          if(reportedError) return; // ignore
          throw new Error("async(): The callback was already called.");
      }

1 Errors. (3 files checked)

Satisfying that check causes ESLint to report:

> eslint lib test

/home/mike/projects/oss/loader-runner/lib/LoaderRunner.js
  95:32  error  Unexpected space before function parentheses  space-before-function-paren

✖ 1 problem (1 error, 0 warnings)

Since the distinction is highly subjective, a project maintainer needs to step in to define which format is preferred.

[sokra]

Thanks