Skip to content

Commit

Permalink
Use node's path module where possible
Browse files Browse the repository at this point in the history 
Additionally, all stringifyRequest tests are now executed on every OS.
  • Loading branch information
@jhnns
jhnns committed Feb 17, 2017
1 parent 87cc614 commit 57452fd
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 38 deletions.
20 changes: 14 additions & 6 deletions index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@ const util = require("util");
const os = require("os");
const emojiRegex = /[\uD800-\uDFFF]./;
const emojiList = require("emojis-list").filter(emoji => emojiRegex.test(emoji));
const matchAbsolutePath = /^\/|^[A-Z]:[/\\]|^\\\\/i; // node 0.10 does not support path.isAbsolute()
const matchAbsoluteWin32Path = /^[A-Z]:[/\\]|^\\\\/i;
const matchNativeWin32Path = /^[A-Z]:[/\\]|^\\\\/i;
const matchRelativePath = /^\.\.?[/\\]/;

const baseEncodeTables = {
Expand All @@ -27,6 +26,14 @@ const parseQueryDeprecationWarning = util.deprecate(() => {},
"parseQuery() will be replaced with getOptions() in the next major version of loader-utils."
);

function isAbsolutePath(str) {
return path.posix.isAbsolute(str) || path.win32.isAbsolute(str);
}

function isRelativePath(str) {
return matchRelativePath.test(str);
}

function encodeStringToEmoji(content, length) {
if(emojiCache[content]) return emojiCache[content];
length = length || 1;
Expand Down Expand Up @@ -135,15 +142,15 @@ function stringifyRequest(loaderContext, request) {
const splittedPart = part.match(/^(.*?)(\?.*)/);
let singlePath = splittedPart ? splittedPart[1] : part;
const query = splittedPart ? splittedPart[2] : "";
if(matchAbsolutePath.test(singlePath) && context) {
if(isAbsolutePath(singlePath) && context) {
singlePath = path.relative(context, singlePath);
if(matchAbsolutePath.test(singlePath)) {
if(isAbsolutePath(singlePath)) {
// If singlePath still matches an absolute path, singlePath was on a different drive than context.
// In this case, we leave the path platform-specific without replacing any separators.
// @see https://github.com/webpack/loader-utils/pull/14
return singlePath + query;
}
if(matchRelativePath.test(singlePath) === false) {
if(isRelativePath(singlePath) === false) {
// Ensure that the relative path starts at least with ./ otherwise it would be a request into the modules directory (like node_modules).
singlePath = "./" + singlePath;
}
Expand Down Expand Up @@ -185,7 +192,8 @@ function urlToRequest(url, root) {
const moduleRequestRegex = /^[^?]*~/;
let request;

if(matchAbsoluteWin32Path.test(url)) {
// we can't use path.win32.isAbsolute because it also matches paths starting with a forward slash
if(matchNativeWin32Path.test(url)) {
// absolute windows path, keep it
request = url;
} else if(root !== undefined && root !== false && /^\//.test(url)) {
Expand Down
73 changes: 41 additions & 32 deletions test/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -252,86 +252,95 @@ describe("loader-utils", () => {
posix: "path/to/thing",
win: "path\\to\\file"
});
const win = path.sep === "\\";
const posix = path.sep === "/";
[
{ request: "./a.js", expected: s("./a.js") },
{ request: ".\\a.js", expected: s("./a.js") },
{ request: "./a/b.js", expected: s("./a/b.js") },
{ request: ".\\a\\b.js", expected: s("./a/b.js") },
{ request: "module", expected: s("module") }, // without ./ is a request into the modules directory
{ request: "module/a.js", expected: s("module/a.js") },
{ request: "module\\a.js", expected: s("module/a.js") },
{ request: "./a.js" + paramQueryString, expected: s("./a.js" + paramQueryString) },
{ request: "./a.js" + jsonQueryString, expected: s("./a.js" + jsonQueryString) },
{ request: "module" + paramQueryString, expected: s("module" + paramQueryString) },
{ request: "module" + jsonQueryString, expected: s("module" + jsonQueryString) },
posix && { context: "/path/to", request: "/path/to/module/a.js", expected: s("./module/a.js") },
win && { context: "C:\\path\\to\\", request: "C:\\path\\to\\module\\a.js", expected: s("./module/a.js") },
posix && { context: "/path/to/thing", request: "/path/to/module/a.js", expected: s("../module/a.js") },
win && { context: "C:\\path\\to\\thing", request: "C:\\path\\to\\module\\a.js", expected: s("../module/a.js") },
win && { context: "\\\\A\\path\\to\\thing", request: "\\\\A\\path\\to\\module\\a.js", expected: s("../module/a.js") },
{ test: 1, request: "./a.js", expected: s("./a.js") },
{ test: 2, request: ".\\a.js", expected: s("./a.js") },
{ test: 3, request: "./a/b.js", expected: s("./a/b.js") },
{ test: 4, request: ".\\a\\b.js", expected: s("./a/b.js") },
{ test: 5, request: "module", expected: s("module") }, // without ./ is a request into the modules directory
{ test: 6, request: "module/a.js", expected: s("module/a.js") },
{ test: 7, request: "module\\a.js", expected: s("module/a.js") },
{ test: 8, request: "./a.js" + paramQueryString, expected: s("./a.js" + paramQueryString) },
{ test: 9, request: "./a.js" + jsonQueryString, expected: s("./a.js" + jsonQueryString) },
{ test: 10, request: "module" + paramQueryString, expected: s("module" + paramQueryString) },
{ test: 11, request: "module" + jsonQueryString, expected: s("module" + jsonQueryString) },
{ test: 12, os: "posix", context: "/path/to", request: "/path/to/module/a.js", expected: s("./module/a.js") },
{ test: 13, os: "win32", context: "C:\\path\\to\\", request: "C:\\path\\to\\module\\a.js", expected: s("./module/a.js") },
{ test: 14, os: "posix", context: "/path/to/thing", request: "/path/to/module/a.js", expected: s("../module/a.js") },
{ test: 15, os: "win32", context: "C:\\path\\to\\thing", request: "C:\\path\\to\\module\\a.js", expected: s("../module/a.js") },
{ test: 16, os: "win32", context: "\\\\A\\path\\to\\thing", request: "\\\\A\\path\\to\\module\\a.js", expected: s("../module/a.js") },
// If context and request are on different drives, the path should not be relative
// @see https://github.com/webpack/loader-utils/pull/14
win && { context: "D:\\path\\to\\thing", request: "C:\\path\\to\\module\\a.js", expected: s("C:\\path\\to\\module\\a.js") },
win && { context: "\\\\A\\path\\to\\thing", request: "\\\\B\\path\\to\\module\\a.js", expected: s("\\\\B\\path\\to\\module\\a.js") },
posix && {
{ test: 17, os: "win32", context: "D:\\path\\to\\thing", request: "C:\\path\\to\\module\\a.js", expected: s("C:\\path\\to\\module\\a.js") },
{ test: 18, os: "win32", context: "\\\\A\\path\\to\\thing", request: "\\\\B\\path\\to\\module\\a.js", expected: s("\\\\B\\path\\to\\module\\a.js") },
{
test: 19,
os: "posix",
context: "/path/to",
request: "/path/to/module/a.js" + paramQueryString,
expected: s("./module/a.js" + paramQueryString)
},
win && {
{
test: 20,
os: "win32",
context: "C:\\path\\to\\",
request: "C:\\path\\to\\module\\a.js" + paramQueryString,
expected: s("./module/a.js" + paramQueryString)
},
{
test: 21,
request:
["./a.js", "./b.js", "./c.js"].join("!"),
expected: s(
["./a.js", "./b.js", "./c.js"].join("!")
)
},
{
test: 22,
request:
["a/b.js", "c/d.js", "e/f.js", "g"].join("!"),
expected: s(
["a/b.js", "c/d.js", "e/f.js", "g"].join("!")
)
},
{
test: 23,
request:
["a/b.js" + paramQueryString, "c/d.js" + jsonQueryString, "e/f.js"].join("!"),
expected: s(
["a/b.js" + paramQueryString, "c/d.js" + jsonQueryString, "e/f.js"].join("!")
)
},
posix && {
{
test: 24,
os: "posix",
context: "/path/to",
request:
["/a/b.js" + paramQueryString, "c/d.js" + jsonQueryString, "/path/to/e/f.js"].join("!"),
expected: s(
["../../a/b.js" + paramQueryString, "c/d.js" + jsonQueryString, "./e/f.js"].join("!")
)
},
win && {
{
test: 25,
os: "win32",
context: "C:\\path\\to\\",
request:
["C:\\a\\b.js" + paramQueryString, "c\\d.js" + jsonQueryString, "C:\\path\\to\\e\\f.js"].join("!"),
expected: s(
["../../a/b.js" + paramQueryString, "c/d.js" + jsonQueryString, "./e/f.js"].join("!")
)
}
].forEach((testCase, i) => {
if(!testCase) {
// If testCase is not defined, this test case should not be executed on this OS.
// This is because node's path module is OS agnostic which means that path.relative won't produce
// a relative path with absolute windows paths on posix systems.
return;
}
it("should stringify request " + testCase.request + " to " + testCase.expected + " inside context " + testCase.context, () => {
].forEach(testCase => {
it(`${ testCase.test }. should stringify request ${ testCase.request } to ${ testCase.expected } inside context ${ testCase.context }`, () => {
const relative = path.relative;
if(testCase.os) {
// monkey patch path.relative in order to make this test work in every OS
path.relative = path[testCase.os].relative;
}
const actual = loaderUtils.stringifyRequest({ context: testCase.context }, testCase.request);
assert.equal(actual, testCase.expected);
path.relative = relative;
});
});
});
Expand Down

0 comments on commit 57452fd

Please sign in to comment.