diff --git a/.changeset/fresh-tomatoes-crash.md b/.changeset/fresh-tomatoes-crash.md new file mode 100644 index 000000000..18694f8c2 --- /dev/null +++ b/.changeset/fresh-tomatoes-crash.md @@ -0,0 +1,8 @@ +--- +"@node-minify/cli": patch +"@node-minify/core": patch +"@node-minify/run": patch +"@node-minify/utils": patch +--- + +Fix empty in-memory content handling, benchmark CLI defaults, subprocess close handling, and multi-output routing regressions. diff --git a/packages/cli/__tests__/cli-benchmark-bin.test.ts b/packages/cli/__tests__/cli-benchmark-bin.test.ts new file mode 100644 index 000000000..ef46063de --- /dev/null +++ b/packages/cli/__tests__/cli-benchmark-bin.test.ts @@ -0,0 +1,49 @@ +/*! + * node-minify + * Copyright (c) 2011-2026 Rodolphe Stoclin + * MIT Licensed + */ + +import { execFile } from "node:child_process"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; +import { promisify } from "node:util"; +import { describe, expect, test } from "vitest"; + +const execFileAsync = promisify(execFile); +const __dirname = path.dirname(fileURLToPath(import.meta.url)); +const repoRoot = path.resolve(__dirname, "../../.."); +const cliEntry = path.join(repoRoot, "packages/cli/src/bin/cli.ts"); + +describe("cli benchmark command", () => { + test("should emit clean JSON and benchmark defaults when compressors are not provided", async () => { + const { stdout, stderr } = await execFileAsync( + "bun", + [ + cliEntry, + "benchmark", + "tests/fixtures/es5/fixture-1.js", + "--format", + "json", + "--iterations", + "1", + ], + { + cwd: repoRoot, + } + ); + + expect(stderr).toBe(""); + + const trimmedStdout = stdout.trim(); + const parsed = JSON.parse(trimmedStdout) as { + files: Array<{ + results: Array<{ compressor: string }>; + }>; + }; + + expect( + parsed.files[0]?.results.map((result) => result.compressor) + ).toEqual(["terser", "esbuild", "swc", "oxc"]); + }, 60000); +}); diff --git a/packages/cli/src/bin/cli.ts b/packages/cli/src/bin/cli.ts index b00c21b0d..719df93e8 100644 --- a/packages/cli/src/bin/cli.ts +++ b/packages/cli/src/bin/cli.ts @@ -96,32 +96,35 @@ function setupProgram(): Command { .option("--brotli", "include brotli size") .option("-v, --verbose", "verbose output") .action(async (input, options) => { - const globalOpts = program.opts(); - const spinner = ora("Benchmarking...").start(); + const shouldUseSpinner = + options.format !== "json" && !!process.stderr.isTTY; + const spinner = shouldUseSpinner + ? ora("Benchmarking...").start() + : null; try { const results = await benchmark({ input, - compressors: - options.compressors?.split(",") || - globalOpts.compressor?.split(","), + compressors: options.compressors?.split(","), iterations: parseInt(options.iterations, 10), format: options.format, output: options.output, includeGzip: !!options.gzip, includeBrotli: !!options.brotli, - type: globalOpts.type, + type: program.opts().type, verbose: !!options.verbose, onProgress: (compressor: string, file: string) => { - spinner.text = `Benchmarking ${compressor} on ${file}...`; + if (spinner) { + spinner.text = `Benchmarking ${compressor} on ${file}...`; + } }, }); - spinner.stop(); + spinner?.stop(); const reporter = getReporter(options.format); console.log(reporter(results)); process.exit(0); } catch (error) { - spinner.fail("Benchmark failed"); + spinner?.fail("Benchmark failed"); console.error(error); process.exit(1); } diff --git a/packages/core/__tests__/core.test.ts b/packages/core/__tests__/core.test.ts index f890d543b..ce7f57aa8 100644 --- a/packages/core/__tests__/core.test.ts +++ b/packages/core/__tests__/core.test.ts @@ -216,6 +216,15 @@ describe("Package: core", async () => { expect(min).toBeDefined(); }); + test("should accept empty string content in memory mode", async () => { + const settings: Settings = { + compressor: noCompress, + content: "", + }; + + await expect(minify(settings)).resolves.toBe(""); + }); + test("should throw an error if binary does not exist", async () => { const settings: Settings = { compressor: "fake" as unknown as Compressor, diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 35940f6c0..9d7632492 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -22,6 +22,9 @@ export async function minify( settings: Settings ): Promise { const compressorSettings = setup(settings); - const method = settings.content ? compressSingleFile : compress; + const method = + compressorSettings.content !== undefined + ? compressSingleFile + : compress; return await method(compressorSettings); } diff --git a/packages/core/src/setup.ts b/packages/core/src/setup.ts index ef3606fd2..f137e8edb 100644 --- a/packages/core/src/setup.ts +++ b/packages/core/src/setup.ts @@ -33,8 +33,8 @@ function setup( } as Settings; // In memory - if (settings.content) { - validateMandatoryFields(inputSettings, ["compressor", "content"]); + if (settings.content !== undefined) { + validateMandatoryFields(inputSettings, ["compressor"]); return settings; } diff --git a/packages/run/__tests__/run.test.ts b/packages/run/__tests__/run.test.ts index 2ad9d2c47..3c5d1f0ee 100644 --- a/packages/run/__tests__/run.test.ts +++ b/packages/run/__tests__/run.test.ts @@ -166,7 +166,7 @@ describe("Package: run", () => { mockStdout.emit("error", new Error("stdout error")); mockStderr.emit("error", new Error("stderr error")); mockStdout.emit("data", Buffer.from("output")); - mockChild.emit("exit", 0); + mockChild.emit("close", 0); }); const result = await promise; @@ -221,7 +221,7 @@ describe("Package: run", () => { setImmediate(() => { mockStdin.emit("error", new Error("stdin error")); mockStdout.emit("data", Buffer.from("output")); - mockChild.emit("exit", 0); + mockChild.emit("close", 0); }); const result = await promise; @@ -231,6 +231,39 @@ describe("Package: run", () => { consoleSpy.mockRestore(); }); + test("should wait for close before resolving stdout", async () => { + const mockChild = new EventEmitter() as ReturnType< + typeof childProcess.spawn + >; + const mockStdin = new EventEmitter(); + const mockStdout = new EventEmitter(); + const mockStderr = new EventEmitter(); + + Object.assign(mockChild, { + stdin: Object.assign(mockStdin, { + end: vi.fn(), + }), + stdout: mockStdout, + stderr: mockStderr, + kill: vi.fn(), + }); + + spy.mockReturnValue(mockChild); + + const promise = runCommandLine({ + args: ["-jar", "fake.jar"], + data: "test", + }); + + setImmediate(() => { + mockChild.emit("exit", 0); + mockStdout.emit("data", Buffer.from("delayed-output")); + mockChild.emit("close", 0); + }); + + await expect(promise).resolves.toBe("delayed-output"); + }); + test("should reject when maxBuffer is exceeded", async () => { const mockChild = new EventEmitter() as ReturnType< typeof childProcess.spawn diff --git a/packages/run/src/index.ts b/packages/run/src/index.ts index 697f8a10e..ff1224a5d 100644 --- a/packages/run/src/index.ts +++ b/packages/run/src/index.ts @@ -96,7 +96,7 @@ export async function run({ child.stdout?.on("error", handleError("child.stdout")); child.stderr?.on("error", handleError("child.stderr")); - child.on("exit", (code: number | null) => { + child.on("close", (code: number | null) => { if (settled) return; settled = true; if (timeoutId) clearTimeout(timeoutId); diff --git a/packages/utils/__tests__/utils.test.ts b/packages/utils/__tests__/utils.test.ts index 26583bf5f..030c1c915 100644 --- a/packages/utils/__tests__/utils.test.ts +++ b/packages/utils/__tests__/utils.test.ts @@ -7,6 +7,7 @@ import { existsSync, lstatSync, unlinkSync } from "node:fs"; import path from "node:path"; import { fileURLToPath } from "node:url"; +import type { Settings } from "@node-minify/types"; import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; const __dirname = path.dirname(fileURLToPath(import.meta.url)); @@ -1247,11 +1248,11 @@ describe("Package: utils", () => { { format: "avif", content: avifContent }, ], }); - const settings = { + const settings: Settings = { compressor, input: testFile, output: ["$1", `${tmpDir}/explicit-output.avif`], - } as any; + }; await run({ settings, content: "" }); @@ -1265,6 +1266,39 @@ describe("Package: utils", () => { ); }); + test("should replace $1 inside per-format output arrays", async () => { + const testFile = `${tmpDir}/array-pattern-source.png`; + const webpOutput = `${tmpDir}/array-pattern-source-custom.webp`; + const avifOutput = `${tmpDir}/array-pattern-source-custom.avif`; + filesToCleanup.add(testFile); + filesToCleanup.add(webpOutput); + filesToCleanup.add(avifOutput); + writeFile({ file: testFile, content: "test" }); + + const webpContent = Buffer.from("WEBP_ARRAY_PATTERN"); + const avifContent = Buffer.from("AVIF_ARRAY_PATTERN"); + const compressor = vi.fn().mockResolvedValue({ + code: "", + outputs: [ + { format: "webp", content: webpContent }, + { format: "avif", content: avifContent }, + ], + }); + const settings: Settings = { + compressor, + input: testFile, + output: [ + `${tmpDir}/$1-custom.webp`, + `${tmpDir}/$1-custom.avif`, + ], + }; + + await run({ settings, content: "" }); + + expect(readFile(webpOutput, true)).toEqual(webpContent); + expect(readFile(avifOutput, true)).toEqual(avifContent); + }); + test("should handle $1 pattern in path (replace and append format)", async () => { const testFile = `${tmpDir}/my-image.png`; filesToCleanup.add(testFile); @@ -1289,6 +1323,85 @@ describe("Package: utils", () => { ); }); + test("should not double-append format for indexed $1 output patterns", async () => { + const testFile = `${tmpDir}/indexed-pattern-source.png`; + const outputFile = `${tmpDir}/indexed-pattern-source.webp`; + filesToCleanup.add(testFile); + filesToCleanup.add(outputFile); + writeFile({ file: testFile, content: "test" }); + + const webpContent = Buffer.from("WEBP_INDEXED_PATTERN"); + const compressor = vi.fn().mockResolvedValue({ + code: "", + outputs: [{ format: "webp", content: webpContent }], + }); + const settings: Settings = { + compressor, + input: [testFile], + output: [`${tmpDir}/$1.webp`], + }; + + await run({ settings, content: "", index: 0 }); + + expect(readFile(outputFile, true)).toEqual(webpContent); + }); + + test("should preserve explicit extension for single string multi-format outputs", async () => { + const testFile = `${tmpDir}/explicit-output-source.png`; + const outputFile = `${tmpDir}/explicit-output.webp`; + filesToCleanup.add(testFile); + filesToCleanup.add(outputFile); + writeFile({ file: testFile, content: "test" }); + + const webpContent = Buffer.from("WEBP_EXPLICIT_OUTPUT"); + const compressor = vi.fn().mockResolvedValue({ + code: "", + outputs: [{ format: "webp", content: webpContent }], + }); + const settings: Settings = { + compressor, + input: testFile, + output: outputFile, + }; + + await run({ settings, content: "" }); + + expect(readFile(outputFile, true)).toEqual(webpContent); + }); + + test("should append format to extensionless per-format array targets", async () => { + const testFile = `${tmpDir}/array-explicit-source.png`; + const webpOutput = `${tmpDir}/array-explicit-output.webp`; + const avifOutput = `${tmpDir}/array-explicit-other.avif`; + filesToCleanup.add(testFile); + filesToCleanup.add(webpOutput); + filesToCleanup.add(avifOutput); + writeFile({ file: testFile, content: "test" }); + + const webpContent = Buffer.from("WEBP_ARRAY_EXPLICIT"); + const avifContent = Buffer.from("AVIF_ARRAY_EXPLICIT"); + const compressor = vi.fn().mockResolvedValue({ + code: "", + outputs: [ + { format: "webp", content: webpContent }, + { format: "avif", content: avifContent }, + ], + }); + const settings: Settings = { + compressor, + input: testFile, + output: [ + `${tmpDir}/array-explicit-output`, + `${tmpDir}/array-explicit-other`, + ], + }; + + await run({ settings, content: "" }); + + expect(readFile(webpOutput, true)).toEqual(webpContent); + expect(readFile(avifOutput, true)).toEqual(avifContent); + }); + test("should handle empty string in output array (fallback to auto-generated)", async () => { const testFile = `${tmpDir}/fallback-test.png`; filesToCleanup.add(testFile); @@ -1468,6 +1581,63 @@ describe("Package: utils", () => { ); }); + test("should derive multi-format targets from the current input when processing input arrays", async () => { + const firstInput = `${tmpDir}/multi-array-first.png`; + const secondInput = `${tmpDir}/multi-array-second.png`; + const firstWebp = `${tmpDir}/multi-array-first.webp`; + const firstAvif = `${tmpDir}/multi-array-first.avif`; + const secondWebp = `${tmpDir}/multi-array-second.webp`; + const secondAvif = `${tmpDir}/multi-array-second.avif`; + filesToCleanup.add(firstInput); + filesToCleanup.add(secondInput); + filesToCleanup.add(firstWebp); + filesToCleanup.add(firstAvif); + filesToCleanup.add(secondWebp); + filesToCleanup.add(secondAvif); + writeFile({ file: firstInput, content: "first" }); + writeFile({ file: secondInput, content: "second" }); + + const compressor: Settings["compressor"] = async () => ({ + code: "", + outputs: [ + { format: "webp", content: Buffer.from("FORMAT_WEBP") }, + { format: "avif", content: Buffer.from("FORMAT_AVIF") }, + ], + }); + const settings: Settings = { + compressor, + input: [firstInput, secondInput], + output: [ + firstInput.replace(/\.png$/, ""), + secondInput.replace(/\.png$/, ""), + ], + }; + + await run({ + settings, + content: Buffer.from("first"), + index: 0, + }); + await run({ + settings, + content: Buffer.from("second"), + index: 1, + }); + + expect(readFile(firstWebp, true)).toEqual( + Buffer.from("FORMAT_WEBP") + ); + expect(readFile(firstAvif, true)).toEqual( + Buffer.from("FORMAT_AVIF") + ); + expect(readFile(secondWebp, true)).toEqual( + Buffer.from("FORMAT_WEBP") + ); + expect(readFile(secondAvif, true)).toEqual( + Buffer.from("FORMAT_AVIF") + ); + }); + test("should fallback to auto-generated path when output is non-string truthy value", async () => { const testFile = `${tmpDir}/fallback-nonstring.png`; filesToCleanup.add(testFile); diff --git a/packages/utils/src/compressSingleFile.ts b/packages/utils/src/compressSingleFile.ts index d90932d38..f0fbd306e 100644 --- a/packages/utils/src/compressSingleFile.ts +++ b/packages/utils/src/compressSingleFile.ts @@ -37,7 +37,7 @@ export async function compressSingleFile< async function determineContent< T extends CompressorOptions = CompressorOptions, >(settings: Settings): Promise { - if (settings.content) { + if (settings.content !== undefined) { return settings.content; } diff --git a/packages/utils/src/run.ts b/packages/utils/src/run.ts index ac43f724d..83c6210c0 100644 --- a/packages/utils/src/run.ts +++ b/packages/utils/src/run.ts @@ -89,7 +89,7 @@ async function writeOutput( settings: Settings, index?: number ): Promise { - const isInMemoryMode = Boolean(settings.content); + const isInMemoryMode = settings.content !== undefined; if (isInMemoryMode || !settings.output) { return; } @@ -144,16 +144,23 @@ async function writeOutput( } /** - * Extract the first input file path from the input configuration. + * Extract an input file path from the input configuration. * * @param input - A single file path, an array of paths, or undefined - * @returns The first input file path, or an empty string if none available + * @param index - Optional index of the current input being processed when `input` is an array + * @returns The indexed input file path when available, otherwise the first input file path or an empty string */ -function getFirstInputFile(input: string | string[] | undefined): string { +function getInputFile( + input: string | string[] | undefined, + index?: number +): string { if (typeof input === "string") { return input; } if (Array.isArray(input) && input.length > 0) { + if (index !== undefined) { + return input[index] ?? ""; + } return input[0] ?? ""; } return ""; @@ -176,11 +183,15 @@ async function writeMultipleOutputs< index?: number ): Promise { const output = settings.output; - const isArrayOutput = Array.isArray(output); - const outputsArray = isArrayOutput ? output : [output]; - const inputFile = getFirstInputFile(settings.input); + const perFormatOutputs = Array.isArray(output) && index === undefined; + const inputFile = getInputFile(settings.input, index); const inputDir = parse(inputFile).dir; const inputBase = parse(inputFile).name; + const baseName = inputBase || "output"; + const currentOutput = + Array.isArray(output) && index !== undefined ? output[index] : output; + const withFormatExtension = (file: string, format: string): string => + file.endsWith(`.${format}`) ? file : `${file}.${format}`; const promises = outputs.map(async (outputResult, i) => { if (!outputResult) { @@ -191,33 +202,47 @@ async function writeMultipleOutputs< const format = outputResult.format || "out"; let targetFile: string; - const arrayItem = outputsArray[i]; + const arrayItem = perFormatOutputs ? output[i] : undefined; if ( - isArrayOutput && + perFormatOutputs && arrayItem !== undefined && arrayItem !== "$1" && arrayItem.trim() !== "" ) { - // Explicit output path provided in array - use as-is - targetFile = arrayItem; - } else if (typeof output === "string" && output === "$1") { + if (arrayItem.includes("$1")) { + const outputFile = arrayItem.replace(/\$1/g, baseName); + targetFile = parse(outputFile).ext + ? outputFile + : withFormatExtension(outputFile, format); + } else { + targetFile = parse(arrayItem).ext + ? arrayItem + : withFormatExtension(arrayItem, format); + } + } else if ( + typeof currentOutput === "string" && + currentOutput === "$1" + ) { // $1 only: auto-generate from input filename in input directory - const baseName = inputBase || "output"; targetFile = inputDir ? join(inputDir, `${baseName}.${format}`) : `${baseName}.${format}`; - } else if (typeof output === "string" && output.includes("$1")) { + } else if ( + typeof currentOutput === "string" && + currentOutput.includes("$1") + ) { // $1 pattern in path: replace and append format - const extensionlessName = inputBase || "output"; - const outputFile = output.replace(/\$1/g, extensionlessName); - targetFile = `${outputFile}.${format}`; - } else if (typeof output === "string") { + const outputFile = currentOutput.replace(/\$1/g, baseName); + targetFile = withFormatExtension(outputFile, format); + } else if ( + typeof currentOutput === "string" && + currentOutput.trim() !== "" + ) { // Single string output: append format extension - targetFile = `${output}.${format}`; + targetFile = withFormatExtension(currentOutput, format); } else { // Fallback - const baseName = inputBase || "output"; targetFile = inputDir ? join(inputDir, `${baseName}.${format}`) : `${baseName}.${format}`;