Skip to content

Remove engine option #206

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.

Sign up for GitHub

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
bholloway merged 3 commits into from
Jun 9, 2021
Merged

Remove engine option #206

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5654fa4
remove the engine option and related tests, use getOptions from loade…
bholloway Jun 9, 2021
d6a7d7d
fix premature exit of test process on windows
bholloway Jun 9, 2021
921ddda
fix failing test on windows using correct assertion
bholloway Jun 9, 2021
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
65 changes: 26 additions & 39 deletions packages/resolve-url-loader/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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)) {
Expand All @@ -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
Expand All @@ -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));
}
Expand All @@ -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',
Expand All @@ -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) {
Expand All @@ -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)
Expand Down Expand Up @@ -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',
Expand All @@ -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,
Expand All @@ -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);
Expand Down
7 changes: 0 additions & 7 deletions packages/resolve-url-loader/lib/engine/fail-initialisation.js
View file
Open in desktop

This file was deleted.

18 changes: 0 additions & 18 deletions packages/resolve-url-loader/lib/engine/fail-processing.js
View file
Open in desktop

This file was deleted.

18 changes: 10 additions & 8 deletions packages/test-my-cli/operations/exec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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');
Expand All @@ -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
});
}
})
),
Expand Down
9 changes: 6 additions & 3 deletions test/cases/absolute-asset.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand Down Expand Up @@ -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
Expand Down
104 changes: 0 additions & 104 deletions test/cases/common/test/invalid.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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"`)
)
);
Loading