Skip to content

Commit

Permalink
fix joinFn iterator, pass resourcePath to joinFn
Browse files Browse the repository at this point in the history
  • Loading branch information
bholloway committed Sep 8, 2018
1 parent 920fdc7 commit b74afac
Show file tree
Hide file tree
Showing 15 changed files with 93 additions and 45 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"packages/*"
],
"scripts": {
"lint": "jshint --exclude **/node_modules test",
"lint": "jshint packages --exclude **/node_modules",
"test:unit": "tape **/*.spec.js | tap-diff",
"test:e2e": "tape test | tap-diff"
},
Expand Down
8 changes: 4 additions & 4 deletions packages/resolve-url-loader/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,16 +88,16 @@ function resolveUrlLoader(content, sourceMap) {
'loader misconfiguration',
'"join" option must be a Function'
);
} else if (options.join.length !== 1) {
} else if (options.join.length !== 2) {
return handleAsError(
'loader misconfiguration',
'"join" Function must take exactly 1 argument, an options hash'
'"join" Function must take exactly 2 arguments (filename and options hash)'
);
}

// validate root option
if (typeof options.root === 'string') {
const isValid = (options.root === '') ||
var isValid = (options.root === '') ||
(path.isAbsolute(options.root) && fs.existsSync(options.root) && fs.statSync(options.root).isDirectory());

if (!isValid) {
Expand Down Expand Up @@ -164,7 +164,7 @@ function resolveUrlLoader(content, sourceMap) {
Promise
.resolve(require(enginePath)(loader.resourcePath, content, {
outputSourceMap : !!options.sourceMap,
transformDeclaration: valueProcessor(loader.context, options),
transformDeclaration: valueProcessor(loader.resourcePath, options),
absSourceMap : absSourceMap,
sourceMapConsumer : sourceMapConsumer
}))
Expand Down
65 changes: 37 additions & 28 deletions packages/resolve-url-loader/lib/join-function.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,39 +4,47 @@
*/
'use strict';

var path = require('path'),
fs = require('fs'),
compose = require('compose-function');
var path = require('path'),
fs = require('fs'),
compose = require('compose-function'),
Iterator = require('es6-iterator');

var PACKAGE_NAME = require('../package.json').name;

var simpleJoin = compose(path.normalize, path.join);


/**
* The default join function iterates over possible base paths until a suitable join is found.
*
* The first base path is the fallback for the case where no base paths match.
*
* Where the uri is absolute (base or iterator absent) then `options.root` is used as the base path.
* The first base path is used as fallback for the case where none of the base paths can locate the actual file.
*
* @type {function}
*/
exports.defaultJoin = createJoinForPredicate(
function predicate(uri, base, i, next) {
function predicate(_, uri, base, i, next) {
var absolute = simpleJoin(base, uri);
return fs.existsSync(absolute) ? absolute : next((i === 0) ? absolute : null);
},
'defaultJoin'
);


/**
* Define a join function by a predicate tests possible base paths from an iterator.
* Define a join function by a predicate that tests possible base paths from an iterator.
*
* The `predicate` is of the form:
*
* ```
* function(filename, uri, base, i, next):string|null
* ```
*
* The predicate accepts the `uri`, `base`, `index` and `next` function. Given the uri and base it should either
* return an absolute path as a success value or return a call to `next()`. Any value given to `next()` is considered a
* fallback value to use if success does not occur.
* Given the uri and base it should either return:
* - an absolute path success
* - a call to `next(null)` as failure
* - a call to `next(absolute)` where absolute is placeholder and the iterator continues
*
* The value given to `next(...)` is only used if success does not eventually occur.
*
* The `file` value is typically unused but useful if you would like to differentiate behaviour.
*
* You can write a much simpler function than this if you have specific requirements.
*
Expand All @@ -47,9 +55,10 @@ function createJoinForPredicate(predicate, name) {
/**
* A factory for a join function with logging.
*
* @param {string} filename The current file being processed
* @param {{debug:function|boolean,root:string}} options An options hash
*/
function join(options) {
function join(filename, options) {
var log = createDebugLogger(options.debug);

/**
Expand All @@ -58,24 +67,25 @@ function createJoinForPredicate(predicate, name) {
* For absolute uri only `uri` will be provided. In this case we substitute any `root` given in options.
*
* @param {string} uri A uri path, relative or absolute
* @param {string|{next:function():string}} [baseOrIteratorOrAbsent] Optional absolute base path or iterator thereof
* @param {string|Iterator.<string>} [baseOrIteratorOrAbsent] Optional absolute base path or iterator thereof
* @return {string} Just the uri where base is empty or the uri appended to the base
*/
return function joinProper(uri, baseOrIteratorOrAbsent) {
var iterator =
(typeof baseOrIteratorOrAbsent === 'undefined') && {next() { return options.root; }} ||
(typeof baseOrIteratorOrAbsent === 'string' ) && {next() { return baseOrIteratorOrAbsent; }} ||
(typeof baseOrIteratorOrAbsent === 'undefined') && new Iterator([options.root ]) ||
(typeof baseOrIteratorOrAbsent === 'string' ) && new Iterator([baseOrIteratorOrAbsent]) ||
baseOrIteratorOrAbsent;

var result = runIterator([]);
log(createJoinMsg, [uri, result, result.isFound]);
log(createJoinMsg, [filename, uri, result, result.isFound]);

return (typeof result.absolute === 'string') ? result.absolute : uri;

function runIterator(accumulator) {
var base = iterator.next();
var nextItem = iterator.next();
var base = !nextItem.done && nextItem.value;
if (typeof base === 'string') {
var element = predicate(uri, base, accumulator.length, next);
var element = predicate(filename, uri, base, accumulator.length, next);

if ((typeof element === 'string') && path.isAbsolute(element)) {
return Object.assign(
Expand Down Expand Up @@ -106,25 +116,25 @@ function createJoinForPredicate(predicate, name) {
}

return Object.assign(join, name && {
valueOf: toString,
valueOf : toString,
toString: toString
});
}

exports.createJoinForIterator = createJoinForPredicate;

exports.createJoinForPredicate = createJoinForPredicate;

/**
* Format a debug message.
*
* @param {string} file The file being processed by webpack
* @param {string} uri A uri path, relative or absolute
* @param {Array.<string>} bases Absolute base paths up to and including the found one
* @param {boolean} isFound Indicates the last base was correct
* @return {string} Formatted message
*/
function createJoinMsg(uri, bases, isFound) {
return [PACKAGE_NAME + ': ' + uri]
.concat(bases.map(formatBase).filter(Boolean))
function createJoinMsg(file, uri, bases, isFound) {
return [PACKAGE_NAME + ': ' + pathToString(file) + ': ' + uri]
.concat(bases.map(pathToString).filter(Boolean))
.concat(isFound ? 'FOUND' : 'NOT FOUND')
.join('\n ');

Expand All @@ -134,7 +144,7 @@ function createJoinMsg(uri, bases, isFound) {
* @param {string} absolute An absolute path
* @return {string} A relative or absolute path
*/
function formatBase(absolute) {
function pathToString(absolute) {
if (!absolute) {
return null;
} else {
Expand All @@ -149,7 +159,6 @@ function createJoinMsg(uri, bases, isFound) {

exports.createJoinMsg = createJoinMsg;


/**
* A factory for a log function predicated on the given debug parameter.
*
Expand Down
4 changes: 4 additions & 0 deletions packages/resolve-url-loader/lib/log-to-test-harness.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
/*
* MIT License http://opensource.org/licenses/MIT
* Author: Ben Holloway @bholloway
*/
'use strict';

var stream = require('stream');
Expand Down
7 changes: 4 additions & 3 deletions packages/resolve-url-loader/lib/value-processor.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,14 @@ var path = require('path'),
/**
* Create a value processing function for a given file path.
*
* @param {string} directory The directory of the current file being processed
* @param {string} filename The current file being processed
* @param {{absolute:string, keepQuery:boolean, join:function, root:string}} options Options hash
* @return {function} value processing function
*/
function valueProcessor(directory, options) {
function valueProcessor(filename, options) {
var URL_STATEMENT_REGEX = /(url\s*\()\s*(?:(['"])((?:(?!\2).)*)(\2)|([^'"](?:(?!\)).)*[^'"]))\s*(\))/g;
var join = options.join(options);
var directory = path.dirname(filename);
var join = options.join(filename, options);

/**
* Process the given CSS declaration value.
Expand Down
1 change: 1 addition & 0 deletions packages/resolve-url-loader/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
"camelcase": "^4.1.0",
"compose-function": "^3.0.3",
"convert-source-map": "^1.5.1",
"es6-iterator": "^2.0.3",
"loader-utils": "^1.1.0",
"lodash.defaults": "^4.0.0",
"postcss": "^7.0.0",
Expand Down
2 changes: 1 addition & 1 deletion test/cases/absolute-asset.js
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ const assertSourceMapSources = assertSourceMapContent([
]);

const assertDebugMessages = assertStdout('debug')(1)`
^resolve-url-loader:[ ]*${process.cwd()}.*${join('images', 'img.jpg')}
^resolve-url-loader:[^:]+:[ ]*${process.cwd()}.*${join('images', 'img.jpg')}
[ ]+FOUND$
`;

Expand Down
2 changes: 1 addition & 1 deletion test/cases/adjacent-asset.js
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ const assertSourceMapSources = assertSourceMapContent([
]);

const assertDebugMessages = assertStdout('debug')(1)`
^resolve-url-loader:[ ]*${'../../../packageB/images/img.jpg'}
^resolve-url-loader:[^:]+:[ ]*${'../../../packageB/images/img.jpg'}
[ ]+${'./src/feature'}
[ ]+FOUND$
`;
Expand Down
4 changes: 2 additions & 2 deletions test/cases/common/tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -161,10 +161,10 @@ exports.testNonFunctionJoin = (...rest) =>

exports.testWrongArityJoin = (...rest) =>
test(
'join=!arity1',
'join=!arity2',
layer()(
env({
LOADER_JOIN: 'return (a, b) => a;',
LOADER_JOIN: 'return (a) => a;',
OUTPUT: 'wrong-arity-join'
}),
...rest,
Expand Down
2 changes: 1 addition & 1 deletion test/cases/deep-asset.js
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ const assertSourceMapSources = assertSourceMapContent([
]);

const assertDebugMessages = assertStdout('debug')(1)`
^resolve-url-loader:[ ]*${'images/img.jpg'}
^resolve-url-loader:[^:]+:[ ]*${'images/img.jpg'}
[ ]+${'./src/feature'}
[ ]+FOUND$
`;
Expand Down
2 changes: 1 addition & 1 deletion test/cases/immediate-asset.js
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ const assertSourceMapSources = assertSourceMapContent([
]);

const assertDebugMessages = assertStdout('debug')(1)`
^resolve-url-loader:[ ]*${'img.jpg'}
^resolve-url-loader:[^:]+:[ ]*${'img.jpg'}
[ ]+${'./src/feature'}
[ ]+FOUND$
`;
Expand Down
2 changes: 1 addition & 1 deletion test/cases/misconfiguration.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ const assertNonFunctionJoinError = assertMisconfigError(
);

const assertWrongArityJoinError = assertMisconfigError(
'"join" Function must take exactly 1 argument, an options hash'
'"join" Function must take exactly 2 arguments (filename and options hash)'
);

const assertNonExistentRootError = assertMisconfigError(
Expand Down
2 changes: 1 addition & 1 deletion test/cases/root-relative-asset.js
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ const assertSourceMapSources = assertSourceMapContent([
]);

const assertDebugMessages = assertStdout('debug')(1)`
^resolve-url-loader:[ ]*${'/images/img.jpg'}
^resolve-url-loader:[^:]+:[ ]*${'/images/img.jpg'}
[ ]+${'.'}
[ ]+FOUND$
`;
Expand Down
2 changes: 1 addition & 1 deletion test/cases/shallow-asset.js
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ const assertSourceMapSources = assertSourceMapContent([
]);

const assertDebugMessages = assertStdout('debug')(1)`
^resolve-url-loader:[ ]*${'../images/img.jpg'}
^resolve-url-loader:[^:]+:[ ]*${'../images/img.jpg'}
[ ]+${'./src/feature'}
[ ]+FOUND$
`;
Expand Down
33 changes: 33 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,12 @@ css@^2.0.0:
source-map-resolve "^0.5.1"
urix "^0.1.0"

d@1:
version "1.0.0"
resolved "https://registry.npmjs.org/d/-/d-1.0.0.tgz#754bb5bfe55451da69a58b94d45f4c5b0462d58f"
dependencies:
es5-ext "^0.10.9"

date-now@^0.1.4:
version "0.1.4"
resolved "https://registry.npmjs.org/date-now/-/date-now-0.1.4.tgz#eaf439fd4d4848ad74e5cc7dbef200672b9e345b"
Expand Down Expand Up @@ -377,10 +383,33 @@ es-to-primitive@^1.1.1:
is-date-object "^1.0.1"
is-symbol "^1.0.1"

es5-ext@^0.10.35, es5-ext@^0.10.9, es5-ext@~0.10.14:
version "0.10.46"
resolved "https://registry.npmjs.org/es5-ext/-/es5-ext-0.10.46.tgz#efd99f67c5a7ec789baa3daa7f79870388f7f572"
dependencies:
es6-iterator "~2.0.3"
es6-symbol "~3.1.1"
next-tick "1"

es6-iterator@^2.0.3, es6-iterator@~2.0.3:
version "2.0.3"
resolved "https://registry.npmjs.org/es6-iterator/-/es6-iterator-2.0.3.tgz#a7de889141a05a94b0854403b2d0a0fbfa98f3b7"
dependencies:
d "1"
es5-ext "^0.10.35"
es6-symbol "^3.1.1"

es6-promisify@^6.0.0:
version "6.0.0"
resolved "https://registry.npmjs.org/es6-promisify/-/es6-promisify-6.0.0.tgz#b526a75eaa5ca600e960bf3d5ad98c40d75c7203"

es6-symbol@^3.1.1, es6-symbol@~3.1.1:
version "3.1.1"
resolved "https://registry.npmjs.org/es6-symbol/-/es6-symbol-3.1.1.tgz#bf00ef4fdab6ba1b46ecb7b629b4c7ed5715cc77"
dependencies:
d "1"
es5-ext "~0.10.14"

escape-string-regexp@^1.0.2, escape-string-regexp@^1.0.5:
version "1.0.5"
resolved "https://registry.npmjs.org/escape-string-regexp/-/escape-string-regexp-1.0.5.tgz#1b61c0562190a8dff6ae3bb2cf0200ca130b86d4"
Expand Down Expand Up @@ -939,6 +968,10 @@ nanomatch@^1.2.9:
snapdragon "^0.8.1"
to-regex "^3.0.1"

next-tick@1:
version "1.0.0"
resolved "https://registry.npmjs.org/next-tick/-/next-tick-1.0.0.tgz#ca86d1fe8828169b0120208e3dc8424b9db8342c"

nice-try@^1.0.4:
version "1.0.4"
resolved "https://registry.npmjs.org/nice-try/-/nice-try-1.0.4.tgz#d93962f6c52f2c1558c0fbda6d512819f1efe1c4"
Expand Down

0 comments on commit b74afac

Please sign in to comment.