Skip to content

Commit 67b8008

Browse files
committed
fix(utils,core,action): normalize path handling across platforms
Normalize path handling in core and utils to consistently support Windows and POSIX separators, and adjust action/test assertions accordingly. Also include reviewer-driven test updates for compress directory creation and wildcard/public folder edge cases to stabilize cross-platform CI behavior.
1 parent ee4b1c6 commit 67b8008

9 files changed

Lines changed: 191 additions & 26 deletions

File tree

packages/action/__tests__/package-manifest.test.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ describe("package manifest", () => {
1313
);
1414
const packageJson: PackageJson = JSON.parse(packageJsonRaw);
1515

16-
expect(packageJson.dependencies?.["fast-glob"]).toBe("^3.3.3");
16+
const fastGlobVersion = packageJson.dependencies?.["fast-glob"];
17+
expect(fastGlobVersion).toBeTypeOf("string");
18+
expect(fastGlobVersion).not.toHaveLength(0);
1719
});
1820
});
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
import { mkdir } from "node:fs/promises";
2+
import path from "node:path";
3+
import type { Compressor, Settings } from "@node-minify/types";
4+
import { afterEach, describe, expect, test, vi } from "vitest";
5+
6+
vi.mock("node:fs/promises", () => ({
7+
mkdir: vi.fn().mockResolvedValue(undefined),
8+
}));
9+
10+
vi.mock("@node-minify/utils", async (importOriginal) => {
11+
const actual = await importOriginal<typeof import("@node-minify/utils")>();
12+
return {
13+
...actual,
14+
compressSingleFile: vi.fn().mockResolvedValue("ok"),
15+
};
16+
});
17+
18+
import { compressSingleFile } from "@node-minify/utils";
19+
import { compress } from "../src/compress.ts";
20+
21+
describe("compress path handling", () => {
22+
afterEach(() => {
23+
vi.clearAllMocks();
24+
vi.restoreAllMocks();
25+
});
26+
27+
test("should create directory for output paths using backslashes", async () => {
28+
const compressor: Compressor = async () => ({ code: "ok" });
29+
const settings: Settings = {
30+
compressor,
31+
input: "input.js",
32+
output: "nested\\dir\\file.min.js",
33+
};
34+
35+
await compress(settings);
36+
37+
expect(vi.mocked(mkdir)).toHaveBeenCalledWith("nested/dir", {
38+
recursive: true,
39+
});
40+
expect(vi.mocked(compressSingleFile)).toHaveBeenCalledTimes(1);
41+
});
42+
43+
test("should normalize dirname output before mkdir", async () => {
44+
vi.spyOn(path, "dirname").mockReturnValue("nested\\dir");
45+
46+
const compressor: Compressor = async () => ({ code: "ok" });
47+
const settings: Settings = {
48+
compressor,
49+
input: "input.js",
50+
output: "nested\\dir\\file.min.js",
51+
};
52+
53+
await compress(settings);
54+
55+
expect(vi.mocked(mkdir)).toHaveBeenCalledWith("nested/dir", {
56+
recursive: true,
57+
});
58+
expect(vi.mocked(compressSingleFile)).toHaveBeenCalledTimes(1);
59+
});
60+
});

packages/core/src/compress.ts

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
*/
66

77
import { mkdir } from "node:fs/promises";
8+
import path from "node:path";
89
/**
910
* Module dependencies.
1011
*/
@@ -105,13 +106,13 @@ async function createDirectory(filePath: string | string[]) {
105106
const paths = Array.isArray(filePath) ? filePath : [filePath];
106107
const uniqueDirs = new Set<string>();
107108

108-
for (const path of paths) {
109-
if (typeof path !== "string") {
109+
for (const outputPath of paths) {
110+
if (typeof outputPath !== "string") {
110111
continue;
111112
}
112113

113-
// Extract directory path
114-
const dirPath = path.substring(0, path.lastIndexOf("/"));
114+
// Use platform dirname first, then fallback for Windows-style separators on POSIX.
115+
const dirPath = getDirectoryPath(outputPath);
115116

116117
// Early return if no directory path
117118
if (!dirPath) {
@@ -126,3 +127,22 @@ async function createDirectory(filePath: string | string[]) {
126127
Array.from(uniqueDirs).map((dir) => mkdir(dir, { recursive: true }))
127128
);
128129
}
130+
131+
/**
132+
* Resolve the directory path from an output file path.
133+
* @param outputPath Full path to the output file
134+
* @returns A directory path when resolvable, or an empty string
135+
*/
136+
function getDirectoryPath(outputPath: string): string {
137+
const dirPath = path.dirname(outputPath);
138+
if (dirPath && dirPath !== ".") {
139+
return dirPath.replaceAll("\\", "/");
140+
}
141+
142+
const windowsDirPath = path.win32.dirname(outputPath);
143+
if (windowsDirPath && windowsDirPath !== ".") {
144+
return windowsDirPath.replaceAll("\\", "/");
145+
}
146+
147+
return "";
148+
}

packages/utils/__tests__/setPublicFolder.test.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,11 @@ describe("setPublicFolder", () => {
1414
expect(result).toEqual({ input: path.normalize("public/file.js") });
1515
});
1616

17+
test("should prepend public folder without requiring trailing slash", () => {
18+
const result = setPublicFolder("file.js", "public");
19+
expect(result).toEqual({ input: path.normalize("public/file.js") });
20+
});
21+
1722
test("should prepend public folder to array input", () => {
1823
const result = setPublicFolder(["file1.js", "file2.js"], "public/");
1924
expect(result).toEqual({
@@ -29,6 +34,13 @@ describe("setPublicFolder", () => {
2934
expect(result).toEqual({ input: path.normalize("public/file.js") });
3035
});
3136

37+
test("should prepend when public folder name appears in middle of path", () => {
38+
const result = setPublicFolder("src/distribute/app.js", "dist");
39+
expect(result).toEqual({
40+
input: path.normalize("dist/src/distribute/app.js"),
41+
});
42+
});
43+
3244
test("should not prepend if already present in array input", () => {
3345
const result = setPublicFolder(
3446
["public/file1.js", "file2.js"],

packages/utils/__tests__/utils.test.ts

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -457,11 +457,26 @@ describe("Package: utils", () => {
457457
"public/foo.min.js"
458458
));
459459

460+
test("should return file name min with public folder without trailing slash", () =>
461+
expect(setFileNameMin("foo.js", "$1.min.js", "public")).toBe(
462+
"public/foo.min.js"
463+
));
464+
460465
test("should return file name min in place", () =>
461466
expect(
462467
setFileNameMin("src/foo.js", "$1.min.js", undefined, true)
463468
).toBe("src/foo.min.js"));
464469

470+
test("should normalize windows-style input separators to forward slashes", () =>
471+
expect(
472+
setFileNameMin("src\\foo.js", "$1.min.js", undefined, true)
473+
).toBe("src/foo.min.js"));
474+
475+
test("should normalize backslash public folder to forward slashes", () =>
476+
expect(setFileNameMin("foo.js", "$1.min.js", "public\\")).toBe(
477+
"public/foo.min.js"
478+
));
479+
465480
test("should throw if no file", () => {
466481
expect(() => setFileNameMin("", "$1.min.js")).toThrow(
467482
ValidationError
@@ -487,15 +502,16 @@ describe("Package: utils", () => {
487502
});
488503

489504
test("should throw generic error if something unexpected happens", () => {
490-
const spy = vi
491-
.spyOn(String.prototype, "lastIndexOf")
492-
.mockImplementation(() => {
493-
throw new Error("Unexpected error");
494-
});
495-
expect(() => setFileNameMin("foo.js", "$1.min.js")).toThrow(
496-
ValidationError
497-
);
498-
spy.mockRestore();
505+
const spy = vi.spyOn(path.posix, "parse").mockImplementation(() => {
506+
throw new Error("Unexpected error");
507+
});
508+
try {
509+
expect(() => setFileNameMin("foo.js", "$1.min.js")).toThrow(
510+
ValidationError
511+
);
512+
} finally {
513+
spy.mockRestore();
514+
}
499515
});
500516
});
501517

packages/utils/__tests__/wildcards-ignore.test.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,26 @@ describe("wildcards with ignore patterns", () => {
3434
});
3535
});
3636

37+
test("should support string publicFolder without trailing slash", () => {
38+
vi.mocked(fg.globSync).mockReturnValue(["public/app.js"]);
39+
40+
const result = wildcards("*.js", "public");
41+
expect(result).toEqual({ input: ["public/app.js"] });
42+
expect(fg.globSync).toHaveBeenLastCalledWith("public/*.js", {
43+
ignore: undefined,
44+
});
45+
});
46+
47+
test("should normalize backslash publicFolder to forward-slash glob pattern", () => {
48+
vi.mocked(fg.globSync).mockReturnValue(["public/app.js"]);
49+
50+
const result = wildcards("*.js", "public\\");
51+
expect(result).toEqual({ input: ["public/app.js"] });
52+
expect(fg.globSync).toHaveBeenLastCalledWith("public/*.js", {
53+
ignore: undefined,
54+
});
55+
});
56+
3757
test("should work with object { publicFolder } option", () => {
3858
vi.mocked(fg.globSync).mockReturnValue(["public/app.js"]);
3959

packages/utils/src/setFileNameMin.ts

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,18 @@
44
* MIT Licensed
55
*/
66

7+
import path from "node:path";
78
import { ValidationError } from "./error.ts";
89

10+
/**
11+
* Normalize any Windows path separators to POSIX separators.
12+
* @param value Path-like string
13+
* @returns Path string using `/` separators
14+
*/
15+
function toPosixPath(value: string): string {
16+
return value.replaceAll("\\", "/");
17+
}
18+
919
/**
1020
* Set the file name as minified.
1121
* @param file Original file path
@@ -35,26 +45,29 @@ export function setFileNameMin(
3545
}
3646

3747
try {
38-
const lastSlashIndex = file.lastIndexOf("/");
39-
const filePath = file.substring(0, lastSlashIndex + 1);
40-
const fileWithoutPath = file.substring(lastSlashIndex + 1);
41-
const lastDotIndex = fileWithoutPath.lastIndexOf(".");
48+
const parsedFile = path.posix.parse(toPosixPath(file));
4249

43-
if (lastDotIndex === -1) {
50+
if (!parsedFile.ext) {
4451
throw new ValidationError("File must have an extension");
4552
}
4653

47-
let fileWithoutExtension = fileWithoutPath.substring(0, lastDotIndex);
54+
let fileWithoutExtension = parsedFile.name;
4855

4956
if (publicFolder) {
5057
if (typeof publicFolder !== "string") {
5158
throw new ValidationError("Public folder must be a string");
5259
}
53-
fileWithoutExtension = publicFolder + fileWithoutExtension;
60+
fileWithoutExtension = path.posix.join(
61+
toPosixPath(publicFolder),
62+
fileWithoutExtension
63+
);
5464
}
5565

5666
if (replaceInPlace) {
57-
fileWithoutExtension = filePath + fileWithoutExtension;
67+
fileWithoutExtension = path.posix.join(
68+
parsedFile.dir,
69+
fileWithoutExtension
70+
);
5871
}
5972

6073
return output.replace("$1", fileWithoutExtension);

packages/utils/src/setPublicFolder.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import path from "node:path";
1010
* Prepend the public folder to each file.
1111
* @param input Path to file(s)
1212
* @param publicFolder Path to the public folder
13+
* @returns Object containing the updated input path(s), or an empty object if `publicFolder` is invalid
1314
*/
1415
export function setPublicFolder(
1516
input: string | string[],
@@ -21,11 +22,19 @@ export function setPublicFolder(
2122

2223
const normalizedPublicFolder = path.normalize(publicFolder);
2324

25+
const isAlreadyInPublicFolder = (filePath: string) => {
26+
const relativePath = path.relative(normalizedPublicFolder, filePath);
27+
return (
28+
relativePath === "" ||
29+
(!relativePath.startsWith("..") && !path.isAbsolute(relativePath))
30+
);
31+
};
32+
2433
const addPublicFolder = (item: string) => {
2534
const normalizedPath = path.normalize(item);
26-
return normalizedPath.includes(normalizedPublicFolder)
35+
return isAlreadyInPublicFolder(normalizedPath)
2736
? normalizedPath
28-
: path.normalize(normalizedPublicFolder + item);
37+
: path.join(normalizedPublicFolder, normalizedPath);
2938
};
3039

3140
return {

packages/utils/src/wildcards.ts

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
*/
66

77
import os from "node:os";
8+
import path from "node:path";
89
import fg from "fast-glob";
910

1011
/**
@@ -42,6 +43,15 @@ function isWindows() {
4243
return os.platform() === "win32";
4344
}
4445

46+
/**
47+
* Normalize any Windows path separators to POSIX separators.
48+
* @param value Path-like string
49+
* @returns Path string using `/` separators
50+
*/
51+
function toPosixPath(value: string): string {
52+
return value.replaceAll("\\", "/");
53+
}
54+
4555
/**
4656
* Handle wildcards in a path, get the real path of each file.
4757
* @param input - Path with wildcards
@@ -90,7 +100,10 @@ function wildcardsString(input: string, options: WildcardOptions) {
90100
function wildcardsArray(input: string[], options: WildcardOptions) {
91101
const inputWithPublicFolder = input.map((item) => {
92102
const input2 = options.publicFolder
93-
? options.publicFolder + item
103+
? path.posix.join(
104+
toPosixPath(options.publicFolder),
105+
toPosixPath(item)
106+
)
94107
: item;
95108
return isWindows() ? fg.convertPathToPattern(input2) : input2;
96109
});
@@ -117,7 +130,7 @@ function wildcardsArray(input: string[], options: WildcardOptions) {
117130
*/
118131
function getFilesFromWildcards(input: string, options: WildcardOptions) {
119132
const fullPath = options.publicFolder
120-
? `${options.publicFolder}${input}`
133+
? path.posix.join(toPosixPath(options.publicFolder), toPosixPath(input))
121134
: input;
122135
return fg.globSync(
123136
isWindows() ? fg.convertPathToPattern(fullPath) : fullPath,

0 commit comments

Comments
 (0)