Skip to content
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.

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

feat: add support for win32 paths #402

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 1 addition & 2 deletions lib/DescriptionFilePlugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,7 @@ module.exports = class DescriptionFilePlugin {
);
return callback();
}
const relativePath =
"." + path.slice(result.directory.length).replace(/\\/g, "/");
const relativePath = "." + path.slice(result.directory.length);
/** @type {ResolveRequest} */
const obj = {
...request,
Expand Down
11 changes: 5 additions & 6 deletions lib/DescriptionFileUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

"use strict";

const path = require("path");
const forEachBail = require("./forEachBail");

/** @typedef {import("./Resolver")} Resolver */
Expand Down Expand Up @@ -185,12 +186,10 @@ function getField(content, field) {
* @returns {string|null} parent directory or null
*/
function cdUp(directory) {
if (directory === "/") return null;
const i = directory.lastIndexOf("/"),
j = directory.lastIndexOf("\\");
const p = i < 0 ? j : j < 0 ? i : i < j ? j : i;
if (p < 0) return null;
return directory.slice(0, p || 1);
if (directory === path.sep) return null;
const i = directory.lastIndexOf(path.sep);
if (i < 0) return null;
return directory.slice(0, i || 1);
}

exports.loadDescriptionFile = loadDescriptionFile;
Expand Down
3 changes: 2 additions & 1 deletion lib/Resolver.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

"use strict";

const nodePath = require("path");
const { AsyncSeriesBailHook, AsyncSeriesHook, SyncHook } = require("tapable");
const createInnerContext = require("./createInnerContext");
const { parseIdentifier } = require("./util/identifier");
Expand Down Expand Up @@ -577,7 +578,7 @@ class Resolver {
* @returns {boolean} true, if the path is a directory path
*/
isDirectory(path) {
return path.endsWith("/");
return path.endsWith(nodePath.sep);
}

/**
Expand Down
6 changes: 5 additions & 1 deletion lib/ResolverFactory.js
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,11 @@ function createOptions(options) {
: ["node_modules"],
item => {
const type = getType(item);
return type === PathType.Normal || type === PathType.Relative;
return (
type === PathType.Normal ||
type === PathType.RelativePosix ||
type === PathType.RelativeWin
);
}
),
mainFields,
Expand Down
7 changes: 4 additions & 3 deletions lib/RestrictionsPlugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@

"use strict";

const path = require("path");

/** @typedef {import("./Resolver")} Resolver */
/** @typedef {import("./Resolver").ResolveStepHook} ResolveStepHook */

const slashCode = "/".charCodeAt(0);
const backslashCode = "\\".charCodeAt(0);
const separatorCode = path.sep.charCodeAt(0);

/**
* @param {string} path path
Expand All @@ -20,7 +21,7 @@ const isInside = (path, parent) => {
if (!path.startsWith(parent)) return false;
if (path.length === parent.length) return true;
const charCode = path.charCodeAt(parent.length);
return charCode === slashCode || charCode === backslashCode;
return charCode === separatorCode;
};

module.exports = class RestrictionsPlugin {
Expand Down
3 changes: 2 additions & 1 deletion lib/RootsPlugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

"use strict";

const path = require("path");
const forEachBail = require("./forEachBail");

/** @typedef {import("./Resolver")} Resolver */
Expand Down Expand Up @@ -35,7 +36,7 @@ class RootsPlugin {
.tapAsync("RootsPlugin", (request, resolveContext, callback) => {
const req = request.request;
if (!req) return callback();
if (!req.startsWith("/")) return callback();
if (!req.startsWith(path.sep)) return callback();

forEachBail(
this.roots,
Expand Down
5 changes: 3 additions & 2 deletions lib/SelfReferencePlugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,15 @@

"use strict";

const path = require("path");
const DescriptionFileUtils = require("./DescriptionFileUtils");

/** @typedef {import("./Resolver")} Resolver */
/** @typedef {import("./Resolver").JsonObject} JsonObject */
/** @typedef {import("./Resolver").ResolveRequest} ResolveRequest */
/** @typedef {import("./Resolver").ResolveStepHook} ResolveStepHook */

const slashCode = "/".charCodeAt(0);
const separatorCode = path.sep.charCodeAt(0);

module.exports = class SelfReferencePlugin {
/**
Expand Down Expand Up @@ -56,7 +57,7 @@
if (
req.startsWith(name) &&
(req.length === name.length ||
req.charCodeAt(name.length) === slashCode)
req.charCodeAt(name.length) === separatorCode)

Check warning on line 60 in lib/SelfReferencePlugin.js

View check run for this annotation

Codecov / codecov/patch

lib/SelfReferencePlugin.js#L60

Added line #L60 was not covered by tests
) {
const remainingRequest = `.${req.slice(name.length)}`;
/** @type {ResolveRequest} */
Expand Down
14 changes: 7 additions & 7 deletions lib/getPaths.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,14 @@

"use strict";

const nodePath = require("path");

/**
* @param {string} path path
* @returns {{paths: string[], segments: string[]}}} paths and segments
*/
module.exports = function getPaths(path) {
if (path === "/") return { paths: ["/"], segments: [""] };
if (path === nodePath.sep) return { paths: [nodePath.sep], segments: [""] };
const parts = path.split(/(.*?[\\/]+)/);
const paths = [path];
const segments = [parts[parts.length - 1]];
Expand All @@ -19,7 +21,7 @@ module.exports = function getPaths(path) {
for (let i = parts.length - 2; i > 2; i -= 2) {
paths.push(path);
part = parts[i];
path = path.substring(0, path.length - part.length) || "/";
path = path.substring(0, path.length - part.length) || nodePath.sep;
segments.push(part.slice(0, -1));
}
part = parts[1];
Expand All @@ -36,10 +38,8 @@ module.exports = function getPaths(path) {
* @returns {string|null} basename or null
*/
module.exports.basename = function basename(path) {
const i = path.lastIndexOf("/"),
j = path.lastIndexOf("\\");
const p = i < 0 ? j : j < 0 ? i : i < j ? j : i;
if (p < 0) return null;
const s = path.slice(p + 1);
const i = path.lastIndexOf(nodePath.sep);
if (i < 0) return null;
const s = path.slice(i + 1);
return s;
};
49 changes: 37 additions & 12 deletions lib/util/path.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@
const PathType = Object.freeze({
Empty: 0,
Normal: 1,
Relative: 2,
RelativeWin: 6,
RelativePosix: 2,
AbsoluteWin: 3,
AbsolutePosix: 4,
Internal: 5
Expand All @@ -45,7 +46,9 @@
const c0 = p.charCodeAt(0);
switch (c0) {
case CHAR_DOT:
return PathType.Relative;
return path.sep.charCodeAt(0) === CHAR_SLASH
? PathType.RelativePosix
: PathType.RelativeWin;

Check warning on line 51 in lib/util/path.js

View check run for this annotation

Codecov / codecov/patch

lib/util/path.js#L51

Added line #L51 was not covered by tests
case CHAR_SLASH:
return PathType.AbsolutePosix;
case CHAR_HASH:
Expand All @@ -60,8 +63,13 @@
const c1 = p.charCodeAt(1);
switch (c1) {
case CHAR_DOT:
return path.sep.charCodeAt(0) === CHAR_SLASH
? PathType.RelativePosix
: PathType.RelativeWin;

Check warning on line 68 in lib/util/path.js

View check run for this annotation

Codecov / codecov/patch

lib/util/path.js#L68

Added line #L68 was not covered by tests
case CHAR_SLASH:
return PathType.Relative;
return PathType.RelativePosix;
case CHAR_BACKSLASH:
return PathType.RelativeWin;

Check warning on line 72 in lib/util/path.js

View check run for this annotation

Codecov / codecov/patch

lib/util/path.js#L71-L72

Added lines #L71 - L72 were not covered by tests
}
return PathType.Normal;
}
Expand All @@ -88,10 +96,15 @@
const c1 = p.charCodeAt(1);
switch (c1) {
case CHAR_SLASH:
return PathType.Relative;
return PathType.RelativePosix;
case CHAR_BACKSLASH:
return PathType.RelativeWin;

Check warning on line 101 in lib/util/path.js

View check run for this annotation

Codecov / codecov/patch

lib/util/path.js#L100-L101

Added lines #L100 - L101 were not covered by tests
case CHAR_DOT: {
const c2 = p.charCodeAt(2);
if (c2 === CHAR_SLASH) return PathType.Relative;

if (c2 === CHAR_SLASH) return PathType.RelativePosix;
if (c2 === CHAR_BACKSLASH) return PathType.RelativeWin;

return PathType.Normal;
}
}
Expand Down Expand Up @@ -127,12 +140,16 @@
return p;
case PathType.AbsoluteWin:
return winNormalize(p);
case PathType.Relative: {
case PathType.RelativePosix: {
const r = posixNormalize(p);
return getType(r) === PathType.Relative ? r : `./${r}`;
return getType(r) === PathType.RelativePosix ? r : `./${r}`;
}
case PathType.RelativeWin: {
const r = winNormalize(p);

Check warning on line 148 in lib/util/path.js

View check run for this annotation

Codecov / codecov/patch

lib/util/path.js#L147-L148

Added lines #L147 - L148 were not covered by tests
return getType(r) === PathType.RelativeWin ? r : `.\\${r}`;
}
}
return posixNormalize(p);
return path.normalize(p);
};
exports.normalize = normalize;

Expand All @@ -152,21 +169,29 @@
}
switch (getType(rootPath)) {
case PathType.Normal:
case PathType.Relative:
return path.sep.charCodeAt(0) === CHAR_SLASH
? posixNormalize(`${rootPath}/${request}`)
: winNormalize(`${rootPath}\\${request}`);

Check warning on line 174 in lib/util/path.js

View check run for this annotation

Codecov / codecov/patch

lib/util/path.js#L173-L174

Added lines #L173 - L174 were not covered by tests
case PathType.RelativePosix:
case PathType.AbsolutePosix:
return posixNormalize(`${rootPath}/${request}`);
case PathType.RelativeWin:

Check warning on line 178 in lib/util/path.js

View check run for this annotation

Codecov / codecov/patch

lib/util/path.js#L178

Added line #L178 was not covered by tests
case PathType.AbsoluteWin:
return winNormalize(`${rootPath}\\${request}`);
}
switch (requestType) {
case PathType.Empty:
return rootPath;
case PathType.Relative: {
case PathType.RelativePosix: {
const r = posixNormalize(rootPath);

Check warning on line 186 in lib/util/path.js

View check run for this annotation

Codecov / codecov/patch

lib/util/path.js#L185-L186

Added lines #L185 - L186 were not covered by tests
return getType(r) === PathType.RelativePosix ? r : `./${r}`;
}
case PathType.RelativeWin: {

Check warning on line 189 in lib/util/path.js

View check run for this annotation

Codecov / codecov/patch

lib/util/path.js#L189

Added line #L189 was not covered by tests
const r = posixNormalize(rootPath);
return getType(r) === PathType.Relative ? r : `./${r}`;
return getType(r) === PathType.RelativeWin ? r : `.\\${r}`;
}
}
return posixNormalize(rootPath);
return path.normalize(rootPath);

Check warning on line 194 in lib/util/path.js

View check run for this annotation

Codecov / codecov/patch

lib/util/path.js#L194

Added line #L194 was not covered by tests
};
exports.join = join;

Expand Down
67 changes: 66 additions & 1 deletion test/path.test.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,15 @@
const { checkImportsExportsFieldTarget } = require("../lib/util/path");
const path = require("path");
const {
checkImportsExportsFieldTarget,
PathType,
getType,
normalize
} = require("../lib/util/path");

// It's enough to use path.sep for tests because the repository has Windows test-runners,
// but for understanding what to expect, we should know about platform and path-type in tests.
const isWin32 = process.platform === "win32";
const currentPathType = isWin32 ? "win32" : "posix";

describe("checkImportsExportsFieldTarget", () => {
/**
Expand Down Expand Up @@ -27,3 +38,57 @@ describe("checkImportsExportsFieldTarget", () => {
});
});
});

describe("getPath", () => {
const relativePathType = isWin32 ? "RelativeWin" : "RelativePosix";

it(`should resolve PathType.${relativePathType} for paths if path.sep is ${currentPathType} (${path.sep})`, () => {
expect(getType(".")).toBe(PathType[relativePathType]);
expect(getType("..")).toBe(PathType[relativePathType]);
expect(getType(`..${path.sep}`)).toBe(PathType[relativePathType]);
expect(getType(`..${path.sep}test${path.sep}index.js`)).toBe(
PathType[relativePathType]
);
});
});

describe("normalize", () => {
it(`should correctly normalize for empty path if path.sep is ${currentPathType} (${path.sep})`, () => {
const pathToNormalize = "";

expect(getType(pathToNormalize)).toBe(PathType.Empty);
expect(normalize(pathToNormalize)).toBe("");
});

it(`should correctly normalize for relative path if path.sep is ${currentPathType} (${path.sep})`, () => {
const pathToNormalize = `..${path.sep}hello${path.sep}world${path.sep}..${path.sep}test.js`;

expect(getType(pathToNormalize)).toBe(
isWin32 ? PathType.RelativeWin : PathType.RelativePosix
);
expect(normalize(pathToNormalize)).toBe(
`..${path.sep}hello${path.sep}test.js`
);
});

it(`should correctly normalize for absolute path if path.sep is ${currentPathType} (${path.sep})`, () => {
const basePath = `${path.sep}hello${path.sep}world${path.sep}..${path.sep}test.js`;
const getAbsolutePathPrefixBasedOnPlatform = pathStr =>
isWin32 ? `X:${pathStr}` : pathStr;
const pathToNormalize = getAbsolutePathPrefixBasedOnPlatform(basePath);

expect(getType(pathToNormalize)).toBe(
isWin32 ? PathType.AbsoluteWin : PathType.AbsolutePosix
);
expect(normalize(pathToNormalize)).toBe(
getAbsolutePathPrefixBasedOnPlatform(`${path.sep}hello${path.sep}test.js`)
);
});

it("should correctly normalize for PathType.Normal", () => {
const pathToNormalize = "enhancedResolve/lib/util/../index";

expect(getType(pathToNormalize)).toBe(PathType.Normal);
expect(normalize(pathToNormalize)).toBe("enhancedResolve/lib/index");
});
});