diff --git a/packages/action/__tests__/compare.test.ts b/packages/action/__tests__/compare.test.ts new file mode 100644 index 000000000..55fc7212e --- /dev/null +++ b/packages/action/__tests__/compare.test.ts @@ -0,0 +1,544 @@ +/*! node-minify action tests - MIT Licensed */ + +import { beforeEach, describe, expect, test, vi } from "vitest"; + +vi.mock("@actions/core", () => ({ + info: vi.fn(), + warning: vi.fn(), +})); + +vi.mock("@actions/github", () => ({ + context: { + payload: {}, + repo: { owner: "test-owner", repo: "test-repo" }, + }, + getOctokit: vi.fn(), +})); + +import { info, warning } from "@actions/core"; +import { context, getOctokit } from "@actions/github"; +import { + calculateTotalChange, + compareWithBase, + formatChange, + hasIncrease, +} from "../src/compare.ts"; +import type { ComparisonResult, MinifyResult } from "../src/types.ts"; + +describe("formatChange", () => { + test("formats size increase with warning emoji", () => { + const comparison: ComparisonResult = { + file: "app.min.js", + baseSize: 1000, + currentSize: 1050, + change: 5, + isNew: false, + }; + expect(formatChange(comparison)).toBe("+5.0% ⚠️"); + }); + + test("formats size decrease with success emoji", () => { + const comparison: ComparisonResult = { + file: "app.min.js", + baseSize: 1000, + currentSize: 900, + change: -10, + isNew: false, + }; + expect(formatChange(comparison)).toBe("-10.0% ✅"); + }); + + test("formats no change", () => { + const comparison: ComparisonResult = { + file: "app.min.js", + baseSize: 1000, + currentSize: 1000, + change: 0, + isNew: false, + }; + expect(formatChange(comparison)).toBe("+0.0% ✅"); + }); + + test("returns 'new' for new files", () => { + const comparison: ComparisonResult = { + file: "app.min.js", + baseSize: null, + currentSize: 1000, + change: null, + isNew: true, + }; + expect(formatChange(comparison)).toBe("new"); + }); + + test("returns 'new' when change is null", () => { + const comparison: ComparisonResult = { + file: "app.min.js", + baseSize: null, + currentSize: 500, + change: null, + isNew: false, + }; + expect(formatChange(comparison)).toBe("new"); + }); +}); + +describe("hasIncrease", () => { + test("detects size increase", () => { + const comparisons: ComparisonResult[] = [ + { + file: "a.js", + baseSize: 1000, + currentSize: 900, + change: -10, + isNew: false, + }, + { + file: "b.js", + baseSize: 1000, + currentSize: 1100, + change: 10, + isNew: false, + }, + ]; + expect(hasIncrease(comparisons)).toBe(true); + }); + + test("returns false when all files decreased", () => { + const comparisons: ComparisonResult[] = [ + { + file: "a.js", + baseSize: 1000, + currentSize: 900, + change: -10, + isNew: false, + }, + { + file: "b.js", + baseSize: 1000, + currentSize: 950, + change: -5, + isNew: false, + }, + ]; + expect(hasIncrease(comparisons)).toBe(false); + }); + + test("ignores new files", () => { + const comparisons: ComparisonResult[] = [ + { + file: "a.js", + baseSize: null, + currentSize: 1000, + change: null, + isNew: true, + }, + ]; + expect(hasIncrease(comparisons)).toBe(false); + }); + + test("returns false for empty array", () => { + expect(hasIncrease([])).toBe(false); + }); +}); + +describe("calculateTotalChange", () => { + test("calculates total change across files", () => { + const comparisons: ComparisonResult[] = [ + { + file: "a.js", + baseSize: 1000, + currentSize: 800, + change: -20, + isNew: false, + }, + { + file: "b.js", + baseSize: 1000, + currentSize: 900, + change: -10, + isNew: false, + }, + ]; + + const result = calculateTotalChange(comparisons); + expect(result.totalBaseSize).toBe(2000); + expect(result.totalCurrentSize).toBe(1700); + expect(result.totalChangePercent).toBe(-15); + }); + + test("excludes new files from base calculation", () => { + const comparisons: ComparisonResult[] = [ + { + file: "existing.js", + baseSize: 1000, + currentSize: 800, + change: -20, + isNew: false, + }, + { + file: "new.js", + baseSize: null, + currentSize: 500, + change: null, + isNew: true, + }, + ]; + + const result = calculateTotalChange(comparisons); + expect(result.totalBaseSize).toBe(1000); + expect(result.totalCurrentSize).toBe(1300); + // totalChangePercent uses only comparable files (800 vs 1000 = -20%) + expect(result.totalChangePercent).toBe(-20); + }); + + test("handles all new files", () => { + const comparisons: ComparisonResult[] = [ + { + file: "a.js", + baseSize: null, + currentSize: 500, + change: null, + isNew: true, + }, + { + file: "b.js", + baseSize: null, + currentSize: 300, + change: null, + isNew: true, + }, + ]; + + const result = calculateTotalChange(comparisons); + expect(result.totalBaseSize).toBe(0); + expect(result.totalCurrentSize).toBe(800); + expect(result.totalChangePercent).toBeNull(); + }); + + test("handles empty array", () => { + const result = calculateTotalChange([]); + expect(result.totalBaseSize).toBe(0); + expect(result.totalCurrentSize).toBe(0); + expect(result.totalChangePercent).toBeNull(); + }); + + test("handles size increase", () => { + const comparisons: ComparisonResult[] = [ + { + file: "a.js", + baseSize: 1000, + currentSize: 1200, + change: 20, + isNew: false, + }, + ]; + + const result = calculateTotalChange(comparisons); + expect(result.totalChangePercent).toBe(20); + }); + + test("handles zero total base size with comparable files", () => { + const comparisons: ComparisonResult[] = [ + { + file: "a.js", + baseSize: 0, + currentSize: 100, + change: 100, + isNew: false, + }, + ]; + + const result = calculateTotalChange(comparisons); + expect(result.totalBaseSize).toBe(0); + expect(result.totalCurrentSize).toBe(100); + expect(result.totalChangePercent).toBe(0); + }); +}); + +describe("compareWithBase", () => { + const mockResult: MinifyResult = { + files: [ + { + file: "dist/app.min.js", + originalSize: 10000, + minifiedSize: 3000, + reduction: 70, + timeMs: 50, + }, + ], + compressor: "terser", + totalOriginalSize: 10000, + totalMinifiedSize: 3000, + totalReduction: 70, + totalTimeMs: 50, + }; + + beforeEach(() => { + vi.resetAllMocks(); + (context as { payload: Record }).payload = {}; + }); + + test("returns empty array when no token provided", async () => { + const result = await compareWithBase(mockResult, undefined); + + expect(result).toEqual([]); + expect(warning).toHaveBeenCalledWith( + "No GitHub token provided, skipping base branch comparison" + ); + }); + + test("returns empty array when not a pull request", async () => { + (context as { payload: Record }).payload = {}; + + const result = await compareWithBase(mockResult, "fake-token"); + + expect(result).toEqual([]); + expect(info).toHaveBeenCalledWith( + "Not a pull request, skipping base branch comparison" + ); + }); + + test("compares files against base branch", async () => { + (context as { payload: Record }).payload = { + pull_request: { base: { ref: "main" } }, + }; + + const mockGetContent = vi.fn().mockResolvedValue({ + data: { type: "file", size: 3500 }, + }); + + vi.mocked(getOctokit).mockReturnValue({ + rest: { + repos: { getContent: mockGetContent }, + }, + } as unknown as ReturnType); + + const result = await compareWithBase(mockResult, "fake-token"); + + expect(result).toHaveLength(1); + expect(result[0]?.file).toBe("dist/app.min.js"); + expect(result[0]?.baseSize).toBe(3500); + expect(result[0]?.currentSize).toBe(3000); + expect(result[0]?.isNew).toBe(false); + expect(result[0]?.change).toBeCloseTo(-14.29, 1); + + expect(mockGetContent).toHaveBeenCalledWith({ + owner: "test-owner", + repo: "test-repo", + path: "dist/app.min.js", + ref: "main", + }); + }); + + test("handles new file (404 error)", async () => { + (context as { payload: Record }).payload = { + pull_request: { base: { ref: "main" } }, + }; + + const notFoundError = Object.assign(new Error("Not Found"), { + status: 404, + }); + const mockGetContent = vi.fn().mockRejectedValue(notFoundError); + + vi.mocked(getOctokit).mockReturnValue({ + rest: { + repos: { getContent: mockGetContent }, + }, + } as unknown as ReturnType); + + const result = await compareWithBase(mockResult, "fake-token"); + + expect(result).toHaveLength(1); + expect(result[0]?.isNew).toBe(true); + expect(result[0]?.baseSize).toBeNull(); + expect(result[0]?.change).toBeNull(); + }); + + test("handles directory response as new file", async () => { + (context as { payload: Record }).payload = { + pull_request: { base: { ref: "main" } }, + }; + + const mockGetContent = vi.fn().mockResolvedValue({ + data: [{ type: "file", name: "index.js" }], + }); + + vi.mocked(getOctokit).mockReturnValue({ + rest: { + repos: { getContent: mockGetContent }, + }, + } as unknown as ReturnType); + + const result = await compareWithBase(mockResult, "fake-token"); + + expect(result[0]?.isNew).toBe(true); + }); + + test("handles symlink/submodule as new file", async () => { + (context as { payload: Record }).payload = { + pull_request: { base: { ref: "main" } }, + }; + + const mockGetContent = vi.fn().mockResolvedValue({ + data: { type: "symlink", target: "some-target" }, + }); + + vi.mocked(getOctokit).mockReturnValue({ + rest: { + repos: { getContent: mockGetContent }, + }, + } as unknown as ReturnType); + + const result = await compareWithBase(mockResult, "fake-token"); + + expect(result[0]?.isNew).toBe(true); + }); + + test("handles unexpected errors gracefully", async () => { + (context as { payload: Record }).payload = { + pull_request: { base: { ref: "main" } }, + }; + + const unexpectedError = new Error("Network error"); + const mockGetContent = vi.fn().mockRejectedValue(unexpectedError); + + vi.mocked(getOctokit).mockReturnValue({ + rest: { + repos: { getContent: mockGetContent }, + }, + } as unknown as ReturnType); + + const result = await compareWithBase(mockResult, "fake-token"); + + expect(result[0]?.isNew).toBe(true); + expect(warning).toHaveBeenCalledWith( + expect.stringContaining("Failed to fetch base branch version") + ); + }); + + test("handles zero base size", async () => { + (context as { payload: Record }).payload = { + pull_request: { base: { ref: "main" } }, + }; + + const mockGetContent = vi.fn().mockResolvedValue({ + data: { type: "file", size: 0 }, + }); + + vi.mocked(getOctokit).mockReturnValue({ + rest: { + repos: { getContent: mockGetContent }, + }, + } as unknown as ReturnType); + + const result = await compareWithBase(mockResult, "fake-token"); + + expect(result[0]?.baseSize).toBe(0); + expect(result[0]?.change).toBe(100); + }); + + test("handles multiple files", async () => { + const multiFileResult: MinifyResult = { + files: [ + { + file: "dist/a.js", + originalSize: 5000, + minifiedSize: 1500, + reduction: 70, + timeMs: 25, + }, + { + file: "dist/b.js", + originalSize: 5000, + minifiedSize: 1500, + reduction: 70, + timeMs: 25, + }, + ], + compressor: "terser", + totalOriginalSize: 10000, + totalMinifiedSize: 3000, + totalReduction: 70, + totalTimeMs: 50, + }; + + (context as { payload: Record }).payload = { + pull_request: { base: { ref: "main" } }, + }; + + const mockGetContent = vi + .fn() + .mockResolvedValueOnce({ data: { type: "file", size: 1600 } }) + .mockResolvedValueOnce({ data: { type: "file", size: 1400 } }); + + vi.mocked(getOctokit).mockReturnValue({ + rest: { + repos: { getContent: mockGetContent }, + }, + } as unknown as ReturnType); + + const result = await compareWithBase(multiFileResult, "fake-token"); + + expect(result).toHaveLength(2); + expect(mockGetContent).toHaveBeenCalledTimes(2); + }); + + test("handles zero base size with zero current size", async () => { + const zeroResult: MinifyResult = { + files: [ + { + file: "dist/empty.js", + originalSize: 0, + minifiedSize: 0, + reduction: 0, + timeMs: 1, + }, + ], + compressor: "terser", + totalOriginalSize: 0, + totalMinifiedSize: 0, + totalReduction: 0, + totalTimeMs: 1, + }; + + (context as { payload: Record }).payload = { + pull_request: { base: { ref: "main" } }, + }; + + const mockGetContent = vi.fn().mockResolvedValue({ + data: { type: "file", size: 0 }, + }); + + vi.mocked(getOctokit).mockReturnValue({ + rest: { + repos: { getContent: mockGetContent }, + }, + } as unknown as ReturnType); + + const result = await compareWithBase(zeroResult, "fake-token"); + + expect(result[0]?.baseSize).toBe(0); + expect(result[0]?.change).toBe(0); + }); + + test("handles non-Error thrown values", async () => { + (context as { payload: Record }).payload = { + pull_request: { base: { ref: "main" } }, + }; + + const mockGetContent = vi.fn().mockRejectedValue("string error"); + + vi.mocked(getOctokit).mockReturnValue({ + rest: { + repos: { getContent: mockGetContent }, + }, + } as unknown as ReturnType); + + const result = await compareWithBase(mockResult, "fake-token"); + + expect(result[0]?.isNew).toBe(true); + expect(warning).toHaveBeenCalledWith( + expect.stringContaining("string error") + ); + }); +}); diff --git a/packages/action/src/compare.ts b/packages/action/src/compare.ts new file mode 100644 index 000000000..2c54723f8 --- /dev/null +++ b/packages/action/src/compare.ts @@ -0,0 +1,229 @@ +/*! + * node-minify + * Copyright (c) 2011-2026 Rodolphe Stoclin + * MIT Licensed + */ + +import { info, warning } from "@actions/core"; +import { context, getOctokit } from "@actions/github"; +import type { ComparisonResult, MinifyResult } from "./types.ts"; + +/** + * Compares minified file sizes against the base branch for pull requests. + * + * Fetches the minified output files from the base branch (if they exist) and + * computes the size difference. This allows PR comments to show before/after + * comparisons. + * + * @param result - The minification result containing file information + * @param githubToken - GitHub API token for fetching base branch files + * @returns Array of comparison results for each file, or empty array if not a PR or no token + */ +export async function compareWithBase( + result: MinifyResult, + githubToken: string | undefined +): Promise { + if (!githubToken) { + warning("No GitHub token provided, skipping base branch comparison"); + return []; + } + + const pullRequest = context.payload.pull_request; + if (!pullRequest) { + info("Not a pull request, skipping base branch comparison"); + return []; + } + + const baseBranch = pullRequest.base.ref as string; + const octokit = getOctokit(githubToken); + const { owner, repo } = context.repo; + + const results = await Promise.all( + result.files.map((fileResult) => + compareFile( + octokit, + owner, + repo, + baseBranch, + fileResult.file, + fileResult.minifiedSize + ) + ) + ); + + return results; +} + +/** + * Compares a single file against the base branch version. + * + * @param octokit - Authenticated Octokit instance + * @param owner - Repository owner + * @param repo - Repository name + * @param baseBranch - Base branch ref (e.g., "main") + * @param filePath - Path to the file to compare + * @param currentSize - Current minified size in bytes + * @returns Comparison result with base size (null if file is new) and change percentage + */ +async function compareFile( + octokit: ReturnType, + owner: string, + repo: string, + baseBranch: string, + filePath: string, + currentSize: number +): Promise { + try { + const { data } = await octokit.rest.repos.getContent({ + owner, + repo, + path: filePath, + ref: baseBranch, + }); + + // getContent returns different shapes depending on the path + // For a single file, it returns an object with content + if (Array.isArray(data)) { + // Path is a directory, not a file + return { + file: filePath, + baseSize: null, + currentSize, + change: null, + isNew: true, + }; + } + + if (data.type !== "file" || !("size" in data)) { + // Not a regular file (could be a symlink or submodule) + return { + file: filePath, + baseSize: null, + currentSize, + change: null, + isNew: true, + }; + } + + const baseSize = data.size; + const change = + baseSize > 0 + ? ((currentSize - baseSize) / baseSize) * 100 + : currentSize > 0 + ? 100 + : 0; + + return { + file: filePath, + baseSize, + currentSize, + change, + isNew: false, + }; + } catch (error) { + // File doesn't exist in base branch (new file) or other error + if ( + error instanceof Error && + "status" in error && + (error as { status: number }).status === 404 + ) { + return { + file: filePath, + baseSize: null, + currentSize, + change: null, + isNew: true, + }; + } + + // Log unexpected errors but don't fail the action + warning( + `Failed to fetch base branch version of ${filePath}: ${error instanceof Error ? error.message : String(error)}` + ); + + return { + file: filePath, + baseSize: null, + currentSize, + change: null, + isNew: true, + }; + } +} + +/** + * Formats a comparison result for display (e.g., in PR comments). + * + * @param comparison - The comparison result to format + * @returns Formatted string like "+2.5% ⚠️", "-1.6% ✅", or "new" for new files + */ +export function formatChange(comparison: ComparisonResult): string { + if (comparison.isNew || comparison.change === null) { + return "new"; + } + + const sign = comparison.change >= 0 ? "+" : ""; + const emoji = comparison.change > 0 ? "⚠️" : "✅"; + + return `${sign}${comparison.change.toFixed(1)}% ${emoji}`; +} + +/** + * Checks if any comparison shows a size increase. + * + * @param comparisons - Array of comparison results + * @returns True if any file increased in size + */ +export function hasIncrease(comparisons: ComparisonResult[]): boolean { + return comparisons.some( + (c) => !c.isNew && c.change !== null && c.change > 0 + ); +} + +/** + * Calculates the total size change across all compared files. + * + * @param comparisons - Array of comparison results + * @returns Object with totalBaseSize, totalCurrentSize, and totalChangePercent (null if no comparable files) + */ +export function calculateTotalChange(comparisons: ComparisonResult[]): { + totalBaseSize: number; + totalCurrentSize: number; + totalChangePercent: number | null; +} { + const totalCurrentSize = comparisons.reduce( + (sum, c) => sum + c.currentSize, + 0 + ); + + const comparable = comparisons.filter( + (c) => !c.isNew && c.baseSize !== null + ); + + if (comparable.length === 0) { + return { + totalBaseSize: 0, + totalCurrentSize, + totalChangePercent: null, + }; + } + + const totalBaseSize = comparable.reduce( + (sum, c) => sum + (c.baseSize ?? 0), + 0 + ); + const comparableCurrentSize = comparable.reduce( + (sum, c) => sum + c.currentSize, + 0 + ); + const totalChangePercent = + totalBaseSize > 0 + ? ((comparableCurrentSize - totalBaseSize) / totalBaseSize) * 100 + : 0; + + return { + totalBaseSize, + totalCurrentSize, + totalChangePercent, + }; +} diff --git a/packages/action/src/index.ts b/packages/action/src/index.ts index 057edbbc7..02f706d76 100644 --- a/packages/action/src/index.ts +++ b/packages/action/src/index.ts @@ -8,6 +8,7 @@ import { info, setFailed } from "@actions/core"; import { context } from "@actions/github"; import { runBenchmark } from "./benchmark.ts"; import { checkThresholds } from "./checks.ts"; +import { compareWithBase } from "./compare.ts"; import { parseInputs, validateCompressor } from "./inputs.ts"; import { runMinification } from "./minify.ts"; import { setBenchmarkOutputs, setMinifyOutputs } from "./outputs.ts"; @@ -42,7 +43,11 @@ async function run(): Promise { } if (inputs.reportPRComment && context.payload.pull_request) { - await postPRComment(result, inputs.githubToken); + const comparisons = await compareWithBase( + result, + inputs.githubToken + ); + await postPRComment(result, inputs.githubToken, comparisons); } if (inputs.reportAnnotations) { diff --git a/packages/action/src/reporters/comment.ts b/packages/action/src/reporters/comment.ts index b244e2f79..4f1cd2193 100644 --- a/packages/action/src/reporters/comment.ts +++ b/packages/action/src/reporters/comment.ts @@ -7,7 +7,8 @@ import { info, warning } from "@actions/core"; import { context, getOctokit } from "@actions/github"; import { prettyBytes } from "@node-minify/utils"; -import type { MinifyResult } from "../types.ts"; +import { formatChange } from "../compare.ts"; +import type { ComparisonResult, MinifyResult } from "../types.ts"; const COMMENT_TAG = ""; @@ -16,10 +17,12 @@ const COMMENT_TAG = ""; * * @param result - Minification results containing per-file metrics, totals, compressor name, and execution time * @param githubToken - GitHub API token used to authenticate requests; when `undefined`, the function skips posting + * @param comparisons - Optional comparison results against base branch for showing size changes */ export async function postPRComment( result: MinifyResult, - githubToken: string | undefined + githubToken: string | undefined, + comparisons: ComparisonResult[] = [] ): Promise { if (!githubToken) { warning("No GitHub token provided, skipping PR comment"); @@ -36,7 +39,7 @@ export async function postPRComment( const octokit = getOctokit(githubToken); const { owner, repo } = context.repo; - const body = generateCommentBody(result); + const body = generateCommentBody(result, comparisons); const comments = await octokit.paginate( octokit.rest.issues.listComments, @@ -79,22 +82,35 @@ export async function postPRComment( /** * Builds the Markdown body for the node-minify PR comment, including a per-file table, totals, and a configuration section. * - * @param result - Minification results used to populate the report (expects `files`, `totalOriginalSize`, `totalMinifiedSize`, `totalReduction`, `compressor`, and `totalTimeMs`) - * @returns The Markdown string for the PR comment, beginning with `COMMENT_TAG` and containing the file table, total summary, and configuration details + * @param result - Minification results used to populate the report + * @param comparisons - Comparison results against base branch + * @returns The Markdown string for the PR comment */ -function generateCommentBody(result: MinifyResult): string { +function generateCommentBody( + result: MinifyResult, + comparisons: ComparisonResult[] +): string { + const hasComparisons = comparisons.length > 0; + const comparisonMap = new Map(comparisons.map((c) => [c.file, c])); + const filesTable = result.files - .map( - (f) => - `| \`${f.file}\` | ${prettyBytes(f.originalSize)} | ${prettyBytes(f.minifiedSize)} | ${f.reduction.toFixed(1)}% |` - ) + .map((f) => { + const comparison = comparisonMap.get(f.file); + const changeCol = hasComparisons + ? ` ${comparison ? formatChange(comparison) : "-"} |` + : ""; + return `| \`${f.file}\` | ${prettyBytes(f.originalSize)} | ${prettyBytes(f.minifiedSize)} | ${f.reduction.toFixed(1)}% |${changeCol}`; + }) .join("\n"); + const headerChange = hasComparisons ? " vs Base |" : ""; + const headerSep = hasComparisons ? "---------|" : ""; + return `${COMMENT_TAG} ## 📦 node-minify Report -| File | Original | Minified | Reduction | -|------|----------|----------|-----------| +| File | Original | Minified | Reduction |${headerChange} +|------|----------|----------|-----------|${headerSep} ${filesTable} **Total:** ${prettyBytes(result.totalOriginalSize)} → ${prettyBytes(result.totalMinifiedSize)} (${result.totalReduction.toFixed(1)}% reduction)