diff --git a/build-tools/packages/build-cli/package.json b/build-tools/packages/build-cli/package.json index 623922dc7aeb..ebe9b2b8bebb 100644 --- a/build-tools/packages/build-cli/package.json +++ b/build-tools/packages/build-cli/package.json @@ -111,7 +111,6 @@ "issue-parser": "^7.0.1", "json5": "^2.2.3", "jssm": "^5.104.2", - "jszip": "^3.10.1", "latest-version": "^9.0.0", "mdast": "^3.0.0", "mdast-util-heading-range": "^4.0.0", diff --git a/build-tools/packages/build-cli/src/codeCoverage/getCoverageMetrics.ts b/build-tools/packages/build-cli/src/codeCoverage/getCoverageMetrics.ts index ce823da853de..2115954d02f2 100644 --- a/build-tools/packages/build-cli/src/codeCoverage/getCoverageMetrics.ts +++ b/build-tools/packages/build-cli/src/codeCoverage/getCoverageMetrics.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. */ -import type JSZip from "jszip"; +import type { UnzippedContents } from "@fluidframework/bundle-size-tools"; import { Parser } from "xml2js"; import type { CommandLogger } from "../logging.js"; @@ -56,20 +56,20 @@ const extractCoverageMetrics = ( /** * Method that returns the coverage report for the build from the artifact. - * @param baselineZip - zipped coverage files for the build + * @param artifactZip - unzipped coverage files for the build * @param logger - The logger to log messages. * @returns an map of coverage metrics for build containing packageName, lineCoverage and branchCoverage */ export const getCoverageMetricsFromArtifact = async ( - artifactZip: JSZip, + artifactZip: UnzippedContents, logger?: CommandLogger, ): Promise> => { const coverageReportsFiles: string[] = []; - // eslint-disable-next-line unicorn/no-array-for-each -- required as JSZip does not implement [Symbol.iterator]() which is required by for...of - artifactZip.forEach((filePath) => { - if (filePath.endsWith("cobertura-coverage-patched.xml")) + for (const filePath of artifactZip.keys()) { + if (filePath.endsWith("cobertura-coverage-patched.xml")) { coverageReportsFiles.push(filePath); - }); + } + } let coverageMetricsForBaseline: Map = new Map(); const xmlParser = new Parser(); @@ -78,29 +78,22 @@ export const getCoverageMetricsFromArtifact = async ( logger?.info(`${coverageReportsFiles.length} coverage data files found.`); for (const coverageReportFile of coverageReportsFiles) { - const jsZipObject = artifactZip.file(coverageReportFile); - if (jsZipObject === undefined) { + const coverageReportXML = artifactZip.get(coverageReportFile); + if (coverageReportXML === undefined) { logger?.warning( `could not find file ${coverageReportFile} in the code coverage artifact`, ); + continue; } - // eslint-disable-next-line no-await-in-loop -- Since we only need 1 report file, it is easier to run it serially rather than extracting all jsZipObjects and then awaiting promises in parallel - const coverageReportXML = await jsZipObject?.async("nodebuffer"); - if (coverageReportXML !== undefined) { - xmlParser.parseString( - coverageReportXML, - (err: Error | null, result: unknown): void => { - if (err) { - console.warn(`Error processing file ${coverageReportFile}: ${err}`); - return; - } - coverageMetricsForBaseline = extractCoverageMetrics( - result as XmlCoverageReportSchema, - ); - }, - ); - } + xmlParser.parseString(coverageReportXML, (err: Error | null, result: unknown): void => { + if (err) { + console.warn(`Error processing file ${coverageReportFile}: ${err}`); + return; + } + coverageMetricsForBaseline = extractCoverageMetrics(result as XmlCoverageReportSchema); + }); + if (coverageMetricsForBaseline.size > 0) { break; } diff --git a/build-tools/packages/build-cli/src/library/azureDevops/getBaselineBuildMetrics.ts b/build-tools/packages/build-cli/src/library/azureDevops/getBaselineBuildMetrics.ts index 856927f0fab2..8b86ca9381ec 100644 --- a/build-tools/packages/build-cli/src/library/azureDevops/getBaselineBuildMetrics.ts +++ b/build-tools/packages/build-cli/src/library/azureDevops/getBaselineBuildMetrics.ts @@ -4,11 +4,13 @@ */ import { strict as assert } from "node:assert"; -import { getZipObjectFromArtifact } from "@fluidframework/bundle-size-tools"; +import { + type UnzippedContents, + getZipObjectFromArtifact, +} from "@fluidframework/bundle-size-tools"; import type { WebApi } from "azure-devops-node-api"; import { BuildResult } from "azure-devops-node-api/interfaces/BuildInterfaces.js"; import type { Build } from "azure-devops-node-api/interfaces/BuildInterfaces.js"; -import type JSZip from "jszip"; import type { CommandLogger } from "../../logging.js"; import type { IAzureDevopsBuildCoverageConstants } from "./constants.js"; import { getBuild, getBuilds } from "./utils.js"; @@ -16,9 +18,9 @@ import { getBuild, getBuilds } from "./utils.js"; export interface IBuildMetrics { build: Build & { id: number }; /** - * The artifact that was published by the PR build in zip format + * The artifact that was published by the PR build as unzipped contents */ - artifactZip: JSZip; + artifactZip: UnzippedContents; } /** @@ -43,7 +45,7 @@ export async function getBaselineBuildMetrics( }); let baselineBuild: Build | undefined; - let baselineArtifactZip: JSZip | undefined; + let baselineArtifactZip: UnzippedContents | undefined; for (const build of recentBuilds) { if (build.result !== BuildResult.Succeeded) { continue; @@ -159,7 +161,7 @@ export async function getBuildArtifactForSpecificBuild( `codeCoverageAnalysisArtifactName: ${azureDevopsBuildCoverageConstants.artifactName}`, ); - const artifactZip: JSZip | undefined = await getZipObjectFromArtifact( + const artifactZip: UnzippedContents | undefined = await getZipObjectFromArtifact( adoConnection, azureDevopsBuildCoverageConstants.projectName, build.id, diff --git a/build-tools/packages/bundle-size-tools/api-report/bundle-size-tools.api.md b/build-tools/packages/bundle-size-tools/api-report/bundle-size-tools.api.md index 591c1ae61c3a..ba4dde8fe006 100644 --- a/build-tools/packages/bundle-size-tools/api-report/bundle-size-tools.api.md +++ b/build-tools/packages/bundle-size-tools/api-report/bundle-size-tools.api.md @@ -9,8 +9,6 @@ import { Build } from 'azure-devops-node-api/interfaces/BuildInterfaces'; import type { CommentThreadStatus } from 'azure-devops-node-api/interfaces/GitInterfaces'; import { Compiler } from 'webpack'; -import type JSZip from 'jszip'; -import { jszip } from 'jszip'; import type { StatsCompilation } from 'webpack'; import { WebApi } from 'azure-devops-node-api'; import type Webpack from 'webpack'; @@ -183,7 +181,7 @@ export function getBuilds(adoConnection: WebApi, options: GetBuildOptions): Prom export function getBuildTagForCommit(commitHash: string): string; // @public -export function getBundleBuddyConfigFileFromZip(jsZip: JSZip, relativePath: string): Promise; +export function getBundleBuddyConfigFileFromZip(files: UnzippedContents, relativePath: string): Promise; // @public export function getBundleBuddyConfigFromFileSystem(path: string): Promise; @@ -209,7 +207,7 @@ export function getBundleFilePathsFromFolder(relativePathsInFolder: string[]): B export function getBundlePathsFromFileSystem(bundleReportPath: string): Promise; // @public -export function getBundlePathsFromZipObject(jsZip: JSZip): BundleFileData[]; +export function getBundlePathsFromZipObject(files: UnzippedContents): BundleFileData[]; // @public (undocumented) export function getBundleSummaries(args: GetBundleSummariesArgs): Promise; @@ -251,13 +249,13 @@ export function getSimpleComment(message: string, baselineCommit: string): strin export function getStatsFileFromFileSystem(path: string): Promise; // @public -export function getStatsFileFromZip(jsZip: JSZip, relativePath: string): Promise; +export function getStatsFileFromZip(files: UnzippedContents, relativePath: string): Promise; // @public export function getTotalSizeStatsProcessor(options: TotalSizeStatsProcessorOptions): WebpackStatsProcessor; // @public -export function getZipObjectFromArtifact(adoConnection: WebApi, projectName: string, buildNumber: number, bundleAnalysisArtifactName: string): Promise; +export function getZipObjectFromArtifact(adoConnection: WebApi, projectName: string, buildNumber: number, bundleAnalysisArtifactName: string): Promise; // @public (undocumented) export interface IADOConstants { @@ -295,8 +293,11 @@ export interface TotalSizeStatsProcessorOptions { metricName: string; } -// @public (undocumented) -export function unzipStream(stream: NodeJS.ReadableStream): Promise; +// @public +export type UnzippedContents = Map; + +// @public +export function unzipStream(stream: NodeJS.ReadableStream, baseFolder?: string): Promise; // @public export type WebpackStatsProcessor = (stats: StatsCompilation, config: BundleBuddyConfig | undefined) => BundleMetricSet | undefined; diff --git a/build-tools/packages/bundle-size-tools/package.json b/build-tools/packages/bundle-size-tools/package.json index dfcb39be9868..5fd6cd3c42bb 100644 --- a/build-tools/packages/bundle-size-tools/package.json +++ b/build-tools/packages/bundle-size-tools/package.json @@ -25,10 +25,11 @@ "build:compile": "fluid-build --task compile", "build:copy": "copyfiles -u 1 \"src/**/*.fsl\" dist", "build:docs": "api-extractor run --local", + "build:test": "tsc --project ./src/test/tsconfig.json", "check:biome": "biome check .", "check:format": "npm run check:biome", "ci:build:docs": "api-extractor run", - "clean": "rimraf --glob dist \"*.tsbuildinfo\" _api-extractor-temp \"*.build.log\"", + "clean": "rimraf --glob dist lib \"*.tsbuildinfo\" _api-extractor-temp \"*.build.log\" nyc", "compile": "fluid-build . --task compile", "eslint": "eslint --format stylish src", "eslint:fix": "eslint --format stylish src --fix", @@ -36,13 +37,13 @@ "format:biome": "biome check --write .", "lint": "npm run eslint", "lint:fix": "npm run eslint:fix", - "test": "echo \"Error: no test specified\" && exit 1", + "test": "npm run test:mocha", + "test:mocha": "mocha --forbid-only \"lib/test/**/*.test.js\"", "tsc": "tsc" }, "dependencies": { "azure-devops-node-api": "^11.2.0", "fflate": "^0.8.2", - "jszip": "^3.10.1", "msgpack-lite": "^0.1.26", "typescript": "~5.4.5", "webpack": "^5.103.0" @@ -53,10 +54,15 @@ "@fluidframework/build-tools-bin": "npm:@fluidframework/build-tools@~0.49.0", "@fluidframework/eslint-config-fluid": "^8.1.0", "@microsoft/api-extractor": "^7.55.1", + "@types/chai": "^5.2.3", + "@types/mocha": "^10.0.10", "@types/msgpack-lite": "^0.1.12", "@types/node": "^22.19.1", + "c8": "^10.1.3", + "chai": "^6.2.1", "copyfiles": "^2.4.1", "eslint": "~8.57.0", + "mocha": "^11.7.5", "rimraf": "^6.1.2" } } diff --git a/build-tools/packages/bundle-size-tools/src/ADO/AdoArtifactFileProvider.ts b/build-tools/packages/bundle-size-tools/src/ADO/AdoArtifactFileProvider.ts index e05b53bf1396..a626eaac7423 100644 --- a/build-tools/packages/bundle-size-tools/src/ADO/AdoArtifactFileProvider.ts +++ b/build-tools/packages/bundle-size-tools/src/ADO/AdoArtifactFileProvider.ts @@ -5,40 +5,36 @@ import { strict as assert } from "assert"; import type { WebApi } from "azure-devops-node-api"; -import type JSZip from "jszip"; import type { StatsCompilation } from "webpack"; import type { BundleBuddyConfig } from "../BundleBuddyTypes"; -import { decompressStatsFile, unzipStream } from "../utilities"; +import { type UnzippedContents, decompressStatsFile, unzipStream } from "../utilities"; import { type BundleFileData, getBundleFilePathsFromFolder, } from "./getBundleFilePathsFromFolder"; /** - * Gets a list of all paths relevant to bundle buddy from the zip archive - * @param jsZip - A zip file that has been processed with the jszip library + * Gets a list of all paths relevant to bundle buddy from the unzipped archive + * @param files - The unzipped archive contents as a Map of paths to Buffers */ -export function getBundlePathsFromZipObject(jsZip: JSZip): BundleFileData[] { - const relativePaths: string[] = []; - jsZip.forEach((path) => { - relativePaths.push(path); - }); - +export function getBundlePathsFromZipObject(files: UnzippedContents): BundleFileData[] { + const relativePaths: string[] = [...files.keys()]; return getBundleFilePathsFromFolder(relativePaths); } /** - * Downloads an Azure Devops artifacts and parses it with the jszip library. + * Downloads an Azure Devops artifacts and unzips it. * @param adoConnection - A connection to the ADO api. * @param buildNumber - The ADO build number that contains the artifact we wish to fetch + * @returns A Map of file paths to their contents as Buffers */ export async function getZipObjectFromArtifact( adoConnection: WebApi, projectName: string, buildNumber: number, bundleAnalysisArtifactName: string, -): Promise { +): Promise { const buildApi = await adoConnection.getBuildApi(); // IMPORTANT @@ -57,10 +53,10 @@ export async function getZipObjectFromArtifact( // Undo hack from above buildApi.createAcceptHeader = originalCreateAcceptHeader; - // We want our relative paths to be clean, so navigating JsZip into the top level folder - const result = (await unzipStream(artifactStream)).folder(bundleAnalysisArtifactName); + // We want our relative paths to be clean, so filter to files within the artifact folder + const result = await unzipStream(artifactStream, bundleAnalysisArtifactName); assert( - result, + result.size > 0, `getZipObjectFromArtifact could not find the folder ${bundleAnalysisArtifactName}`, ); @@ -68,33 +64,31 @@ export async function getZipObjectFromArtifact( } /** - * Retrieves a decompressed stats file from a jszip object - * @param jsZip - A zip file that has been processed with the jszip library + * Retrieves a decompressed stats file from an unzipped archive + * @param files - The unzipped archive contents as a Map of paths to Buffers * @param relativePath - The relative path to the file that will be retrieved */ export async function getStatsFileFromZip( - jsZip: JSZip, + files: UnzippedContents, relativePath: string, ): Promise { - const jsZipObject = jsZip.file(relativePath); - assert(jsZipObject, `getStatsFileFromZip could not find file ${relativePath}`); + const buffer = files.get(relativePath); + assert(buffer, `getStatsFileFromZip could not find file ${relativePath}`); - const buffer = await jsZipObject.async("nodebuffer"); return decompressStatsFile(buffer); } /** - * Retrieves and parses a bundle buddy config file from a jszip object - * @param jsZip - A zip file that has been processed with the jszip library + * Retrieves and parses a bundle buddy config file from an unzipped archive + * @param files - The unzipped archive contents as a Map of paths to Buffers * @param relativePath - The relative path to the file that will be retrieved */ export async function getBundleBuddyConfigFileFromZip( - jsZip: JSZip, + files: UnzippedContents, relativePath: string, ): Promise { - const jsZipObject = jsZip.file(relativePath); - assert(jsZipObject, `getBundleBuddyConfigFileFromZip could not find file ${relativePath}`); + const buffer = files.get(relativePath); + assert(buffer, `getBundleBuddyConfigFileFromZip could not find file ${relativePath}`); - const buffer = await jsZipObject.async("nodebuffer"); return JSON.parse(buffer.toString()); } diff --git a/build-tools/packages/bundle-size-tools/src/ADO/AdoSizeComparator.ts b/build-tools/packages/bundle-size-tools/src/ADO/AdoSizeComparator.ts index 18227f1d6e82..fab879ad6ddd 100644 --- a/build-tools/packages/bundle-size-tools/src/ADO/AdoSizeComparator.ts +++ b/build-tools/packages/bundle-size-tools/src/ADO/AdoSizeComparator.ts @@ -6,10 +6,10 @@ import { join } from "path"; import type { WebApi } from "azure-devops-node-api"; import { BuildResult, BuildStatus } from "azure-devops-node-api/interfaces/BuildInterfaces"; -import type JSZip from "jszip"; import type { BundleComparison, BundleComparisonResult } from "../BundleBuddyTypes"; import { compareBundles } from "../compareBundles"; +import type { UnzippedContents } from "../utilities"; import { getBaselineCommit, getBuilds, getPriorCommit } from "../utilities"; import { getBundlePathsFromZipObject, @@ -216,7 +216,9 @@ export class ADOSizeComparator { } } - private async createComparisonFromZip(baselineZip: JSZip): Promise { + private async createComparisonFromZip( + baselineZip: UnzippedContents, + ): Promise { const baselineZipBundlePaths = getBundlePathsFromZipObject(baselineZip); const prBundleFileSystemPaths = await getBundlePathsFromFileSystem(this.localReportPath); diff --git a/build-tools/packages/bundle-size-tools/src/index.ts b/build-tools/packages/bundle-size-tools/src/index.ts index 05b1cee9127c..b29a527ac7f6 100644 --- a/build-tools/packages/bundle-size-tools/src/index.ts +++ b/build-tools/packages/bundle-size-tools/src/index.ts @@ -67,5 +67,6 @@ export { getChunkParsedSize, getLastCommitHashFromPR, getPriorCommit, + UnzippedContents, unzipStream, } from "./utilities"; diff --git a/build-tools/packages/bundle-size-tools/src/test/tsconfig.json b/build-tools/packages/bundle-size-tools/src/test/tsconfig.json new file mode 100644 index 000000000000..209df0288fb4 --- /dev/null +++ b/build-tools/packages/bundle-size-tools/src/test/tsconfig.json @@ -0,0 +1,22 @@ +{ + "extends": "@fluidframework/build-common/ts-common-config.json", + "compilerOptions": { + "declaration": true, + "importHelpers": true, + "declarationMap": false, + "rootDir": "./", + "outDir": "../../lib/test", + "types": ["node", "mocha"], + "typeRoots": [ + "../../../../node_modules/@types", + "../../../../node_modules/.pnpm/node_modules/@types", + ], + "skipLibCheck": true, + }, + "include": ["./**/*"], + "references": [ + { + "path": "../..", + }, + ], +} diff --git a/build-tools/packages/bundle-size-tools/src/test/unzipStream.test.ts b/build-tools/packages/bundle-size-tools/src/test/unzipStream.test.ts new file mode 100644 index 000000000000..4e7a23e66fa8 --- /dev/null +++ b/build-tools/packages/bundle-size-tools/src/test/unzipStream.test.ts @@ -0,0 +1,473 @@ +/*! + * Copyright (c) Microsoft Corporation and contributors. All rights reserved. + * Licensed under the MIT License. + */ + +import { assert } from "chai"; +import { zipSync } from "fflate"; +import { Readable } from "stream"; +import { unzipStream } from "../../dist/utilities/unzipStream"; + +/** + * Helper to create a readable stream from a buffer + */ +function bufferToStream(buffer: Buffer): NodeJS.ReadableStream { + const stream = new Readable(); + stream.push(buffer); + stream.push(null); + return stream; +} + +/** + * Helper to create a zip file with test content + */ +function createZipFile(files: Record): Buffer { + const zipContents: Record = {}; + for (const [path, content] of Object.entries(files)) { + if (typeof content === "string") { + zipContents[path] = new TextEncoder().encode(content); + } else { + zipContents[path] = content; + } + } + const zipped = zipSync(zipContents); + return Buffer.from(zipped); +} + +describe("unzipStream", () => { + describe("basic extraction", () => { + it("should extract a simple zip file with one file", async () => { + const testContent = "Hello, World!"; + const zipBuffer = createZipFile({ "test.txt": testContent }); + const stream = bufferToStream(zipBuffer); + + const result = await unzipStream(stream); + + assert.strictEqual(result.size, 1, "Should have exactly one file"); + assert.isTrue(result.has("test.txt"), "Should contain test.txt"); + const content = result.get("test.txt"); + assert.isDefined(content, "File content should be defined"); + assert.strictEqual( + content!.toString("utf-8"), + testContent, + "Content should match original", + ); + }); + + it("should extract a zip file with multiple files", async () => { + const files = { + "file1.txt": "Content 1", + "file2.txt": "Content 2", + "file3.txt": "Content 3", + }; + const zipBuffer = createZipFile(files); + const stream = bufferToStream(zipBuffer); + + const result = await unzipStream(stream); + + assert.strictEqual(result.size, 3, "Should have three files"); + for (const [path, expectedContent] of Object.entries(files)) { + assert.isTrue(result.has(path), `Should contain ${path}`); + const content = result.get(path); + assert.isDefined(content, `Content of ${path} should be defined`); + assert.strictEqual( + content!.toString("utf-8"), + expectedContent, + `Content of ${path} should match`, + ); + } + }); + + it("should extract nested directory structures", async () => { + const files = { + "dir1/file1.txt": "File in dir1", + "dir1/dir2/file2.txt": "File in nested dir", + "dir3/file3.txt": "File in dir3", + "root.txt": "File at root", + }; + const zipBuffer = createZipFile(files); + const stream = bufferToStream(zipBuffer); + + const result = await unzipStream(stream); + + assert.strictEqual(result.size, 4, "Should have four files"); + for (const [path, expectedContent] of Object.entries(files)) { + assert.isTrue(result.has(path), `Should contain ${path}`); + const content = result.get(path); + assert.strictEqual( + content!.toString("utf-8"), + expectedContent, + `Content of ${path} should match`, + ); + } + }); + }); + + describe("filtering with baseFolder", () => { + it("should filter files by baseFolder prefix", async () => { + const files = { + "folder/file1.txt": "Content 1", + "folder/file2.txt": "Content 2", + "other/file3.txt": "Content 3", + }; + const zipBuffer = createZipFile(files); + const stream = bufferToStream(zipBuffer); + + const result = await unzipStream(stream, "folder"); + + assert.strictEqual(result.size, 2, "Should have two files from folder"); + assert.isTrue(result.has("file1.txt"), "Should contain file1.txt without folder prefix"); + assert.isTrue(result.has("file2.txt"), "Should contain file2.txt without folder prefix"); + assert.isFalse( + result.has("file3.txt"), + "Should not contain file3.txt from other folder", + ); + }); + + it("should strip baseFolder prefix from returned paths", async () => { + const files = { + "my-artifact/stats.json": '{"webpack": "stats"}', + "my-artifact/bundle.js": "console.log('hello');", + }; + const zipBuffer = createZipFile(files); + const stream = bufferToStream(zipBuffer); + + const result = await unzipStream(stream, "my-artifact"); + + assert.strictEqual(result.size, 2, "Should have two files"); + assert.isTrue(result.has("stats.json"), "Should have stats.json without prefix"); + assert.isTrue(result.has("bundle.js"), "Should have bundle.js without prefix"); + assert.isFalse(result.has("my-artifact/stats.json"), "Should not have prefixed path"); + }); + + it("should handle nested folders with baseFolder", async () => { + const files = { + "base/sub1/file1.txt": "Content 1", + "base/sub2/file2.txt": "Content 2", + "other/file3.txt": "Content 3", + }; + const zipBuffer = createZipFile(files); + const stream = bufferToStream(zipBuffer); + + const result = await unzipStream(stream, "base"); + + assert.strictEqual(result.size, 2, "Should have two files from base folder"); + assert.isTrue(result.has("sub1/file1.txt"), "Should preserve nested structure"); + assert.isTrue(result.has("sub2/file2.txt"), "Should preserve nested structure"); + }); + + it("should return empty map when baseFolder doesn't exist", async () => { + const files = { + "folder1/file1.txt": "Content 1", + "folder2/file2.txt": "Content 2", + }; + const zipBuffer = createZipFile(files); + const stream = bufferToStream(zipBuffer); + + const result = await unzipStream(stream, "nonexistent"); + + assert.strictEqual(result.size, 0, "Should have no files"); + }); + }); + + describe("lazy decompression", () => { + it("should support Map interface methods", async () => { + const files = { + "file1.txt": "Content 1", + "file2.txt": "Content 2", + "file3.txt": "Content 3", + }; + const zipBuffer = createZipFile(files); + const stream = bufferToStream(zipBuffer); + + const result = await unzipStream(stream); + + // Test keys() + const keys = Array.from(result.keys()).sort(); + assert.deepStrictEqual( + keys, + ["file1.txt", "file2.txt", "file3.txt"], + "keys() should return all file names", + ); + + // Test values() + const values = Array.from(result.values()); + assert.strictEqual(values.length, 3, "values() should return all file contents"); + for (const value of values) { + assert.instanceOf(value, Buffer, "Each value should be a Buffer"); + } + + // Test entries() + const entries = Array.from(result.entries()); + assert.strictEqual(entries.length, 3, "entries() should return all key-value pairs"); + for (const [key, value] of entries) { + assert.isString(key, "Entry key should be a string"); + assert.instanceOf(value, Buffer, "Entry value should be a Buffer"); + } + + // Test has() + assert.isTrue(result.has("file1.txt"), "has() should return true for existing file"); + assert.isFalse( + result.has("nonexistent.txt"), + "has() should return false for non-existing file", + ); + + // Test get() + assert.isDefined( + result.get("file1.txt"), + "get() should return buffer for existing file", + ); + assert.isUndefined( + result.get("nonexistent.txt"), + "get() should return undefined for non-existing file", + ); + + // Test size + assert.strictEqual(result.size, 3, "size should return correct count"); + }); + + it("should cache decompressed files on subsequent access", async () => { + const testContent = "Test content for caching"; + const zipBuffer = createZipFile({ "test.txt": testContent }); + const stream = bufferToStream(zipBuffer); + + const result = await unzipStream(stream); + + // First access + const buffer1 = result.get("test.txt"); + // Second access - should return the same buffer instance if cached + const buffer2 = result.get("test.txt"); + + assert.strictEqual(buffer1, buffer2, "Should return the same buffer instance (cached)"); + }); + + it("should handle iteration correctly", async () => { + const files = { + "a.txt": "A", + "b.txt": "B", + "c.txt": "C", + }; + const zipBuffer = createZipFile(files); + const stream = bufferToStream(zipBuffer); + + const result = await unzipStream(stream); + + // Test for...of iteration + const collected: Array<[string, Buffer]> = []; + for (const entry of result) { + collected.push(entry); + } + + assert.strictEqual(collected.length, 3, "Should iterate over all entries"); + for (const [key, value] of collected) { + assert.isTrue(result.has(key), `Iterated key ${key} should exist in map`); + assert.instanceOf(value, Buffer, "Iterated value should be a Buffer"); + } + }); + + it("should decompress files only when accessed", async () => { + const files = { + "file1.txt": "A".repeat(1000), + "file2.txt": "B".repeat(1000), + "file3.txt": "C".repeat(1000), + }; + const zipBuffer = createZipFile(files); + const stream = bufferToStream(zipBuffer); + + const result = await unzipStream(stream); + + // Map should know about all files without decompressing them + assert.strictEqual(result.size, 3, "Should know size without decompression"); + assert.isTrue(result.has("file1.txt"), "Should know file1 exists"); + assert.isTrue(result.has("file2.txt"), "Should know file2 exists"); + assert.isTrue(result.has("file3.txt"), "Should know file3 exists"); + + // Access only one file + const content1 = result.get("file1.txt"); + assert.isDefined(content1, "Should decompress when accessed"); + assert.strictEqual(content1!.toString("utf-8"), "A".repeat(1000)); + + // Other files should still be accessible + const content2 = result.get("file2.txt"); + assert.strictEqual(content2!.toString("utf-8"), "B".repeat(1000)); + }); + + it("should handle forEach iteration", async () => { + const files = { + "file1.txt": "Content 1", + "file2.txt": "Content 2", + }; + const zipBuffer = createZipFile(files); + const stream = bufferToStream(zipBuffer); + + const result = await unzipStream(stream); + + const collected: Array<[Buffer, string, Map]> = []; + result.forEach((value, key, map) => { + collected.push([value, key, map]); + }); + + assert.strictEqual(collected.length, 2, "forEach should visit all entries"); + for (const [value, key, map] of collected) { + assert.instanceOf(value, Buffer, "Value should be a Buffer"); + assert.isString(key, "Key should be a string"); + assert.strictEqual(map, result, "Map should be the result object"); + } + }); + }); + + describe("binary content handling", () => { + it("should correctly handle binary content", async () => { + const binaryContent = Buffer.from([0x00, 0x01, 0xff, 0x7f, 0x80, 0xab, 0xcd, 0xef]); + const zipBuffer = createZipFile({ "binary.bin": binaryContent }); + const stream = bufferToStream(zipBuffer); + + const result = await unzipStream(stream); + + assert.strictEqual(result.size, 1, "Should have one file"); + const extractedContent = result.get("binary.bin"); + assert.isDefined(extractedContent, "Binary content should be defined"); + assert.deepStrictEqual( + extractedContent, + binaryContent, + "Binary content should match exactly", + ); + }); + + it("should handle large file content", async () => { + const largeContent = Buffer.alloc(100 * 1024, "x"); // 100KB + const zipBuffer = createZipFile({ "large.txt": largeContent }); + const stream = bufferToStream(zipBuffer); + + const result = await unzipStream(stream); + + assert.strictEqual(result.size, 1, "Should have one file"); + const extractedContent = result.get("large.txt"); + assert.isDefined(extractedContent, "Large content should be defined"); + assert.strictEqual( + extractedContent!.length, + largeContent.length, + "Large content should have correct length", + ); + }); + }); + + describe("edge cases", () => { + it("should handle empty zip file", async () => { + const emptyZip = zipSync({}); + const zipBuffer = Buffer.from(emptyZip); + const stream = bufferToStream(zipBuffer); + + const result = await unzipStream(stream); + + assert.strictEqual(result.size, 0, "Should have zero files"); + }); + + it("should handle files with special characters in names", async () => { + const files = { + "file with spaces.txt": "Content 1", + "file-with-dashes.txt": "Content 2", + "file_with_underscores.txt": "Content 3", + "file.multiple.dots.txt": "Content 4", + }; + const zipBuffer = createZipFile(files); + const stream = bufferToStream(zipBuffer); + + const result = await unzipStream(stream); + + assert.strictEqual(result.size, 4, "Should have four files"); + for (const [path, expectedContent] of Object.entries(files)) { + assert.isTrue(result.has(path), `Should contain ${path}`); + assert.strictEqual(result.get(path)!.toString("utf-8"), expectedContent); + } + }); + + it("should handle empty files", async () => { + const files = { + "empty.txt": "", + "nonempty.txt": "content", + }; + const zipBuffer = createZipFile(files); + const stream = bufferToStream(zipBuffer); + + const result = await unzipStream(stream); + + assert.strictEqual(result.size, 2, "Should have two files"); + assert.strictEqual( + result.get("empty.txt")!.length, + 0, + "Empty file should have zero length", + ); + assert.isTrue( + result.get("nonempty.txt")!.length > 0, + "Non-empty file should have content", + ); + }); + }); + + describe("error handling", () => { + it("should reject with error for invalid zip data", async () => { + const invalidZip = Buffer.from("This is not a valid zip file"); + const stream = bufferToStream(invalidZip); + + try { + await unzipStream(stream); + assert.fail("Should have thrown an error"); + } catch (error) { + assert.isDefined(error, "Should throw an error for invalid zip data"); + } + }); + }); + + describe("real-world scenarios", () => { + it("should handle webpack stats file extraction scenario", async () => { + const statsContent = JSON.stringify({ + chunks: [{ id: 1, files: ["bundle.js"] }], + assets: [{ name: "bundle.js", size: 12345 }], + }); + const files = { + "bundleAnalysis/stats.json": statsContent, + "bundleAnalysis/bundle.js": "console.log('test');", + "other/file.txt": "ignored", + }; + const zipBuffer = createZipFile(files); + const stream = bufferToStream(zipBuffer); + + const result = await unzipStream(stream, "bundleAnalysis"); + + assert.strictEqual(result.size, 2, "Should extract only bundleAnalysis folder"); + assert.isTrue(result.has("stats.json"), "Should have stats.json"); + const extractedStats = JSON.parse(result.get("stats.json")!.toString("utf-8")); + assert.strictEqual( + extractedStats.chunks[0].id, + 1, + "Should correctly parse extracted JSON", + ); + }); + + it("should iterate files for bundle buddy path extraction", async () => { + const files = { + "artifact/bundle1/stats.json": '{"name": "bundle1"}', + "artifact/bundle2/stats.json": '{"name": "bundle2"}', + "artifact/bundle3/stats.json": '{"name": "bundle3"}', + }; + const zipBuffer = createZipFile(files); + const stream = bufferToStream(zipBuffer); + + const result = await unzipStream(stream, "artifact"); + + // Simulate how getBundlePathsFromZipObject uses the result + const relativePaths: string[] = [...result.keys()]; + + assert.strictEqual(relativePaths.length, 3, "Should get all relative paths"); + assert.isTrue( + relativePaths.every((path) => path.includes("stats.json")), + "All paths should be stats files", + ); + assert.isTrue( + relativePaths.every((path) => !path.startsWith("artifact/")), + "Paths should not include the artifact prefix", + ); + }); + }); +}); diff --git a/build-tools/packages/bundle-size-tools/src/utilities/index.ts b/build-tools/packages/bundle-size-tools/src/utilities/index.ts index 96dc1dd7b95e..0c5d5bf5fdd4 100644 --- a/build-tools/packages/bundle-size-tools/src/utilities/index.ts +++ b/build-tools/packages/bundle-size-tools/src/utilities/index.ts @@ -14,4 +14,4 @@ export { export { getChunkParsedSize } from "./getChunkParsedSize"; export { getLastCommitHashFromPR } from "./getLastCommitHashFromPR"; export { getBaselineCommit, getPriorCommit } from "./gitCommands"; -export { unzipStream } from "./unzipStream"; +export { UnzippedContents, unzipStream } from "./unzipStream"; diff --git a/build-tools/packages/bundle-size-tools/src/utilities/unzipStream.ts b/build-tools/packages/bundle-size-tools/src/utilities/unzipStream.ts index f9ea2d07202b..277459e85f92 100644 --- a/build-tools/packages/bundle-size-tools/src/utilities/unzipStream.ts +++ b/build-tools/packages/bundle-size-tools/src/utilities/unzipStream.ts @@ -3,7 +3,80 @@ * Licensed under the MIT License. */ -import * as JSZip from "jszip"; +import { type Unzipped, unzip } from "fflate"; + +/** + * Lazy wrapper around unzipped archive that decompresses files on access. + */ +class LazyUnzippedContents extends Map { + private readonly unzipped: Unzipped; + private readonly prefix: string; + private readonly fileKeys: Set; + + constructor(unzipped: Unzipped, prefix: string) { + super(); + this.unzipped = unzipped; + this.prefix = prefix; + this.fileKeys = new Set(); + + // Populate available file keys + for (const path of Object.keys(unzipped)) { + if (prefix && !path.startsWith(prefix)) continue; + const relativePath = prefix ? path.slice(prefix.length) : path; + if (relativePath) { + this.fileKeys.add(relativePath); + } + } + } + + override get(key: string): Buffer | undefined { + // Check if the file exists + if (!this.fileKeys.has(key)) { + return undefined; + } + + // Check if already decompressed + const cached = super.get(key); + if (cached !== undefined) { + return cached; + } + + // Decompress on first access + const fullPath = this.prefix ? `${this.prefix}${key}` : key; + const buffer = Buffer.from(this.unzipped[fullPath]); + super.set(key, buffer); + return buffer; + } + + override has(key: string): boolean { + return this.fileKeys.has(key); + } + + override keys(): IterableIterator { + return this.fileKeys.keys(); + } + + override *entries(): IterableIterator<[string, Buffer]> { + for (const key of this.fileKeys) { + yield [key, this.get(key)!]; + } + } + + override *values(): IterableIterator { + for (const key of this.fileKeys) { + yield this.get(key)!; + } + } + + override get size(): number { + return this.fileKeys.size; + } +} + +/** + * Type alias for unzipped archive contents - a Map of file paths to their contents. + */ +export type UnzippedContents = Map; function readStreamAsBuffer(stream: NodeJS.ReadableStream): Promise { return new Promise((resolve, reject) => { @@ -21,6 +94,28 @@ function readStreamAsBuffer(stream: NodeJS.ReadableStream): Promise { }); } -export async function unzipStream(stream: NodeJS.ReadableStream) { - return JSZip.loadAsync(await readStreamAsBuffer(stream)); +/** + * Unzips a stream and returns a Map of file paths to their contents. + * Files are decompressed lazily on first access to reduce memory usage. + * @param stream - The stream containing zip data + * @param baseFolder - Optional folder prefix to filter by and strip from paths + * @returns A Map where keys are relative file paths and values are file contents as Buffers + */ +export async function unzipStream( + stream: NodeJS.ReadableStream, + baseFolder?: string, +): Promise { + const buffer = await readStreamAsBuffer(stream); + + return new Promise((resolve, reject) => { + unzip(new Uint8Array(buffer), (err, unzipped) => { + if (err) { + reject(err); + return; + } + + const prefix = baseFolder ? `${baseFolder}/` : ""; + resolve(new LazyUnzippedContents(unzipped, prefix)); + }); + }); } diff --git a/build-tools/pnpm-lock.yaml b/build-tools/pnpm-lock.yaml index eb96638e8204..6c7910d26693 100644 --- a/build-tools/pnpm-lock.yaml +++ b/build-tools/pnpm-lock.yaml @@ -184,9 +184,6 @@ importers: jssm: specifier: ^5.104.2 version: 5.104.2 - jszip: - specifier: ^3.10.1 - version: 3.10.1 latest-version: specifier: ^9.0.0 version: 9.0.0 @@ -643,9 +640,6 @@ importers: fflate: specifier: ^0.8.2 version: 0.8.2 - jszip: - specifier: ^3.10.1 - version: 3.10.1 msgpack-lite: specifier: ^0.1.26 version: 0.1.26 @@ -671,18 +665,33 @@ importers: '@microsoft/api-extractor': specifier: ^7.55.1 version: 7.55.1(@types/node@22.19.1) + '@types/chai': + specifier: ^5.2.3 + version: 5.2.3 + '@types/mocha': + specifier: ^10.0.10 + version: 10.0.10 '@types/msgpack-lite': specifier: ^0.1.12 version: 0.1.12 '@types/node': specifier: ^22.19.1 version: 22.19.1 + c8: + specifier: ^10.1.3 + version: 10.1.3 + chai: + specifier: ^6.2.1 + version: 6.2.1 copyfiles: specifier: ^2.4.1 version: 2.4.1 eslint: specifier: ~8.57.0 version: 8.57.1 + mocha: + specifier: ^11.7.5 + version: 11.7.5 rimraf: specifier: ^6.1.2 version: 6.1.2