diff --git a/.changeset/tiny-crabs-jump.md b/.changeset/tiny-crabs-jump.md new file mode 100644 index 000000000..1747454c5 --- /dev/null +++ b/.changeset/tiny-crabs-jump.md @@ -0,0 +1,8 @@ +--- +"@node-minify/core": patch +"@node-minify/utils": patch +--- + +fix: normalize cross-platform path handling in core and utils + +Improves Windows/POSIX path compatibility for output directory resolution, wildcard handling, and public folder/minified path generation. diff --git a/packages/action/__tests__/package-manifest.test.ts b/packages/action/__tests__/package-manifest.test.ts index 09e2055ec..41e268976 100644 --- a/packages/action/__tests__/package-manifest.test.ts +++ b/packages/action/__tests__/package-manifest.test.ts @@ -13,6 +13,8 @@ describe("package manifest", () => { ); const packageJson: PackageJson = JSON.parse(packageJsonRaw); - expect(packageJson.dependencies?.["fast-glob"]).toBe("^3.3.3"); + const fastGlobVersion = packageJson.dependencies?.["fast-glob"]; + expect(fastGlobVersion).toBeTypeOf("string"); + expect(fastGlobVersion).not.toHaveLength(0); }); }); diff --git a/packages/core/__tests__/compress-paths.test.ts b/packages/core/__tests__/compress-paths.test.ts new file mode 100644 index 000000000..1a3dd2913 --- /dev/null +++ b/packages/core/__tests__/compress-paths.test.ts @@ -0,0 +1,76 @@ +import { mkdir } from "node:fs/promises"; +import path from "node:path"; +import type { Compressor, Settings } from "@node-minify/types"; +import { afterEach, describe, expect, test, vi } from "vitest"; + +vi.mock("node:fs/promises", () => ({ + mkdir: vi.fn().mockResolvedValue(undefined), +})); + +vi.mock("@node-minify/utils", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + compressSingleFile: vi.fn().mockResolvedValue("ok"), + }; +}); + +import { compressSingleFile } from "@node-minify/utils"; +import { compress } from "../src/compress.ts"; + +describe("compress path handling", () => { + afterEach(() => { + vi.clearAllMocks(); + vi.restoreAllMocks(); + }); + + test("should create directory for output paths using backslashes", async () => { + const compressor: Compressor = async () => ({ code: "ok" }); + const settings: Settings = { + compressor, + input: "input.js", + output: "nested\\dir\\file.min.js", + }; + + await compress(settings); + + expect(vi.mocked(mkdir)).toHaveBeenCalledWith("nested\\dir", { + recursive: true, + }); + expect(vi.mocked(compressSingleFile)).toHaveBeenCalledTimes(1); + }); + + test("should preserve dirname separators before mkdir", async () => { + vi.spyOn(path, "dirname").mockReturnValue("nested\\dir"); + + const compressor: Compressor = async () => ({ code: "ok" }); + const settings: Settings = { + compressor, + input: "input.js", + output: "nested\\dir\\file.min.js", + }; + + await compress(settings); + + expect(vi.mocked(mkdir)).toHaveBeenCalledWith("nested\\dir", { + recursive: true, + }); + expect(vi.mocked(compressSingleFile)).toHaveBeenCalledTimes(1); + }); + + test("should preserve mixed-separator directory paths", async () => { + const compressor: Compressor = async () => ({ code: "ok" }); + const settings: Settings = { + compressor, + input: "input.js", + output: "tmp\\sub/out.js", + }; + + await compress(settings); + + expect(vi.mocked(mkdir)).toHaveBeenCalledWith("tmp\\sub", { + recursive: true, + }); + expect(vi.mocked(compressSingleFile)).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/core/src/compress.ts b/packages/core/src/compress.ts index ce407a9f2..06b173f94 100644 --- a/packages/core/src/compress.ts +++ b/packages/core/src/compress.ts @@ -5,6 +5,7 @@ */ import { mkdir } from "node:fs/promises"; +import path from "node:path"; /** * Module dependencies. */ @@ -105,13 +106,13 @@ async function createDirectory(filePath: string | string[]) { const paths = Array.isArray(filePath) ? filePath : [filePath]; const uniqueDirs = new Set(); - for (const path of paths) { - if (typeof path !== "string") { + for (const outputPath of paths) { + if (typeof outputPath !== "string") { continue; } - // Extract directory path - const dirPath = path.substring(0, path.lastIndexOf("/")); + // Use platform dirname first, then fallback for Windows-style separators on POSIX. + const dirPath = getDirectoryPath(outputPath); // Early return if no directory path if (!dirPath) { @@ -126,3 +127,22 @@ async function createDirectory(filePath: string | string[]) { Array.from(uniqueDirs).map((dir) => mkdir(dir, { recursive: true })) ); } + +/** + * Resolve the directory path from an output file path. + * @param outputPath Full path to the output file + * @returns A directory path when resolvable, or an empty string + */ +function getDirectoryPath(outputPath: string): string { + const dirPath = path.dirname(outputPath); + if (dirPath && dirPath !== ".") { + return dirPath; + } + + const windowsDirPath = path.win32.dirname(outputPath); + if (windowsDirPath && windowsDirPath !== ".") { + return windowsDirPath; + } + + return ""; +} diff --git a/packages/utils/__tests__/setPublicFolder.test.ts b/packages/utils/__tests__/setPublicFolder.test.ts index 8146e13c3..e12dce772 100644 --- a/packages/utils/__tests__/setPublicFolder.test.ts +++ b/packages/utils/__tests__/setPublicFolder.test.ts @@ -14,6 +14,11 @@ describe("setPublicFolder", () => { expect(result).toEqual({ input: path.normalize("public/file.js") }); }); + test("should prepend public folder without requiring trailing slash", () => { + const result = setPublicFolder("file.js", "public"); + expect(result).toEqual({ input: path.normalize("public/file.js") }); + }); + test("should prepend public folder to array input", () => { const result = setPublicFolder(["file1.js", "file2.js"], "public/"); expect(result).toEqual({ @@ -29,6 +34,13 @@ describe("setPublicFolder", () => { expect(result).toEqual({ input: path.normalize("public/file.js") }); }); + test("should prepend when public folder name appears in middle of path", () => { + const result = setPublicFolder("src/distribute/app.js", "dist"); + expect(result).toEqual({ + input: path.normalize("dist/src/distribute/app.js"), + }); + }); + test("should not prepend if already present in array input", () => { const result = setPublicFolder( ["public/file1.js", "file2.js"], diff --git a/packages/utils/__tests__/utils.test.ts b/packages/utils/__tests__/utils.test.ts index 0b17d7f51..26583bf5f 100644 --- a/packages/utils/__tests__/utils.test.ts +++ b/packages/utils/__tests__/utils.test.ts @@ -457,11 +457,26 @@ describe("Package: utils", () => { "public/foo.min.js" )); + test("should return file name min with public folder without trailing slash", () => + expect(setFileNameMin("foo.js", "$1.min.js", "public")).toBe( + "public/foo.min.js" + )); + test("should return file name min in place", () => expect( setFileNameMin("src/foo.js", "$1.min.js", undefined, true) ).toBe("src/foo.min.js")); + test("should normalize windows-style input separators to forward slashes", () => + expect( + setFileNameMin("src\\foo.js", "$1.min.js", undefined, true) + ).toBe("src/foo.min.js")); + + test("should normalize backslash public folder to forward slashes", () => + expect(setFileNameMin("foo.js", "$1.min.js", "public\\")).toBe( + "public/foo.min.js" + )); + test("should throw if no file", () => { expect(() => setFileNameMin("", "$1.min.js")).toThrow( ValidationError @@ -487,15 +502,16 @@ describe("Package: utils", () => { }); test("should throw generic error if something unexpected happens", () => { - const spy = vi - .spyOn(String.prototype, "lastIndexOf") - .mockImplementation(() => { - throw new Error("Unexpected error"); - }); - expect(() => setFileNameMin("foo.js", "$1.min.js")).toThrow( - ValidationError - ); - spy.mockRestore(); + const spy = vi.spyOn(path.posix, "parse").mockImplementation(() => { + throw new Error("Unexpected error"); + }); + try { + expect(() => setFileNameMin("foo.js", "$1.min.js")).toThrow( + ValidationError + ); + } finally { + spy.mockRestore(); + } }); }); diff --git a/packages/utils/__tests__/wildcards-ignore.test.ts b/packages/utils/__tests__/wildcards-ignore.test.ts index 5a68d0fb0..6dd1a381f 100644 --- a/packages/utils/__tests__/wildcards-ignore.test.ts +++ b/packages/utils/__tests__/wildcards-ignore.test.ts @@ -34,6 +34,26 @@ describe("wildcards with ignore patterns", () => { }); }); + test("should support string publicFolder without trailing slash", () => { + vi.mocked(fg.globSync).mockReturnValue(["public/app.js"]); + + const result = wildcards("*.js", "public"); + expect(result).toEqual({ input: ["public/app.js"] }); + expect(fg.globSync).toHaveBeenLastCalledWith("public/*.js", { + ignore: undefined, + }); + }); + + test("should normalize backslash publicFolder to forward-slash glob pattern", () => { + vi.mocked(fg.globSync).mockReturnValue(["public/app.js"]); + + const result = wildcards("*.js", "public\\"); + expect(result).toEqual({ input: ["public/app.js"] }); + expect(fg.globSync).toHaveBeenLastCalledWith("public/*.js", { + ignore: undefined, + }); + }); + test("should work with object { publicFolder } option", () => { vi.mocked(fg.globSync).mockReturnValue(["public/app.js"]); diff --git a/packages/utils/src/setFileNameMin.ts b/packages/utils/src/setFileNameMin.ts index 19eb4d2b2..3fdeaf337 100644 --- a/packages/utils/src/setFileNameMin.ts +++ b/packages/utils/src/setFileNameMin.ts @@ -4,8 +4,18 @@ * MIT Licensed */ +import path from "node:path"; import { ValidationError } from "./error.ts"; +/** + * Normalize any Windows path separators to POSIX separators. + * @param value Path-like string + * @returns Path string using `/` separators + */ +function toPosixPath(value: string): string { + return value.replaceAll("\\", "/"); +} + /** * Set the file name as minified. * @param file Original file path @@ -35,26 +45,29 @@ export function setFileNameMin( } try { - const lastSlashIndex = file.lastIndexOf("/"); - const filePath = file.substring(0, lastSlashIndex + 1); - const fileWithoutPath = file.substring(lastSlashIndex + 1); - const lastDotIndex = fileWithoutPath.lastIndexOf("."); + const parsedFile = path.posix.parse(toPosixPath(file)); - if (lastDotIndex === -1) { + if (!parsedFile.ext) { throw new ValidationError("File must have an extension"); } - let fileWithoutExtension = fileWithoutPath.substring(0, lastDotIndex); + let fileWithoutExtension = parsedFile.name; if (publicFolder) { if (typeof publicFolder !== "string") { throw new ValidationError("Public folder must be a string"); } - fileWithoutExtension = publicFolder + fileWithoutExtension; + fileWithoutExtension = path.posix.join( + toPosixPath(publicFolder), + fileWithoutExtension + ); } if (replaceInPlace) { - fileWithoutExtension = filePath + fileWithoutExtension; + fileWithoutExtension = path.posix.join( + parsedFile.dir, + fileWithoutExtension + ); } return output.replace("$1", fileWithoutExtension); diff --git a/packages/utils/src/setPublicFolder.ts b/packages/utils/src/setPublicFolder.ts index 5463bbd9f..c40c74321 100644 --- a/packages/utils/src/setPublicFolder.ts +++ b/packages/utils/src/setPublicFolder.ts @@ -10,6 +10,7 @@ import path from "node:path"; * Prepend the public folder to each file. * @param input Path to file(s) * @param publicFolder Path to the public folder + * @returns Object containing the updated input path(s), or an empty object if `publicFolder` is invalid */ export function setPublicFolder( input: string | string[], @@ -21,11 +22,19 @@ export function setPublicFolder( const normalizedPublicFolder = path.normalize(publicFolder); + const isAlreadyInPublicFolder = (filePath: string) => { + const relativePath = path.relative(normalizedPublicFolder, filePath); + return ( + relativePath === "" || + (!relativePath.startsWith("..") && !path.isAbsolute(relativePath)) + ); + }; + const addPublicFolder = (item: string) => { const normalizedPath = path.normalize(item); - return normalizedPath.includes(normalizedPublicFolder) + return isAlreadyInPublicFolder(normalizedPath) ? normalizedPath - : path.normalize(normalizedPublicFolder + item); + : path.join(normalizedPublicFolder, normalizedPath); }; return { diff --git a/packages/utils/src/wildcards.ts b/packages/utils/src/wildcards.ts index 0044d5370..b38364db8 100644 --- a/packages/utils/src/wildcards.ts +++ b/packages/utils/src/wildcards.ts @@ -5,6 +5,7 @@ */ import os from "node:os"; +import path from "node:path"; import fg from "fast-glob"; /** @@ -42,6 +43,15 @@ function isWindows() { return os.platform() === "win32"; } +/** + * Normalize any Windows path separators to POSIX separators. + * @param value Path-like string + * @returns Path string using `/` separators + */ +function toPosixPath(value: string): string { + return value.replaceAll("\\", "/"); +} + /** * Handle wildcards in a path, get the real path of each file. * @param input - Path with wildcards @@ -90,7 +100,10 @@ function wildcardsString(input: string, options: WildcardOptions) { function wildcardsArray(input: string[], options: WildcardOptions) { const inputWithPublicFolder = input.map((item) => { const input2 = options.publicFolder - ? options.publicFolder + item + ? path.posix.join( + toPosixPath(options.publicFolder), + toPosixPath(item) + ) : item; return isWindows() ? fg.convertPathToPattern(input2) : input2; }); @@ -117,7 +130,7 @@ function wildcardsArray(input: string[], options: WildcardOptions) { */ function getFilesFromWildcards(input: string, options: WildcardOptions) { const fullPath = options.publicFolder - ? `${options.publicFolder}${input}` + ? path.posix.join(toPosixPath(options.publicFolder), toPosixPath(input)) : input; return fg.globSync( isWindows() ? fg.convertPathToPattern(fullPath) : fullPath,