Merge pull request #206 from bholloway/remove-engine-option

Remove engine option
bholloway · Jun 9, 2021 · 08e153c · 08e153c
2 parents 10f9cdb + 921ddda
commit 08e153c
Show file tree

Hide file tree

Showing 10 changed files with 97 additions and 264 deletions.
diff --git a/packages/resolve-url-loader/index.js b/packages/resolve-url-loader/index.js
@@ -4,23 +4,23 @@
  */
 'use strict';
 
-var os                = require('os'),
-    path              = require('path'),
-    fs                = require('fs'),
-    util              = require('util'),
-    loaderUtils       = require('loader-utils'),
-    SourceMapConsumer = require('source-map').SourceMapConsumer;
+const os                = require('os');
+const path              = require('path');
+const fs                = require('fs');
+const util              = require('util');
+const loaderUtils       = require('loader-utils');
+const SourceMapConsumer = require('source-map').SourceMapConsumer;
 
-var adjustSourceMap = require('adjust-sourcemap-loader/lib/process');
+const adjustSourceMap = require('adjust-sourcemap-loader/lib/process');
 
-var valueProcessor   = require('./lib/value-processor'),
-    joinFn           = require('./lib/join-function'),
-    logToTestHarness = require('./lib/log-to-test-harness');
+const valueProcessor  = require('./lib/value-processor');
+const joinFn           = require('./lib/join-function');
+const logToTestHarness = require('./lib/log-to-test-harness');
 
 const DEPRECATED_OPTIONS = {
   engine: [
     'DEP_RESOLVE_URL_LOADER_OPTION_ENGINE',
-    'the "engine" option is deprecated, "postcss" engine is the default, there are no other available engines'
+    'the "engine" option has been removed, "postcss" is the only available parser'
   ],
   keepQuery: [
     'DEP_RESOLVE_URL_LOADER_OPTION_KEEP_QUERY',
@@ -55,7 +55,7 @@ function resolveUrlLoader(content, sourceMap) {
   /* jshint validthis:true */
 
   // details of the file being processed
-  var loader = this;
+  const loader = this;
 
   // a relative loader.context is a problem
   if (/^\./.test(loader.context)) {
@@ -66,24 +66,20 @@ function resolveUrlLoader(content, sourceMap) {
   }
 
   // infer webpack version from new loader features
-  var isWebpackGte5 = 'getOptions' in loader && typeof loader.getOptions === 'function';
+  const isWebpackGte5 = 'getOptions' in loader && typeof loader.getOptions === 'function';
 
-  // webpack 1: prefer loader query, else options object
-  // webpack 2: prefer loader options
-  // webpack 3: deprecate loader.options object
-  // webpack 4: loader.options no longer defined
-  var rawOptions = loaderUtils.getOptions(loader),
-      options    = Object.assign(
+  // use loader.getOptions where available otherwise use loaderUtils
+  const rawOptions = isWebpackGte5 ? loader.getOptions() : loaderUtils.getOptions(loader);
+  const options = Object.assign(
         {
           sourceMap: loader.sourceMap,
-          engine   : 'postcss',
           silent   : false,
           removeCR : os.EOL.includes('\r'),
           root     : false,
           debug    : false,
           join     : joinFn.defaultJoin
         },
-        rawOptions
+        rawOptions,
       );
 
   // maybe log options for the test harness
@@ -95,7 +91,7 @@ function resolveUrlLoader(content, sourceMap) {
   }
 
   // deprecated options
-  var deprecatedItems = Object.entries(DEPRECATED_OPTIONS).filter(([key]) => key in rawOptions);
+  const deprecatedItems = Object.entries(DEPRECATED_OPTIONS).filter(([key]) => key in rawOptions);
   if (deprecatedItems.length) {
     deprecatedItems.forEach(([, value]) => handleAsDeprecated(...value));
   }
@@ -114,7 +110,7 @@ function resolveUrlLoader(content, sourceMap) {
   }
 
   // validate the result of calling the join option
-  var joinProper = options.join(options, loader);
+  const joinProper = options.join(options, loader);
   if (typeof joinProper !== 'function') {
     return handleAsError(
       'loader misconfiguration',
@@ -129,7 +125,7 @@ function resolveUrlLoader(content, sourceMap) {
 
   // validate root option
   if (typeof options.root === 'string') {
-    var isValid = (options.root === '') ||
+    const isValid = (options.root === '') ||
       (path.isAbsolute(options.root) && fs.existsSync(options.root) && fs.statSync(options.root).isDirectory());
 
     if (!isValid) {
@@ -149,7 +145,8 @@ function resolveUrlLoader(content, sourceMap) {
   loader.cacheable();
 
   // incoming source-map
-  var sourceMapConsumer, absSourceMap;
+  let absSourceMap = null;
+  let sourceMapConsumer = null;
   if (sourceMap) {
 
     // support non-standard string encoded source-map (per less-loader)
@@ -186,20 +183,10 @@ function resolveUrlLoader(content, sourceMap) {
     );
   }
 
-  // choose a CSS engine
-  var enginePath    = /^[\w-]+$/.test(options.engine) && path.join(__dirname, 'lib', 'engine', options.engine + '.js');
-  var isValidEngine = fs.existsSync(enginePath);
-  if (!isValidEngine) {
-    return handleAsError(
-      'loader misconfiguration',
-      '"engine" option is not valid'
-    );
-  }
-
   // allow engine to throw at initialisation
-  var engine;
+  let engine = null;
   try {
-    engine = require(enginePath);
+    engine = require('./lib/engine/postcss');
   } catch (error) {
     return handleAsError(
       'error initialising',
@@ -208,7 +195,7 @@ function resolveUrlLoader(content, sourceMap) {
   }
 
   // process async
-  var callback = loader.async();
+  const callback = loader.async();
   Promise
     .resolve(engine(loader.resourcePath, content, {
       outputSourceMap     : !!options.sourceMap,
@@ -234,7 +221,7 @@ function resolveUrlLoader(content, sourceMap) {
       //  webpack4 and earlier: source-map sources are relative to the file being processed
       //  webpack5: source-map sources are relative to the project root but without a leading slash
       if (options.sourceMap) {
-        var finalMap = adjustSourceMap(loader, {
+        const finalMap = adjustSourceMap(loader, {
           format: isWebpackGte5 ? 'projectRelative' : 'sourceRelative'
         }, result.map);
         callback(null, result.content, finalMap);

diff --git a/packages/resolve-url-loader/lib/engine/fail-initialisation.js b/packages/resolve-url-loader/lib/engine/fail-initialisation.js
diff --git a/packages/resolve-url-loader/lib/engine/fail-processing.js b/packages/resolve-url-loader/lib/engine/fail-processing.js
diff --git a/packages/test-my-cli/operations/exec.js b/packages/test-my-cli/operations/exec.js
@@ -63,7 +63,7 @@ exports.create = (command) => {
     )),
     withTime(({index, root, caller, cwd, env, meta}, {onActivity}) =>
       new Promise((resolve) => {
-        let stdout = '', stderr = '', interval = 0;
+        let stdout = '', stderr = '', interval = 0, caughtError = null;
         const child = spawn(cmd, args, {cwd, env, shell: true, stdio: 'pipe'});
         addOrRemove(true);
 
@@ -72,8 +72,8 @@ exports.create = (command) => {
           const field = isAdd ? 'addListener' : 'removeListener';
           child.stdout[field]('data', onStdout);
           child.stderr[field]('data', onStderr);
-          child.on('exit', onExit);
-          child.on('error', onError);
+          child[field]('error', onError);
+          child[field]('close', onClose);
 
           if (isAdd) {
             child.stdout.setEncoding('utf8');
@@ -92,14 +92,16 @@ exports.create = (command) => {
           stderr += data;
         }
 
-        function onExit(code) {
-          addOrRemove(false);
-          resolve({index, root, caller, cwd, env, meta, code, stdout, stderr});
+        function onError(error) {
+          caughtError = error;
         }
 
-        function onError(error) {
+        function onClose(code) {
           addOrRemove(false);
-          resolve({index, root, caller, cwd, env, meta, code: 1, stdout, stderr: error.toString()});
+          resolve({
+            index, root, caller, cwd, env, meta, code, stdout,
+            stderr: caughtError ? caughtError.toString() : stderr
+          });
         }
       })
     ),

diff --git a/test/cases/absolute-asset.js b/test/cases/absolute-asset.js
@@ -13,7 +13,7 @@ const {
 } = require('./common/exec');
 const {assertCssContent} = require('../lib/assert');
 const {
-  onlyMeta, assertWebpackOk, assertWebpackNotOk, assertNoErrors, assertNoMessages, assertStdout,
+  onlyOS, onlyMeta, assertWebpackOk, assertWebpackNotOk, assertNoErrors, assertNoMessages, assertStdout,
   assertCssSourceMapComment, assertCssFile, assertSourceMapFile, assertAssetError
 } = require('../lib/assert');
 
@@ -90,11 +90,14 @@ module.exports = test(
         // if webpack passes it is incidental but lets check anyway
         testDebug(
           all(buildDevNormal, buildProdNormal)(
-            onlyMeta('meta.version.webpack == 4')(
+            compose(onlyOS('posix'), onlyMeta('meta.version.webpack == 4'))(
               assertWebpackNotOk,
               assertAssetError
             ),
-            onlyMeta('meta.version.webpack >= 5')(
+            all(
+              onlyOS('windows'),
+              compose(onlyOS('posix'), onlyMeta('meta.version.webpack >= 5'))
+            )(
               assertWebpackOk,
               assertNoErrors,
               assertNoMessages

diff --git a/test/cases/common/test/invalid.js b/test/cases/common/test/invalid.js
@@ -6,71 +6,6 @@ const {test, layer, env} = require('test-my-cli');
 const {assertStderr} = require('../../../lib/assert');
 const {escapeStr} = require('../../../lib/util');
 
-exports.testKeepQuery = (...rest) =>
-  test(
-    'keepQuery=true',
-    layer()(
-      env({
-        LOADER_OPTIONS: {keepQuery: true},
-        OUTPUT: 'keepQuery'
-      }),
-      ...rest,
-      test('validate', assertStderr('options.keepQuery')(1)`keepQuery: true`)
-    )
-  );
-
-exports.testAbsolute = (...rest) =>
-  test(
-    'absolute=true',
-    layer()(
-      env({
-        LOADER_OPTIONS: {absolute: true},
-        OUTPUT: 'absolute'
-      }),
-      ...rest,
-      test('validate', assertStderr('options.absolute')(1)`absolute: true`)
-    )
-  );
-
-exports.testAttempts = (...rest) =>
-  test(
-    'attempts=N',
-    layer()(
-      env({
-        LOADER_OPTIONS: {attempts: 1},
-        OUTPUT: 'attempts'
-      }),
-      ...rest,
-      test('validate', assertStderr('options.attempts')(1)`attempts: 1`)
-    )
-  );
-
-exports.testIncludeRoot = (...rest) =>
-  test(
-    'includeRoot=true',
-    layer()(
-      env({
-        LOADER_OPTIONS: {includeRoot: true},
-        OUTPUT: 'includeRoot'
-      }),
-      ...rest
-    ),
-    test('validate', assertStderr('options.includeRoot')(1)`includeRoot: true`)
-  );
-
-exports.testFail = (...rest) =>
-  test(
-    'fail=true',
-    layer()(
-      env({
-        LOADER_OPTIONS: {fail: true},
-        OUTPUT: 'fail'
-      }),
-      ...rest,
-      test('validate', assertStderr('options.fail')(1)`fail: true`)
-    )
-  );
-
 exports.testNonFunctionJoin1 = (...rest) =>
   test(
     'join1=!function',
@@ -148,42 +83,3 @@ exports.testNonExistentRoot = (...rest) =>
       test('validate', assertStderr('options.root')(1)`root: "${escapeStr(process.cwd())}[^"]+foo"`)
     )
   );
-
-exports.testSilent = (...rest) =>
-  test(
-    'silent=true',
-    layer()(
-      env({
-        LOADER_OPTIONS: {silent: true},
-        OUTPUT: 'silent'
-      }),
-      ...rest,
-      test('validate', assertStderr('options.silent')(1)`silent: true`)
-    )
-  );
-
-exports.testEngineFailInitialisation = (...rest) =>
-  test(
-    'engine=fail-initialisation',
-    layer()(
-      env({
-        LOADER_OPTIONS: {engine: 'fail-initialisation'},
-        OUTPUT: 'engine-fail-initialisation'
-      }),
-      ...rest,
-      test('validate', assertStderr('options.engine')(1)`engine: "fail-initialisation"`)
-    )
-  );
-
-exports.testEngineFailProcessing = (...rest) =>
-  test(
-    'engine=fail-processing',
-    layer()(
-      env({
-        LOADER_OPTIONS: {engine: 'fail-processing'},
-        OUTPUT: 'engine-fail-processing'
-      }),
-      ...rest,
-      test('validate', assertStderr('options.engine')(1)`engine: "fail-processing"`)
-    )
-  );