From fec714484f43bf9ee3640e6c1423c6d320b77544 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 25 Nov 2025 19:56:12 +0000 Subject: [PATCH 01/10] Initial plan From 81cd47889549f5b4e60acf92ef6aefb080f6849a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 25 Nov 2025 20:10:42 +0000 Subject: [PATCH 02/10] Replace pako with fflate in bundle-size-tools Co-authored-by: tylerbutler <19589+tylerbutler@users.noreply.github.com> --- .../packages/bundle-size-tools/package.json | 3 +-- .../src/utilities/decompressStatsFile.ts | 6 +++--- build-tools/pnpm-lock.yaml | 14 +++----------- 3 files changed, 7 insertions(+), 16 deletions(-) diff --git a/build-tools/packages/bundle-size-tools/package.json b/build-tools/packages/bundle-size-tools/package.json index c6171856e28e..dafab8a85654 100644 --- a/build-tools/packages/bundle-size-tools/package.json +++ b/build-tools/packages/bundle-size-tools/package.json @@ -41,9 +41,9 @@ }, "dependencies": { "azure-devops-node-api": "^11.2.0", + "fflate": "^0.8.2", "jszip": "^3.10.1", "msgpack-lite": "^0.1.26", - "pako": "^2.1.0", "typescript": "~5.4.5", "webpack": "^5.95.0" }, @@ -55,7 +55,6 @@ "@microsoft/api-extractor": "^7.52.11", "@types/msgpack-lite": "^0.1.11", "@types/node": "^22.8.0", - "@types/pako": "^2.0.3", "copyfiles": "^2.4.1", "eslint": "~8.57.0", "rimraf": "^4.4.1" diff --git a/build-tools/packages/bundle-size-tools/src/utilities/decompressStatsFile.ts b/build-tools/packages/bundle-size-tools/src/utilities/decompressStatsFile.ts index ad8f9b885292..9a0c9462dc5a 100644 --- a/build-tools/packages/bundle-size-tools/src/utilities/decompressStatsFile.ts +++ b/build-tools/packages/bundle-size-tools/src/utilities/decompressStatsFile.ts @@ -3,8 +3,8 @@ * Licensed under the MIT License. */ +import { gunzipSync } from "fflate"; import { decode } from "msgpack-lite"; -import { inflate } from "pako"; import type { StatsCompilation } from "webpack"; /** @@ -12,8 +12,8 @@ import type { StatsCompilation } from "webpack"; * in a compressed file path and outputs the webpack stats object. */ export function decompressStatsFile(buffer: Buffer): StatsCompilation { - // Inflate the gzipped data to get the mspack data - const mspackData = inflate(buffer); + // Decompress the gzipped data to get the msgpack data + const mspackData = gunzipSync(buffer); return decode(mspackData); } diff --git a/build-tools/pnpm-lock.yaml b/build-tools/pnpm-lock.yaml index 6dac7ddac31d..25bbd188a557 100644 --- a/build-tools/pnpm-lock.yaml +++ b/build-tools/pnpm-lock.yaml @@ -641,15 +641,15 @@ importers: azure-devops-node-api: specifier: ^11.2.0 version: 11.2.0 + 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 - pako: - specifier: ^2.1.0 - version: 2.1.0 typescript: specifier: ~5.4.5 version: 5.4.5 @@ -678,9 +678,6 @@ importers: '@types/node': specifier: ^22.8.0 version: 22.8.0 - '@types/pako': - specifier: ^2.0.3 - version: 2.0.3 copyfiles: specifier: ^2.4.1 version: 2.4.1 @@ -1836,9 +1833,6 @@ packages: '@types/normalize-package-data@2.4.1': resolution: {integrity: sha512-Gj7cI7z+98M282Tqmp2K5EIsoouUEzbBJhQQzDE3jSIRk6r9gsz0oUokqIUR4u1R3dMHo0pDHM7sNOHyhulypw==} - '@types/pako@2.0.3': - resolution: {integrity: sha512-bq0hMV9opAcrmE0Byyo0fY3Ew4tgOevJmQ9grUhpXQhYfyLJ1Kqg3P33JT5fdbT2AjeAjR51zqqVjAL/HMkx7Q==} - '@types/prettier@2.7.3': resolution: {integrity: sha512-+68kP9yzs4LMp7VNh8gdzMSPZFL44MLGqiHWvttYJe+6qnuVr4Ek9wSBQoveqY/r+LwjCcU29kNVkidwim+kYA==} @@ -8575,8 +8569,6 @@ snapshots: '@types/normalize-package-data@2.4.1': {} - '@types/pako@2.0.3': {} - '@types/prettier@2.7.3': {} '@types/prompts@2.4.9': From deaa741dd395ca078598cbdb105ab9c67c475dfe Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 25 Nov 2025 20:38:42 +0000 Subject: [PATCH 03/10] Add dependency reduction plan document Co-authored-by: tylerbutler <19589+tylerbutler@users.noreply.github.com> --- build-tools/plans/README.md | 9 + build-tools/plans/dependency-reduction.md | 240 ++++++++++++++++++++++ 2 files changed, 249 insertions(+) create mode 100644 build-tools/plans/README.md create mode 100644 build-tools/plans/dependency-reduction.md diff --git a/build-tools/plans/README.md b/build-tools/plans/README.md new file mode 100644 index 000000000000..847b9df69ef6 --- /dev/null +++ b/build-tools/plans/README.md @@ -0,0 +1,9 @@ +# Plans + +This folder contains planning documents for build-tools development and maintenance. + +## Documents + +| Document | Description | +|----------|-------------| +| [dependency-reduction.md](./dependency-reduction.md) | Plan for reducing dependencies in the build-tools workspace | diff --git a/build-tools/plans/dependency-reduction.md b/build-tools/plans/dependency-reduction.md new file mode 100644 index 000000000000..7785b862a62b --- /dev/null +++ b/build-tools/plans/dependency-reduction.md @@ -0,0 +1,240 @@ +# Dependency Reduction Plan for build-tools Workspace + +This document outlines opportunities to reduce dependencies in the build-tools workspace, along with risk assessments and suggested approaches to reduce risk before implementation. + +## Current State + +The build-tools workspace has accumulated multiple dependencies serving similar purposes, particularly around: +- Globbing/path matching +- Compression +- Command execution +- File system operations + +## Test Coverage Assessment + +Understanding test coverage is critical for assessing risk: + +| Package | Test Files | Coverage Level | Risk for Changes | +|---------|------------|----------------|------------------| +| version-tools | 5 | Good | Low | +| build-infrastructure | 6 | Good | Low | +| build-cli | 20+ | Moderate | Medium | +| build-tools | 2 | Low | High | +| bundle-size-tools | 0 | None | Very High | + +## Completed Changes + +### ✅ Compression Library Consolidation (Partial) + +**Status:** Completed + +**Change:** Replaced `pako` with `fflate` in bundle-size-tools + +**Impact:** +- Removed `pako` and `@types/pako` dependencies +- `fflate` was already in use in build-cli +- Lockfile reduced by 8 lines + +--- + +## Planned Opportunities + +### 1. Globbing Library Consolidation + +**Priority:** Medium +**Risk Level:** Medium-High +**Estimated Impact:** Remove 3-4 direct dependencies + +#### Current State + +Six libraries serve similar globbing/matching purposes: + +| Package | Version | Used In | Purpose | +|---------|---------|---------|---------| +| `glob` | 7.2.3 | build-tools | `globFn()` wrapper in taskUtils.ts | +| `globby` | 11.1.0 | build-cli, build-infrastructure, build-tools | File matching with gitignore support | +| `multimatch` | 5.0.0 | build-tools | Biome config filtering | +| `micromatch` | 4.0.8 | build-infrastructure | Package filtering | +| `picomatch` | 2.3.1 | build-tools | Pattern scanning in DepCruiseTask | +| `minimatch` | 7.4.6 | build-cli | Path matching in repoConfig | + +#### Recommended Consolidation + +Consolidate on `globby` (which uses `fast-glob` with `micromatch` internally): + +1. **Replace `glob` with `globby`** in build-tools/taskUtils.ts +2. **Replace `minimatch` with `micromatch`** in build-cli +3. **Replace `multimatch` with `micromatch`** in build-tools + +#### Key Migration Considerations + +Option name differences between `glob` and `globby`: + +| glob option | globby/fast-glob equivalent | +|-------------|----------------------------| +| `nodir: true` | `onlyFiles: true` (default) | +| `follow: true` | `followSymbolicLinks: true` | +| `ignore: "pattern"` | `ignore: ["pattern"]` (array required) | + +#### Risk Reduction Steps + +1. **Add integration tests for glob-dependent functionality:** + - Create test cases in `build-tools/src/test/` covering: + - `CopyfilesTask` glob behavior with various options + - `TypeValidationTask` output file discovery + - `GoodFence` input file enumeration + - `DepCruiseTask` pattern matching + - Test edge cases: dot files, symlinks, nested directories + +2. **Create a compatibility wrapper:** + ```typescript + // Temporary adapter that accepts old glob options and converts to globby + export function globFn(pattern: string, options: GlobCompatOptions = {}): Promise { + return globby(pattern, { + onlyFiles: options.nodir ?? true, + followSymbolicLinks: options.follow ?? false, + ignore: options.ignore ? [options.ignore] : [], + cwd: options.cwd, + absolute: options.absolute, + dot: options.dot, + }); + } + ``` + +3. **Migrate incrementally by file:** + - Start with files that have test coverage + - Manually verify behavior for untested files + +--- + +### 2. Additional Compression Consolidation + +**Priority:** Low +**Risk Level:** Low-Medium +**Estimated Impact:** Remove 1 dependency + +#### Current State + +| Package | Used In | Purpose | +|---------|---------|---------| +| `fflate` | build-cli, bundle-size-tools | Gzip decompression | +| `jszip` | build-cli, bundle-size-tools | ZIP file handling | + +#### Recommendation + +Consider replacing `jszip` with `fflate` for ZIP handling: +- `fflate` has ZIP support via `unzipSync`/`zipSync` +- However, `jszip` provides streaming and more features + +#### Risk Reduction Steps + +1. Audit all `jszip` usage patterns +2. Verify `fflate` can handle all use cases +3. If not, keep both (different purposes) + +--- + +### 3. Command Execution (execa) + +**Priority:** Deferred +**Risk Level:** High +**Estimated Impact:** Minimal (transitive dependency reduction only) + +#### Current State + +`execa` v5.1.1 is used in 11+ files across: +- build-infrastructure (2 files) +- build-cli (9+ files) + +#### Usage Patterns + +```typescript +// Async command execution +await execa('npm', ['publish', ...args], { cwd, stdio }) + +// Sync command execution +execa.sync('git', ['rev-parse', '--show-toplevel'], { cwd, stdio }) +``` + +#### Recommendation + +**Keep `execa` for now.** Reasons: +- Cross-platform compatibility is essential +- Extensively tested in the npm ecosystem +- Native `child_process` alternatives require significant boilerplate +- Risk outweighs minimal benefit + +#### Future Consideration + +When upgrading to execa v6+, evaluate `tinyexec` as a lighter alternative for simple use cases. + +--- + +### 4. File System Utilities (fs-extra) + +**Priority:** Low +**Risk Level:** Low +**Estimated Impact:** Minimal + +#### Current State + +`fs-extra` used in 7+ files for: +- `readJsonSync`, `writeJson`, `writeJsonSync` +- `mkdirpSync` +- `copySync` + +#### Recommendation + +**Keep `fs-extra`.** Reasons: +- Well-maintained with minimal footprint +- Node.js alternatives require more code +- Not a significant source of bloat + +#### Gradual Migration Path (Optional) + +If desired, these can be replaced with native Node.js equivalents: + +| fs-extra | Native equivalent | +|----------|-------------------| +| `mkdirpSync` | `fs.mkdirSync(path, { recursive: true })` | +| `readJsonSync` | `JSON.parse(fs.readFileSync(path, 'utf8'))` | +| `writeJsonSync` | `fs.writeFileSync(path, JSON.stringify(data, null, 2))` | + +--- + +## Implementation Roadmap + +### Phase 1: Add Test Coverage (Prerequisite) +- [ ] Add glob-related tests to build-tools +- [ ] Add basic integration tests for bundle-size-tools decompression + +### Phase 2: Low-Risk Changes +- [x] Replace `pako` with `fflate` in bundle-size-tools ✅ + +### Phase 3: Glob Consolidation +- [ ] Create compatibility wrapper for `globFn` +- [ ] Migrate build-tools from `glob` to `globby` +- [ ] Replace `minimatch` with `micromatch` in build-cli +- [ ] Replace `multimatch` with `micromatch` in build-tools +- [ ] Remove `glob`, `minimatch`, `multimatch` dependencies + +### Phase 4: Evaluate and Defer +- [ ] Re-evaluate `jszip` vs `fflate` for ZIP handling +- [ ] Monitor for `execa` alternatives during future upgrades + +--- + +## Success Metrics + +- Reduce direct dependency count by 4-6 +- Maintain lockfile line count reduction +- Zero regressions in build functionality +- All existing tests continue to pass + +--- + +## References + +- [globby documentation](https://github.com/sindresorhus/globby) +- [fast-glob options](https://github.com/mrmlnc/fast-glob#options-3) +- [fflate documentation](https://github.com/101arrowz/fflate) From ddcc12c3375ddde89ffb20dd4b1a63662bee82d6 Mon Sep 17 00:00:00 2001 From: Tyler Butler Date: Mon, 1 Dec 2025 14:08:57 -0800 Subject: [PATCH 04/10] Update build-tools/plans/dependency-reduction.md --- build-tools/plans/dependency-reduction.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build-tools/plans/dependency-reduction.md b/build-tools/plans/dependency-reduction.md index 7785b862a62b..86acc4a6e2fc 100644 --- a/build-tools/plans/dependency-reduction.md +++ b/build-tools/plans/dependency-reduction.md @@ -60,7 +60,7 @@ Six libraries serve similar globbing/matching purposes: #### Recommended Consolidation -Consolidate on `globby` (which uses `fast-glob` with `micromatch` internally): +Consolidate on `tiny-globby`, which has a similar api to globby: 1. **Replace `glob` with `globby`** in build-tools/taskUtils.ts 2. **Replace `minimatch` with `micromatch`** in build-cli From 21e544fa4095ad3e32f588328d79a0f02810f212 Mon Sep 17 00:00:00 2001 From: Tyler Butler Date: Mon, 1 Dec 2025 14:09:05 -0800 Subject: [PATCH 05/10] Update build-tools/plans/dependency-reduction.md --- build-tools/plans/dependency-reduction.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/build-tools/plans/dependency-reduction.md b/build-tools/plans/dependency-reduction.md index 86acc4a6e2fc..a1aeeac61a84 100644 --- a/build-tools/plans/dependency-reduction.md +++ b/build-tools/plans/dependency-reduction.md @@ -62,9 +62,9 @@ Six libraries serve similar globbing/matching purposes: Consolidate on `tiny-globby`, which has a similar api to globby: -1. **Replace `glob` with `globby`** in build-tools/taskUtils.ts -2. **Replace `minimatch` with `micromatch`** in build-cli -3. **Replace `multimatch` with `micromatch`** in build-tools +1. **Replace `glob` with `tinyglobby`** in build-tools/taskUtils.ts +2. **Replace `minimatch` with `picomatch`** in build-cli +3. **Replace `multimatch` with `picomatch`** in build-tools #### Key Migration Considerations From cee4d9d39e7d5e9d735564145d539d04e8cdcc51 Mon Sep 17 00:00:00 2001 From: Tyler Butler Date: Mon, 1 Dec 2025 14:34:08 -0800 Subject: [PATCH 06/10] consolidate on tinyglobby --- build-tools/plans/dependency-reduction.md | 117 ++++++++++++++++++---- 1 file changed, 96 insertions(+), 21 deletions(-) diff --git a/build-tools/plans/dependency-reduction.md b/build-tools/plans/dependency-reduction.md index 7785b862a62b..764155b545ad 100644 --- a/build-tools/plans/dependency-reduction.md +++ b/build-tools/plans/dependency-reduction.md @@ -60,40 +60,57 @@ Six libraries serve similar globbing/matching purposes: #### Recommended Consolidation -Consolidate on `globby` (which uses `fast-glob` with `micromatch` internally): +Consolidate on `tinyglobby` for file system globbing and `picomatch` for pattern matching: -1. **Replace `glob` with `globby`** in build-tools/taskUtils.ts -2. **Replace `minimatch` with `micromatch`** in build-cli -3. **Replace `multimatch` with `micromatch`** in build-tools +1. **Replace `glob` with `tinyglobby`** in build-tools/taskUtils.ts +2. **Replace `minimatch` with `picomatch`** in build-cli/repoConfig.ts +3. **Replace `multimatch` with `picomatch`** in build-tools/biomeConfig.ts + +Note: `picomatch` is already used in `miscTasks.ts` for pattern scanning, so consolidating on it reduces total dependencies rather than adding new ones. + +#### API Migration Patterns + +| Original | Replacement | +|----------|-------------| +| `minimatch(path, pattern)` returns boolean | `picomatch(pattern)(path)` returns boolean | +| `multimatch(paths, patterns)` returns filtered array | `paths.filter(picomatch(patterns))` | +| `glob(pattern, options, callback)` | `tinyglobby.glob(pattern, options)` returns Promise | #### Key Migration Considerations -Option name differences between `glob` and `globby`: +Option name differences between `glob` and `tinyglobby`: -| glob option | globby/fast-glob equivalent | -|-------------|----------------------------| -| `nodir: true` | `onlyFiles: true` (default) | -| `follow: true` | `followSymbolicLinks: true` | -| `ignore: "pattern"` | `ignore: ["pattern"]` (array required) | +| glob option | tinyglobby/fast-glob equivalent | Used in | +|-------------|----------------------------|---------| +| `nodir: true` | `onlyFiles: true` (default) | miscTasks.ts, ts2EsmTask.ts | +| `follow: true` | `followSymbolicLinks: true` | miscTasks.ts (CopyfilesTask) | +| `ignore: "pattern"` | `ignore: ["pattern"]` (array required) | fluidRepoBuild.ts | +| `cwd` | `cwd` (same) | prettierTask.ts, ts2EsmTask.ts | +| `absolute: true` | `absolute: true` (same) | ts2EsmTask.ts | +| `dot: true` | `dot: true` (same) | miscTasks.ts (CopyfilesTask) | #### Risk Reduction Steps 1. **Add integration tests for glob-dependent functionality:** - Create test cases in `build-tools/src/test/` covering: - - `CopyfilesTask` glob behavior with various options + - `CopyfilesTask` glob behavior with various options (`dot`, `follow`, `ignore`) - `TypeValidationTask` output file discovery - `GoodFence` input file enumeration - `DepCruiseTask` pattern matching - - Test edge cases: dot files, symlinks, nested directories + - Test edge cases: dot files, symlinks, nested directories, ignore patterns + +2. **Add tests for pattern matching:** + - Test `repoConfig.ts` branch pattern matching with various branch names + - Test `biomeConfig.ts` include/ignore filtering with multiple patterns -2. **Create a compatibility wrapper:** +3. **Create a compatibility wrapper:** ```typescript - // Temporary adapter that accepts old glob options and converts to globby + // Temporary adapter that accepts old glob options and converts to tinyglobby export function globFn(pattern: string, options: GlobCompatOptions = {}): Promise { - return globby(pattern, { + return glob(pattern, { onlyFiles: options.nodir ?? true, followSymbolicLinks: options.follow ?? false, - ignore: options.ignore ? [options.ignore] : [], + ignore: Array.isArray(options.ignore) ? options.ignore : options.ignore ? [options.ignore] : [], cwd: options.cwd, absolute: options.absolute, dot: options.dot, @@ -101,10 +118,67 @@ Option name differences between `glob` and `globby`: } ``` -3. **Migrate incrementally by file:** +4. **Migrate incrementally by file:** - Start with files that have test coverage - Manually verify behavior for untested files +5. **Migration code for minimatch → picomatch** (in repoConfig.ts): + ```typescript + // Before + import { minimatch } from "minimatch"; + if (minimatch(branch, branchPattern) === true) { ... } + + // After + import picomatch from "picomatch"; + const isMatch = picomatch(branchPattern); + if (isMatch(branch)) { ... } + ``` + +6. **Migration code for multimatch → picomatch** (in biomeConfig.ts): + ```typescript + // Before + import multimatch from "multimatch"; + const includedPaths = multimatch([...gitLsFiles], prefixedIncludes); + + // After + import picomatch from "picomatch"; + const isMatch = picomatch(prefixedIncludes); + const includedPaths = [...gitLsFiles].filter(isMatch); + ``` + +7. **Gitignore support with tinyglobby:** + + Unlike `globby`, `tinyglobby` does not have built-in gitignore support. If gitignore filtering is needed, use this pattern (from [e18e.dev](https://e18e.dev/docs/replacements/globby.html)): + + ```typescript + import { execSync } from 'node:child_process'; + import { escapePath, glob } from 'tinyglobby'; + + async function globWithGitignore(patterns, options = {}) { + const { cwd = process.cwd(), ...restOptions } = options; + + try { + const gitIgnored = execSync( + 'git ls-files --others --ignored --exclude-standard --directory', + { cwd, encoding: 'utf8', stdio: ['ignore', 'pipe', 'ignore'] } + ) + .split('\n') + .filter(Boolean) + .map(p => escapePath(p)); + + return glob(patterns, { + ...restOptions, + cwd, + ignore: [...(restOptions.ignore || []), ...gitIgnored] + }); + } catch { + return glob(patterns, options); + } + } + ``` + + Note: The current `globFn` usage in build-tools does not appear to rely on gitignore support, so this may not be needed for the initial migration. + --- ### 2. Additional Compression Consolidation @@ -213,9 +287,9 @@ If desired, these can be replaced with native Node.js equivalents: ### Phase 3: Glob Consolidation - [ ] Create compatibility wrapper for `globFn` -- [ ] Migrate build-tools from `glob` to `globby` -- [ ] Replace `minimatch` with `micromatch` in build-cli -- [ ] Replace `multimatch` with `micromatch` in build-tools +- [ ] Migrate build-tools from `glob` to `tinyglobby` +- [ ] Replace `minimatch` with `picomatch` in build-cli +- [ ] Replace `multimatch` with `picomatch` in build-tools - [ ] Remove `glob`, `minimatch`, `multimatch` dependencies ### Phase 4: Evaluate and Defer @@ -235,6 +309,7 @@ If desired, these can be replaced with native Node.js equivalents: ## References -- [globby documentation](https://github.com/sindresorhus/globby) +- [tinyglobby documentation](https://github.com/SuperchupuDev/tinyglobby) +- [picomatch documentation](https://github.com/micromatch/picomatch) - [fast-glob options](https://github.com/mrmlnc/fast-glob#options-3) - [fflate documentation](https://github.com/101arrowz/fflate) From 17088b4803518f11ed85c6ff6a8fe886fd5a3124 Mon Sep 17 00:00:00 2001 From: Tyler Butler Date: Wed, 3 Dec 2025 10:22:42 -0800 Subject: [PATCH 07/10] replace jszip with fflate wrapper --- build-tools/packages/build-cli/package.json | 1 - .../src/codeCoverage/getCoverageMetrics.ts | 43 +++++++---------- .../azureDevops/getBaselineBuildMetrics.ts | 14 +++--- .../packages/bundle-size-tools/package.json | 1 - .../src/ADO/AdoArtifactFileProvider.ts | 48 ++++++++----------- .../packages/bundle-size-tools/src/index.ts | 1 + .../bundle-size-tools/src/utilities/index.ts | 2 +- .../src/utilities/unzipStream.ts | 35 ++++++++++++-- build-tools/pnpm-lock.yaml | 6 --- 9 files changed, 81 insertions(+), 70 deletions(-) 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/package.json b/build-tools/packages/bundle-size-tools/package.json index dfcb39be9868..3fb43ed0bbbf 100644 --- a/build-tools/packages/bundle-size-tools/package.json +++ b/build-tools/packages/bundle-size-tools/package.json @@ -42,7 +42,6 @@ "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" 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/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/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..a92853e031ef 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,12 @@ * Licensed under the MIT License. */ -import * as JSZip from "jszip"; +import { unzipSync } from "fflate"; + +/** + * 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 +26,30 @@ 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. + * @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); + const unzipped = unzipSync(new Uint8Array(buffer)); + + const files = new Map(); + const prefix = baseFolder ? `${baseFolder}/` : ""; + + for (const [path, data] of Object.entries(unzipped)) { + if (prefix && !path.startsWith(prefix)) continue; + // Strip the base folder prefix for cleaner relative paths + const relativePath = prefix ? path.slice(prefix.length) : path; + if (relativePath) { + files.set(relativePath, Buffer.from(data)); + } + } + + return files; } diff --git a/build-tools/pnpm-lock.yaml b/build-tools/pnpm-lock.yaml index eb96638e8204..1e1274239713 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 From 5cd837f503eef0c3f8fd79dd4fb35b744a340f98 Mon Sep 17 00:00:00 2001 From: Tyler Butler Date: Wed, 3 Dec 2025 11:24:59 -0800 Subject: [PATCH 08/10] updates --- .../api-report/bundle-size-tools.api.md | 17 +++++++++-------- .../src/ADO/AdoSizeComparator.ts | 6 ++++-- 2 files changed, 13 insertions(+), 10 deletions(-) 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/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); From 5c53a4a7d496caf32af1d4bf624b42ced158d270 Mon Sep 17 00:00:00 2001 From: Tyler Butler Date: Wed, 3 Dec 2025 14:43:33 -0800 Subject: [PATCH 09/10] address memory concerns with lazy decompression --- .../src/utilities/unzipStream.ts | 94 ++++++++++++++++--- 1 file changed, 80 insertions(+), 14 deletions(-) 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 a92853e031ef..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,75 @@ * Licensed under the MIT License. */ -import { unzipSync } from "fflate"; +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. @@ -28,6 +96,7 @@ function readStreamAsBuffer(stream: NodeJS.ReadableStream): Promise { /** * 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 @@ -37,19 +106,16 @@ export async function unzipStream( baseFolder?: string, ): Promise { const buffer = await readStreamAsBuffer(stream); - const unzipped = unzipSync(new Uint8Array(buffer)); - const files = new Map(); - const prefix = baseFolder ? `${baseFolder}/` : ""; - - for (const [path, data] of Object.entries(unzipped)) { - if (prefix && !path.startsWith(prefix)) continue; - // Strip the base folder prefix for cleaner relative paths - const relativePath = prefix ? path.slice(prefix.length) : path; - if (relativePath) { - files.set(relativePath, Buffer.from(data)); - } - } + return new Promise((resolve, reject) => { + unzip(new Uint8Array(buffer), (err, unzipped) => { + if (err) { + reject(err); + return; + } - return files; + const prefix = baseFolder ? `${baseFolder}/` : ""; + resolve(new LazyUnzippedContents(unzipped, prefix)); + }); + }); } From 11d64886ff4d4bbeecc2138947818a2aabb9fe36 Mon Sep 17 00:00:00 2001 From: Tyler Butler Date: Thu, 4 Dec 2025 13:54:08 -0800 Subject: [PATCH 10/10] zip tests --- .../packages/bundle-size-tools/package.json | 11 +- .../bundle-size-tools/src/test/tsconfig.json | 22 + .../src/test/unzipStream.test.ts | 473 ++++++++++++++++++ build-tools/pnpm-lock.yaml | 15 + 4 files changed, 519 insertions(+), 2 deletions(-) create mode 100644 build-tools/packages/bundle-size-tools/src/test/tsconfig.json create mode 100644 build-tools/packages/bundle-size-tools/src/test/unzipStream.test.ts diff --git a/build-tools/packages/bundle-size-tools/package.json b/build-tools/packages/bundle-size-tools/package.json index 3fb43ed0bbbf..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,7 +37,8 @@ "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": { @@ -52,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/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/pnpm-lock.yaml b/build-tools/pnpm-lock.yaml index 1e1274239713..6c7910d26693 100644 --- a/build-tools/pnpm-lock.yaml +++ b/build-tools/pnpm-lock.yaml @@ -665,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