From 5654fa4083e6367378e754104e450c6e3c65d7a1 Mon Sep 17 00:00:00 2001 From: Ben Holloway Date: Wed, 9 Jun 2021 12:50:37 +1000 Subject: [PATCH] remove the engine option and related tests, use getOptions from loader where available --- packages/resolve-url-loader/index.js | 65 +++++------ .../lib/engine/fail-initialisation.js | 7 -- .../lib/engine/fail-processing.js | 18 --- test/cases/common/test/invalid.js | 104 ------------------ test/cases/common/test/valid.js | 51 +++++---- test/cases/misconfiguration.js | 48 +------- test/index.js | 35 +++--- 7 files changed, 77 insertions(+), 251 deletions(-) delete mode 100644 packages/resolve-url-loader/lib/engine/fail-initialisation.js delete mode 100644 packages/resolve-url-loader/lib/engine/fail-processing.js diff --git a/packages/resolve-url-loader/index.js b/packages/resolve-url-loader/index.js index d989bdf..0705f01 100644 --- 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 deleted file mode 100644 index cc26000..0000000 --- a/packages/resolve-url-loader/lib/engine/fail-initialisation.js +++ /dev/null @@ -1,7 +0,0 @@ -/* - * MIT License http://opensource.org/licenses/MIT - * Author: Ben Holloway @bholloway - */ -'use strict'; - -throw new Error('This "engine" is designed to fail at require time, for testing purposes only'); diff --git a/packages/resolve-url-loader/lib/engine/fail-processing.js b/packages/resolve-url-loader/lib/engine/fail-processing.js deleted file mode 100644 index bf5ba06..0000000 --- a/packages/resolve-url-loader/lib/engine/fail-processing.js +++ /dev/null @@ -1,18 +0,0 @@ -/* - * MIT License http://opensource.org/licenses/MIT - * Author: Ben Holloway @bholloway - */ -'use strict'; - -/** - * Process the given CSS content into reworked CSS content. - */ -function process() { - return new Promise(function (_, reject) { - setTimeout(function () { - reject(new Error('This "engine" is designed to fail at processing time, for testing purposes only')); - }, 100); - }); -} - -module.exports = process; diff --git a/test/cases/common/test/invalid.js b/test/cases/common/test/invalid.js index e4ed175..3ddfd7d 100644 --- 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"`) - ) - ); diff --git a/test/cases/common/test/valid.js b/test/cases/common/test/valid.js index d72fd66..f9541a8 100644 --- a/test/cases/common/test/valid.js +++ b/test/cases/common/test/valid.js @@ -1,30 +1,23 @@ 'use strict'; const sequence = require('promise-compose'); -const {test, layer, meta, env} = require('test-my-cli'); +const {test, layer, env} = require('test-my-cli'); const {assertStderr} = require('../../../lib/assert'); const {escapeStr} = require('../../../lib/util'); -exports.testBase = (engine) => (...elements) => - test( - `engine=${engine}`, - layer(engine)( - meta({ - engine - }), - env({ - DEVTOOL: true, - LOADER_OPTIONS: (engine === 'postcss') ? {sourceMap: true} : {sourceMap: true, engine}, - LOADER_JOIN: '', - CSS_OPTIONS: {sourceMap: true} - }), - ...elements, - test('validate', sequence( - assertStderr('options.sourceMap')(1)`sourceMap: true`, - assertStderr('options.engine')(1)`engine: "${engine}"` - )) - ) +exports.testBase = (...elements) => + layer()( + env({ + DEVTOOL: true, + LOADER_OPTIONS: {sourceMap: true}, + LOADER_JOIN: '', + CSS_OPTIONS: {sourceMap: true} + }), + ...elements, + test('validate', sequence( + assertStderr('options.sourceMap')(1)`sourceMap: true`, + )) ); exports.testWithLabel = (label) => (...elements) => @@ -38,7 +31,25 @@ exports.testWithLabel = (label) => (...elements) => ) ); +exports.testWithOption = (option) => { + const [key] = Object.keys(option); + const value = option[key]; + return (...rest) => + test( + `${key}=${JSON.stringify(value)}`, + layer()( + env({ + LOADER_OPTIONS: {[key]: value}, + OUTPUT: key + }), + ...rest, + test('validate', assertStderr(`options.${key}`)(1)`${key}: ${JSON.stringify(value)}`) + ) + ); +}; + exports.testDefault = exports.testWithLabel('default'); +exports.testSilent = exports.testWithOption({ silent: true }); exports.testDebug = (...elements) => test( diff --git a/test/cases/misconfiguration.js b/test/cases/misconfiguration.js index ce3731d..ceed3f4 100644 --- a/test/cases/misconfiguration.js +++ b/test/cases/misconfiguration.js @@ -2,15 +2,13 @@ const {join} = require('path'); const outdent = require('outdent'); -const sequence = require('promise-compose'); const compose = require('compose-function'); const {test, layer, fs, env, cwd} = require('test-my-cli'); const {trim} = require('../lib/util'); const {rebaseToCache} = require('../lib/higher-order'); const { - all, testDefault, testSilent, testKeepQuery, testAbsolute, testAttempts, testEngineFailInitialisation, - testEngineFailProcessing, testIncludeRoot, testFail, testNonFunctionJoin1, testWrongArityJoin1, testNonFunctionJoin2, + all, testWithOption, testDefault, testSilent, testNonFunctionJoin1, testWrongArityJoin1, testNonFunctionJoin2, testWrongArityJoin2, testNonStringRoot, testNonExistentRoot } = require('./common/test'); const {buildDevNormal, buildProdNormal} = require('./common/exec'); @@ -38,24 +36,6 @@ const assertMisconfigError = (message) => assertStdout('error')([1, 4])` [ ]+${message} `; -// Allow 1-4 errors -// - known-issue in extract-text-plugin, failed loaders will rerun webpack>=2 -// - webpack may repeat errors with a header line taken from the parent loader -const assertInitialisationError = assertStdout('error')([1, 4])` - ^[ ]*ERROR[^\n]* - ([^\n]+\n){0,2}[^\n]*resolve-url-loader:[ ]*error initialising - [ ]+This "engine" is designed to fail at require time, for testing purposes only - `; - -// Allow 1-4 errors -// - known-issue in extract-text-plugin, failed loaders will rerun webpack>=2 -// - webpack may repeat errors with a header line taken from the parent loader -const assertProcessingError = assertStdout('error')([1, 4])` - ^[ ]*ERROR[^\n]* - ([^\n]+\n){0,2}[^\n]*resolve-url-loader:[ ]*error processing CSS - [ ]+This "engine" is designed to fail at processing time, for testing purposes only - `; - module.exports = test( 'misconfiguration', layer('misconfiguration')( @@ -73,23 +53,7 @@ module.exports = test( env({ ENTRY: join('src', 'index.scss') }), - testEngineFailInitialisation( - all(testDefault, testSilent)( - all(buildDevNormal, buildProdNormal)( - assertWebpackNotOk, - assertInitialisationError - ) - ) - ), - testEngineFailProcessing( - all(testDefault, testSilent)( - all(buildDevNormal, buildProdNormal)( - assertWebpackNotOk, - assertProcessingError - ) - ) - ), - testAttempts( + testWithOption({ attempts: 1 })( testDefault( buildDevNormal( assertWebpackOk, @@ -119,7 +83,7 @@ module.exports = test( ) ) ), - testKeepQuery( + testWithOption({ keepQuery: true })( testDefault( buildDevNormal( assertWebpackOk, @@ -149,7 +113,7 @@ module.exports = test( ) ) ), - testAbsolute( + testWithOption({ absolute: true })( testDefault( buildDevNormal( assertWebpackOk, @@ -179,7 +143,7 @@ module.exports = test( ) ) ), - testIncludeRoot( + testWithOption({ includeRoot: true })( testDefault( buildDevNormal( assertWebpackOk, @@ -209,7 +173,7 @@ module.exports = test( ) ) ), - testFail( + testWithOption({ fail: true })( testDefault( buildDevNormal( assertWebpackOk, diff --git a/test/index.js b/test/index.js index 7001d3c..abe9fbc 100644 --- a/test/index.js +++ b/test/index.js @@ -17,11 +17,6 @@ const {testBase} = require('./cases/common/test'); const PLATFORMS_DIR = compose(normalize, join)(__dirname, '..', 'packages', 'resolve-url-loader', 'test'); const CASES_DIR = join(__dirname, 'cases'); -const testCaseVsEngine = ([_, engineName, caseName]) => { - const split = caseName.split('.'); - return (split.length === 1) || (split[1] === engineName); -}; - const testIncluded = process.env.ONLY ? (arr) => { const patterns = process.env.ONLY.split(' ').map(v => v.trim()); @@ -45,13 +40,13 @@ const getVersionHash = platform => const epoch = Math.round(Date.now() / 1000).toString().padStart(10, 0); console.log(`timestamp: ${epoch}`); -// platforms, engines, cases +// platforms, cases const tests = permute( readdirSync(PLATFORMS_DIR), - ['postcss'], - readdirSync(CASES_DIR).filter((v) => v.endsWith('.js')).map((v) => v.split('.').slice(0, -1).join('.')) + readdirSync(CASES_DIR) + .filter((v) => v.endsWith('.js')) + .map((v) => v.split('.').slice(0, -1).join('.')) ) - .filter(testCaseVsEngine) .filter(testIncluded); const filterTests = (...terms) => @@ -128,18 +123,16 @@ filterTests() debug: (process.env.DEBUG === 'exec') } }), - ...filterTests(platform).map(engine => - testBase(engine)( - env({ - PATH: dirname(process.execPath), - RESOLVE_URL_LOADER_TEST_HARNESS: 'stderr' - }), - meta({ - cacheDir: join(process.cwd(), 'tmp', '.cache', platform), - version: getVersionHash(platform) - }), - ...filterTests(platform, engine).map(caseName => require(join(CASES_DIR, caseName))) - ) + testBase( + env({ + PATH: dirname(process.execPath), + RESOLVE_URL_LOADER_TEST_HARNESS: 'stderr' + }), + meta({ + cacheDir: join(process.cwd(), 'tmp', '.cache', platform), + version: getVersionHash(platform) + }), + ...filterTests(platform).map(caseName => require(join(CASES_DIR, caseName))) ) ) );