From 4f1a7d2ce9adeebc49c86dc46e2fb87b3037424b Mon Sep 17 00:00:00 2001 From: Matt Rakow Date: Mon, 4 May 2026 14:58:08 -0700 Subject: [PATCH 1/3] Drop sizeRegressionDetected from bundleSizeDiff output The producer-side regression-detection check (any non-total metric growing by more than 5KB triggers a `sizeRegressionDetected: boolean` field on the result) was a hardcoded threshold baked into the producer. Per the broader design direction, the producer should be unopinionated about thresholds and emit raw size data; the consumer (the future GH Actions workflow that posts PR comments / manages labels) decides what counts as a regression. Removes: - detectSizeRegression function - sizeRegressionThresholdBytes constant - sizeRegressionDetected field from BundleSizeDiffResult's "changes" variant - BundleMetric type import (only used by the removed function) The result.json schema's "changes" variant now contains only `comparison`; the "no-changes" variant is unchanged. No consumer reads result.json yet (the GH workflow that will is a future PR), so this is a no-op at the seam and lets that future consumer set its own threshold. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../src/commands/generate/bundleSizeDiff.ts | 33 +++---------------- 1 file changed, 4 insertions(+), 29 deletions(-) diff --git a/build-tools/packages/build-cli/src/commands/generate/bundleSizeDiff.ts b/build-tools/packages/build-cli/src/commands/generate/bundleSizeDiff.ts index eebb9b520615..5f87540664c6 100644 --- a/build-tools/packages/build-cli/src/commands/generate/bundleSizeDiff.ts +++ b/build-tools/packages/build-cli/src/commands/generate/bundleSizeDiff.ts @@ -8,7 +8,6 @@ import path from "node:path"; import { ADOSizeComparator, type BundleComparison, - type BundleMetric, bundlesContainNoChanges, getAzureDevopsApi, } from "@fluidframework/bundle-size-tools"; @@ -33,10 +32,6 @@ const defaultLocalReportPath = "./artifacts/bundleAnalysis"; // artifact. const defaultOutputDir = "./artifacts/bundleSizeDiff"; -// Any single non-total metric that grows by more than this threshold is considered a -// regression. -const sizeRegressionThresholdBytes = 5120; - // Output file names. Only one of these is present per run: `result.json` when the // comparison produced a meaningful result, or `error.json` when it did not. Consumers // use file existence as the success/failure discriminator without needing to parse JSON. @@ -46,17 +41,14 @@ const errorFileName = "error.json"; /** * Shape of the `result.json` file produced on a successful comparison, discriminated by * `kind`. On `"no-changes"`, the comparison ran and found no size deltas. On `"changes"`, - * the comparison found size deltas; `comparison` holds the diff and `sizeRegressionDetected` - * flags any non-total metric that grew past the threshold. + * the comparison found size deltas and `comparison` holds the diff. The producer is + * unopinionated about what constitutes a "regression" — consumers apply their own thresholds. */ type BundleSizeDiffResult = { prNumber: number; baseCommit: string; targetBranch: string; -} & ( - | { kind: "no-changes" } - | { kind: "changes"; sizeRegressionDetected: boolean; comparison: BundleComparison[] } -); +} & ({ kind: "no-changes" } | { kind: "changes"; comparison: BundleComparison[] }); /** * Shape of the `error.json` file produced when the command could not produce a comparison @@ -70,18 +62,6 @@ interface BundleSizeDiffError { error: string; } -/** - * Compute whether any bundle shows a metric growing by more than the regression threshold. - */ -function detectSizeRegression(comparison: BundleComparison[]): boolean { - return comparison.some((bundle: BundleComparison) => - Object.values(bundle.commonBundleMetrics).some( - ({ baseline, compare }: { baseline: BundleMetric; compare: BundleMetric }) => - compare.parsedSize - baseline.parsedSize > sizeRegressionThresholdBytes, - ), - ); -} - export default class GenerateBundleSizeDiff extends BaseCommand< typeof GenerateBundleSizeDiff > { @@ -183,12 +163,7 @@ export default class GenerateBundleSizeDiff extends BaseCommand< }; const result: BundleSizeDiffResult = bundlesContainNoChanges(comparison) ? { ...common, kind: "no-changes" } - : { - ...common, - kind: "changes", - sizeRegressionDetected: detectSizeRegression(comparison), - comparison, - }; + : { ...common, kind: "changes", comparison }; await writeFile(resultPath, JSON.stringify(result, undefined, 2)); this.info(`Wrote ${resultPath}`); From afdcb28248457d2536a980fe5ecf725daf7c076b Mon Sep 17 00:00:00 2001 From: Matt Rakow Date: Mon, 4 May 2026 15:12:21 -0700 Subject: [PATCH 2/3] Remove tagWaiting and naive-fallback mechanisms from ADOSizeComparator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two pre-existing behaviors getting cut, both noted as flawed by PR 2's design: 1. **tagWaiting:** when the baseline build was incomplete at PR-build time, the comparator would tag the PR's own build with a marker so a downstream process could re-run the comparison once the baseline finished. The downstream half (consuming the tag, retriggering the PR build) was never written. The tagging was effectively no-op overhead. PR 3's GH-Actions workflow approach (re-run flub generate bundleSizeDiff on a schedule) replaces this need entirely. 2. **naive fallback:** when no CI build was found for the baseline commit (or the build had no usable artifact), the comparator walked prior commits via `getPriorCommit` looking for one with a usable build. The resulting comparison was incorrect-by-design — main-side deltas between merge-base and merge-base − N were attributed to the PR. Removing both lets ADOSizeComparator's getSizeComparison flatten from a while-loop with continue/fallback branches into a linear sequence of early-return validations. The error messages also become more specific — "No CI build found for baseline commit X" / "Baseline build did not publish bundle artifacts" rather than the generic "Could not find a usable baseline build". Removed: - bundle-size-tools/src/ADO/getBuildTagForCommit.ts (whole file) - ADOSizeComparator.tagBuildAsWaiting private method - ADOSizeComparator.naiveFallbackCommitGenerator static method - ADOSizeComparator constructor params: adoBuildId, getFallbackCommit - getSizeComparison's `tagWaiting` parameter - IADOConstants fields: prBuildDefinitionId, projectRepoGuid (only used by the now-gone tagWaiting / comments machinery) - utilities/gitCommands.ts's getPriorCommit (only used by the fallback) - All corresponding barrel exports Updated: - build-cli's bundleSizeDiff.ts: simplified the ADOSizeComparator constructor call (drops the trailing `undefined` and naiveFallbackCommitGenerator args) and the getSizeComparison call (no `false` arg). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../src/commands/generate/bundleSizeDiff.ts | 4 +- .../api-report/bundle-size-tools.api.md | 17 +- .../src/ADO/AdoSizeComparator.ts | 184 +++++------------- .../bundle-size-tools/src/ADO/Constants.ts | 9 - .../src/ADO/getBuildTagForCommit.ts | 11 -- .../bundle-size-tools/src/ADO/index.ts | 1 - .../packages/bundle-size-tools/src/index.ts | 2 - .../src/utilities/gitCommands.ts | 4 - .../bundle-size-tools/src/utilities/index.ts | 2 +- 9 files changed, 55 insertions(+), 179 deletions(-) delete mode 100644 build-tools/packages/bundle-size-tools/src/ADO/getBuildTagForCommit.ts diff --git a/build-tools/packages/build-cli/src/commands/generate/bundleSizeDiff.ts b/build-tools/packages/build-cli/src/commands/generate/bundleSizeDiff.ts index 5f87540664c6..546d399a1fb2 100644 --- a/build-tools/packages/build-cli/src/commands/generate/bundleSizeDiff.ts +++ b/build-tools/packages/build-cli/src/commands/generate/bundleSizeDiff.ts @@ -114,10 +114,8 @@ export default class GenerateBundleSizeDiff extends BaseCommand< adoConnection, localReportPath, targetBranch, - undefined, - ADOSizeComparator.naiveFallbackCommitGenerator, ); - const comparisonResult = await sizeComparator.getSizeComparison(false); + const comparisonResult = await sizeComparator.getSizeComparison(); const resolvedOutputDir = path.resolve(process.cwd(), outputDir); await mkdir(resolvedOutputDir, { recursive: true }); 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 2cb906c4f73b..a4b571f4a4bf 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 @@ -18,11 +18,8 @@ export class ADOSizeComparator { adoConstants: IADOConstants, adoConnection: WebApi, localReportPath: string, - targetBranch: string, - adoBuildId: number | undefined, - getFallbackCommit?: ((startingCommit: string) => Generator) | undefined); - getSizeComparison(tagWaiting: boolean): Promise; - static naiveFallbackCommitGenerator(startingCommit: string): Generator; + targetBranch: string); + getSizeComparison(): Promise; } // @public (undocumented) @@ -124,9 +121,6 @@ export interface GetBuildOptions { // @public export function getBuilds(adoConnection: WebApi, options: GetBuildOptions): Promise; -// @public -export function getBuildTagForCommit(commitHash: string): string; - // @public export function getBundleSummariesFromAnalyzer(args: GetBundleSummariesFromAnalyzerArgs): Promise; @@ -138,9 +132,6 @@ export interface GetBundleSummariesFromAnalyzerArgs { getAnalyzerJson: (relativePath: string) => Promise; } -// @public (undocumented) -export function getPriorCommit(baseCommit: string): string; - // @public export function getZipObjectFromArtifact(adoConnection: WebApi, projectName: string, buildNumber: number, bundleAnalysisArtifactName: string): Promise; @@ -155,11 +146,7 @@ export interface IADOConstants { // (undocumented) orgUrl: string; // (undocumented) - prBuildDefinitionId?: number; - // (undocumented) projectName: string; - // (undocumented) - projectRepoGuid?: string; } // @public 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 96178da1f311..e8f2c986b45c 100644 --- a/build-tools/packages/bundle-size-tools/src/ADO/AdoSizeComparator.ts +++ b/build-tools/packages/bundle-size-tools/src/ADO/AdoSizeComparator.ts @@ -10,7 +10,7 @@ import { join } from "path"; import type { BundleComparison } from "../BundleBuddyTypes"; import { compareBundles } from "../compareBundles"; -import { getBaselineCommit, getBuilds, getPriorCommit } from "../utilities"; +import { getBaselineCommit, getBuilds } from "../utilities"; import { getAnalyzerJsonFromZip, getAnalyzerPathsFromZipObject, @@ -21,7 +21,6 @@ import { getAnalyzerJsonFromFileSystem, getAnalyzerPathsFromFileSystem, } from "./FileSystemBundleFileProvider"; -import { getBuildTagForCommit } from "./getBuildTagForCommit"; import { getBundleSummariesFromAnalyzer } from "./getBundleSummaries"; /** @@ -39,10 +38,9 @@ export type SizeComparison = export class ADOSizeComparator { /** * The default number of most recent builds on the ADO pipeline to search when - * looking for a build matching a baseline commit, and the default number of - * fallback commits returned by the provided default fallback generator. The - * most recent builds may not necessarily match the chain of commits, but - * typically will when the pipeline only builds commits to main. + * looking for a build matching a baseline commit. The most recent builds may not + * necessarily match the chain of commits, but typically will when the pipeline + * only builds commits to main. */ private static readonly defaultBuildsToSearch = 20; @@ -64,45 +62,17 @@ export class ADOSizeComparator { * the baseline commit (`git merge-base origin/ HEAD`). */ private readonly targetBranch: string, - /** - * Optional current PR build id to use, such as to tag for - * later update when the baseline build has not completed - */ - private readonly adoBuildId: number | undefined, - /** - * Option to do fallback on commits when either there is no associated CI build or - * it does not have the needed artifacts. Fallback is not attempted for other - * issues, such as for a failed (but still present) CI build. This generator is - * only used for fallback (it should not provide the first commit to check) - */ - private readonly getFallbackCommit: - | ((startingCommit: string) => Generator) - | undefined = undefined, ) {} - /** - * Naive fallback generator provided for convenience. It yields the commit directly - * prior to the previous commit. - */ - public static *naiveFallbackCommitGenerator(startingCommit: string): Generator { - let currentCommit = startingCommit; - for (let i = 0; i < ADOSizeComparator.defaultBuildsToSearch; i++) { - currentCommit = getPriorCommit(currentCommit); - yield currentCommit; - } - } - /** * Run the bundle size comparison against the baseline build. * - * @param tagWaiting - If the build should be tagged to be updated when the baseline - * build completes (if it wasn't already complete when the comparison runs) * @returns A {@link SizeComparison} tagged with `kind: "success"` or `kind: "error"`. * Never throws: unexpected exceptions from underlying `git` shell-outs, ADO API * calls, or stats-file parsing are caught and reported via the `error` variant so * callers can rely on the return shape. */ - public async getSizeComparison(tagWaiting: boolean): Promise { + public async getSizeComparison(): Promise { // Declared outside the try block so the catch can still report the last-known // commit value in the synthesized error variant. let baselineCommit: string | undefined; @@ -110,96 +80,60 @@ export class ADOSizeComparator { baselineCommit = getBaselineCommit(this.targetBranch); console.log(`The baseline commit for this PR is ${baselineCommit}`); - // Some circumstances may want us to try a fallback, such as when a commit does - // not trigger any CI loops. If a fallback generator is provided, use that. - let baselineZip; - const fallbackGen = this.getFallbackCommit?.(baselineCommit!); const recentBuilds = await getBuilds(this.adoConnection, { project: this.adoConstants.projectName, definitions: [this.adoConstants.ciBuildDefinitionId], maxBuildsPerDefinition: this.adoConstants.buildsToSearch ?? ADOSizeComparator.defaultBuildsToSearch, }); - while (baselineCommit !== undefined) { - const baselineBuild = recentBuilds.find( - (build) => build.sourceVersion === baselineCommit, - ); - - if (baselineBuild === undefined) { - baselineCommit = fallbackGen?.next().value; - // For reasons that I don't understand, the "undefined" string is omitted in the log output, which makes the - // output very confusing. The string is capitalized here and elsewhere in this file as a workaround. - console.log( - `Trying backup baseline commit when baseline build is UNDEFINED: ${baselineCommit}`, - ); - continue; - } - - // Baseline build does not have id - if (baselineBuild.id === undefined) { - const error = `Baseline build does not have a build id`; - console.log(error); - return { kind: "error", baselineCommit, error }; - } - - // Baseline build is pending - if (baselineBuild.status !== BuildStatus.Completed) { - const error = "Baseline build for this PR has not yet completed."; - console.log(error); - - if (tagWaiting) { - await this.tagBuildAsWaiting(baselineCommit); - } - - return { kind: "error", baselineCommit, error }; - } - - // Baseline build failed - if (baselineBuild.result !== BuildResult.Succeeded) { - const error = - "Baseline CI build failed, cannot generate bundle analysis at this time"; - console.log(error); - return { kind: "error", baselineCommit, error }; - } - - // Baseline build succeeded - console.log(`Found baseline build with id: ${baselineBuild.id}`); - console.log(`projectName: ${this.adoConstants.projectName}`); - console.log( - `bundleAnalysisArtifactName: ${this.adoConstants.bundleAnalysisArtifactName}`, - ); - - baselineZip = await getZipObjectFromArtifact( - this.adoConnection, - this.adoConstants.projectName, - baselineBuild.id, - this.adoConstants.bundleAnalysisArtifactName, - ).catch((error) => { - console.log(`Error unzipping object from artifact: ${error.message}`); - console.log(`Error stack: ${error.stack}`); - return undefined; - }); - - // For reasons that I don't understand, the "undefined" string is omitted in the log output, which makes the - // output very confusing. The string is capitalized here and elsewhere in this file as a workaround. - console.log(`Baseline Zip === UNDEFINED: ${baselineZip === undefined}`); - - // Successful baseline build does not have the needed build artifacts - if (baselineZip === undefined) { - baselineCommit = fallbackGen?.next().value; - console.log( - `Trying backup baseline commit when successful baseline build does not have the needed build artifacts ${baselineCommit}`, - ); - continue; - } - - // Found usable baseline zip - break; + + const baselineBuild = recentBuilds.find( + (build) => build.sourceVersion === baselineCommit, + ); + + if (baselineBuild === undefined) { + const error = `No CI build found for baseline commit ${baselineCommit}`; + console.log(error); + return { kind: "error", baselineCommit, error }; + } + + if (baselineBuild.id === undefined) { + const error = `Baseline build does not have a build id`; + console.log(error); + return { kind: "error", baselineCommit, error }; } - // Unable to find a usable baseline - if (baselineCommit === undefined || baselineZip === undefined) { - const error = `Could not find a usable baseline build with search starting at CI ${getBaselineCommit(this.targetBranch)}`; + if (baselineBuild.status !== BuildStatus.Completed) { + const error = "Baseline build for this PR has not yet completed."; + console.log(error); + return { kind: "error", baselineCommit, error }; + } + + if (baselineBuild.result !== BuildResult.Succeeded) { + const error = "Baseline CI build failed, cannot generate bundle analysis at this time"; + console.log(error); + return { kind: "error", baselineCommit, error }; + } + + console.log(`Found baseline build with id: ${baselineBuild.id}`); + console.log(`projectName: ${this.adoConstants.projectName}`); + console.log( + `bundleAnalysisArtifactName: ${this.adoConstants.bundleAnalysisArtifactName}`, + ); + + const baselineZip = await getZipObjectFromArtifact( + this.adoConnection, + this.adoConstants.projectName, + baselineBuild.id, + this.adoConstants.bundleAnalysisArtifactName, + ).catch((error) => { + console.log(`Error unzipping object from artifact: ${error.message}`); + console.log(`Error stack: ${error.stack}`); + return undefined; + }); + + if (baselineZip === undefined) { + const error = "Baseline build did not publish bundle artifacts"; console.log(error); return { kind: "error", baselineCommit, error }; } @@ -217,22 +151,6 @@ export class ADOSizeComparator { } } - private async tagBuildAsWaiting(baselineCommit: string): Promise { - if (!this.adoBuildId) { - console.log( - "No ADO build ID was provided, we will not tag this build for follow up when the baseline build completes", - ); - } else { - // Tag the current build as waiting for the results of the master CI - const buildApi = await this.adoConnection.getBuildApi(); - await buildApi.addBuildTag( - this.adoConstants.projectName, - this.adoBuildId, - getBuildTagForCommit(baselineCommit), - ); - } - } - private async createComparisonFromZip(baselineZip: JSZip): Promise { const baselineZipBundlePaths = getAnalyzerPathsFromZipObject(baselineZip); diff --git a/build-tools/packages/bundle-size-tools/src/ADO/Constants.ts b/build-tools/packages/bundle-size-tools/src/ADO/Constants.ts index 48646ec63046..a2c9c78e1409 100644 --- a/build-tools/packages/bundle-size-tools/src/ADO/Constants.ts +++ b/build-tools/packages/bundle-size-tools/src/ADO/Constants.ts @@ -13,18 +13,9 @@ export interface IADOConstants { // The ID for the build that runs against main when PRs are merged ciBuildDefinitionId: number; - // The ID for the build that runs to validate PRs - // Used to update tagged PRs on CI build completion - // Note: Assumes CI and PR builds both run in the same org/project - prBuildDefinitionId?: number; - // The name of the build artifact that contains the bundle size artifacts bundleAnalysisArtifactName: string; - // The guid of the repo - // Used to post/update comments in ADO - projectRepoGuid?: string; - // The number of most recent ADO builds to pull when searching for one associated // with a specific commit, default 20. Pulling more builds takes longer, but may // be useful when there are a high volume of commits/builds. diff --git a/build-tools/packages/bundle-size-tools/src/ADO/getBuildTagForCommit.ts b/build-tools/packages/bundle-size-tools/src/ADO/getBuildTagForCommit.ts deleted file mode 100644 index a605da70eed0..000000000000 --- a/build-tools/packages/bundle-size-tools/src/ADO/getBuildTagForCommit.ts +++ /dev/null @@ -1,11 +0,0 @@ -/*! - * Copyright (c) Microsoft Corporation and contributors. All rights reserved. - * Licensed under the MIT License. - */ - -/** - * Returns the git tag to use to mark that a build is waiting for the baseline to be available for a commit hash. - */ -export function getBuildTagForCommit(commitHash: string): string { - return `bundle-size-tools-pending-${commitHash}`; -} diff --git a/build-tools/packages/bundle-size-tools/src/ADO/index.ts b/build-tools/packages/bundle-size-tools/src/ADO/index.ts index cac824354dc6..2b7b364043ff 100644 --- a/build-tools/packages/bundle-size-tools/src/ADO/index.ts +++ b/build-tools/packages/bundle-size-tools/src/ADO/index.ts @@ -15,7 +15,6 @@ export { getAnalyzerPathsFromFileSystem, } from "./FileSystemBundleFileProvider"; export { getAzureDevopsApi } from "./getAzureDevopsApi"; -export { getBuildTagForCommit } from "./getBuildTagForCommit"; export { BundleFileData, getAnalyzerFilePathsFromFolder, diff --git a/build-tools/packages/bundle-size-tools/src/index.ts b/build-tools/packages/bundle-size-tools/src/index.ts index 38b28b716f5c..1d2e9ae8a48a 100644 --- a/build-tools/packages/bundle-size-tools/src/index.ts +++ b/build-tools/packages/bundle-size-tools/src/index.ts @@ -13,7 +13,6 @@ export { getAnalyzerPathsFromFileSystem, getAnalyzerPathsFromZipObject, getAzureDevopsApi, - getBuildTagForCommit, getBundleSummariesFromAnalyzer, getZipObjectFromArtifact, IADOConstants, @@ -36,6 +35,5 @@ export { getAllFilesInDirectory, getBaselineCommit, getBuilds, - getPriorCommit, unzipStream, } from "./utilities"; diff --git a/build-tools/packages/bundle-size-tools/src/utilities/gitCommands.ts b/build-tools/packages/bundle-size-tools/src/utilities/gitCommands.ts index 0f7b83aeb52c..9785830ee397 100644 --- a/build-tools/packages/bundle-size-tools/src/utilities/gitCommands.ts +++ b/build-tools/packages/bundle-size-tools/src/utilities/gitCommands.ts @@ -12,7 +12,3 @@ import { execSync } from "child_process"; export function getBaselineCommit(targetBranch: string): string { return execSync(`git merge-base origin/${targetBranch} HEAD`).toString().trim(); } - -export function getPriorCommit(baseCommit: string): string { - return execSync(`git log --pretty=format:"%H" -1 ${baseCommit}~1`).toString().trim(); -} 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 73d47a9a9cb8..2212b04843ca 100644 --- a/build-tools/packages/bundle-size-tools/src/utilities/index.ts +++ b/build-tools/packages/bundle-size-tools/src/utilities/index.ts @@ -5,5 +5,5 @@ export { getAllFilesInDirectory } from "./getAllFilesInDirectory"; export { GetBuildOptions, getBuilds } from "./getBuilds"; -export { getBaselineCommit, getPriorCommit } from "./gitCommands"; +export { getBaselineCommit } from "./gitCommands"; export { unzipStream } from "./unzipStream"; From 12c8bbdcc0279a261ad33c8d6ff5aa819e896121 Mon Sep 17 00:00:00 2001 From: Matt Rakow Date: Mon, 4 May 2026 15:42:37 -0700 Subject: [PATCH 3/3] Replace BannedModulesPlugin with webpack's resolve.fallback Webpack 5 supports `resolve.fallback: { assert: false }` natively, which blocks the Node-core `assert` polyfill from being bundled. This is the same effect the custom BannedModulesPlugin provided, so drop the plugin and its supporting code from bundle-size-tools. Also drops `webpack` from bundle-size-tools' dependencies (it was only used by the plugin) and `@fluidframework/bundle-size-tools` from bundle-size-tests' devDependencies (only used here for the plugin). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../api-report/bundle-size-tools.api.md | 20 ------ .../packages/bundle-size-tools/package.json | 3 +- .../bannedModulesPlugin.ts | 72 ------------------- .../packages/bundle-size-tools/src/index.ts | 5 -- build-tools/pnpm-lock.yaml | 3 - examples/utils/bundle-size-tests/package.json | 1 - .../bundle-size-tests/webpack.config.cjs | 15 ++-- pnpm-lock.yaml | 16 ++--- 8 files changed, 11 insertions(+), 124 deletions(-) delete mode 100644 build-tools/packages/bundle-size-tools/src/bannedModulesPlugin/bannedModulesPlugin.ts 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 a4b571f4a4bf..ec27c7960206 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 @@ -10,7 +10,6 @@ import type { Build } from 'azure-devops-node-api/interfaces/BuildInterfaces'; import type { BundleAnalyzerPlugin } from 'webpack-bundle-analyzer'; import type JSZip from 'jszip'; import { WebApi } from 'azure-devops-node-api'; -import type Webpack from 'webpack'; // @public (undocumented) export class ADOSizeComparator { @@ -22,25 +21,6 @@ export class ADOSizeComparator { getSizeComparison(): Promise; } -// @public (undocumented) -export interface BannedModule { - moduleName: string; - reason: string; -} - -// @public -export class BannedModulesPlugin { - constructor(options: BannedModulesPluginOptions); - // (undocumented) - apply(compiler: Webpack.Compiler): void; -} - -// @public (undocumented) -export interface BannedModulesPluginOptions { - // (undocumented) - bannedModules: BannedModule[]; -} - // @public export interface BundleComparison { // (undocumented) diff --git a/build-tools/packages/bundle-size-tools/package.json b/build-tools/packages/bundle-size-tools/package.json index a0dcbdd645b5..0c7cc35803dd 100644 --- a/build-tools/packages/bundle-size-tools/package.json +++ b/build-tools/packages/bundle-size-tools/package.json @@ -42,8 +42,7 @@ "dependencies": { "azure-devops-node-api": "^11.2.0", "jszip": "^3.10.1", - "typescript": "~5.4.5", - "webpack": "^5.103.0" + "typescript": "~5.4.5" }, "devDependencies": { "@biomejs/biome": "~2.4.5", diff --git a/build-tools/packages/bundle-size-tools/src/bannedModulesPlugin/bannedModulesPlugin.ts b/build-tools/packages/bundle-size-tools/src/bannedModulesPlugin/bannedModulesPlugin.ts deleted file mode 100644 index bffa10094a2f..000000000000 --- a/build-tools/packages/bundle-size-tools/src/bannedModulesPlugin/bannedModulesPlugin.ts +++ /dev/null @@ -1,72 +0,0 @@ -/*! - * Copyright (c) Microsoft Corporation and contributors. All rights reserved. - * Licensed under the MIT License. - */ - -import type Webpack from "webpack"; - -const pluginName = "BannedModulesPlugin"; - -export interface BannedModule { - /** The package name of the module that should not appear in the bundle */ - moduleName: string; - - /** The reason to ban the module */ - reason: string; -} - -export interface BannedModulesPluginOptions { - bannedModules: BannedModule[]; -} - -/** - * Webpack plugin that enforces that certain modules are not included in any chunk of a webpack bundle - */ -export class BannedModulesPlugin { - constructor(private readonly options: BannedModulesPluginOptions) { - if (!options || !Array.isArray(options.bannedModules)) { - throw new Error( - `${pluginName} must have a config object with a bannedModules array as a property`, - ); - } - } - - apply(compiler: Webpack.Compiler): void { - // The banned modules that have been found. Maps the banned module name to an array of module paths that import the banned module - const foundBannedModules = new Map>(); - - compiler.hooks.done.tap(pluginName, (stats) => { - stats.toJson().modules?.forEach((mod) => { - // Infer the name of the package from the path. This current implementation assumes the name has 'node_modules/' in it somewhere - // modulePath should contain the relative path to the module, where the first part of the path should be the module name (e.g. assert/build/assert.js) - const modulePath = mod.name?.substring( - mod.name.indexOf("node_modules") + "node_modules".length + 1, - ); - - for (const bannedModule of this.options.bannedModules) { - if (modulePath?.startsWith(bannedModule.moduleName)) { - // We store the issuers as a set to remove duplicates - const bannedModuleIssuers = foundBannedModules.get(bannedModule) || new Set(); - bannedModuleIssuers.add(JSON.stringify(mod.issuerPath)); - foundBannedModules.set(bannedModule, bannedModuleIssuers); - break; - } - } - }); - if (foundBannedModules.size > 0) { - let errorMessage = `Found ${foundBannedModules.size} banned modules\n`; - foundBannedModules.forEach((issuerPaths, bannedModule) => { - errorMessage += `\tBanned module: ${bannedModule.moduleName}\n`; - errorMessage += `\tReason: ${bannedModule.reason}\n`; - // Generate a string with a friendly issuer map path so that we can easily debug why a banned module is being included - issuerPaths.forEach((issuerPathJson) => { - errorMessage += `\t\tIssuer: \n`; - const pathSegments: { name: string }[] = JSON.parse(issuerPathJson); - pathSegments.forEach((segment) => (errorMessage += `\t\t\t${segment.name}\n`)); - }); - }); - throw new Error(errorMessage); - } - }); - } -} diff --git a/build-tools/packages/bundle-size-tools/src/index.ts b/build-tools/packages/bundle-size-tools/src/index.ts index 1d2e9ae8a48a..0d4ed754f61a 100644 --- a/build-tools/packages/bundle-size-tools/src/index.ts +++ b/build-tools/packages/bundle-size-tools/src/index.ts @@ -24,11 +24,6 @@ export { BundleMetricSet, BundleSummaries, } from "./BundleBuddyTypes"; -export { - BannedModule, - BannedModulesPlugin, - BannedModulesPluginOptions, -} from "./bannedModulesPlugin/bannedModulesPlugin"; export { bundlesContainNoChanges, compareBundles } from "./compareBundles"; export { GetBuildOptions, diff --git a/build-tools/pnpm-lock.yaml b/build-tools/pnpm-lock.yaml index 2ae9f7ee8345..d8e49daceb1c 100644 --- a/build-tools/pnpm-lock.yaml +++ b/build-tools/pnpm-lock.yaml @@ -667,9 +667,6 @@ importers: typescript: specifier: ~5.4.5 version: 5.4.5 - webpack: - specifier: ^5.103.0 - version: 5.103.0 devDependencies: '@biomejs/biome': specifier: ~2.4.5 diff --git a/examples/utils/bundle-size-tests/package.json b/examples/utils/bundle-size-tests/package.json index 61b13f16c835..64ed010be2db 100644 --- a/examples/utils/bundle-size-tests/package.json +++ b/examples/utils/bundle-size-tests/package.json @@ -57,7 +57,6 @@ "@fluid-tools/version-tools": "catalog:buildTools", "@fluidframework/build-common": "^2.0.3", "@fluidframework/build-tools": "catalog:buildTools", - "@fluidframework/bundle-size-tools": "catalog:buildTools", "@fluidframework/eslint-config-fluid": "catalog:eslint", "@mixer/webpack-bundle-compare": "^0.1.0", "@types/mocha": "^10.0.10", diff --git a/examples/utils/bundle-size-tests/webpack.config.cjs b/examples/utils/bundle-size-tests/webpack.config.cjs index 0da81ea4429e..c28a673cb1ae 100644 --- a/examples/utils/bundle-size-tests/webpack.config.cjs +++ b/examples/utils/bundle-size-tests/webpack.config.cjs @@ -7,7 +7,6 @@ const path = require("path"); const { BundleComparisonPlugin } = require("@mixer/webpack-bundle-compare/dist/plugin"); const { BundleAnalyzerPlugin } = require("webpack-bundle-analyzer"); const DuplicatePackageCheckerPlugin = require("@cerner/duplicate-package-checker-webpack-plugin"); -const { BannedModulesPlugin } = require("@fluidframework/bundle-size-tools"); const { isInternalVersionScheme, fromInternalScheme, @@ -91,6 +90,11 @@ module.exports = { }, resolve: { extensions: [".tsx", ".ts", ".js"], + // Block the Node-core `assert` polyfill from being bundled. It's very large in browser + // builds; consumers should use the assert API in @fluidframework/core-utils instead. + fallback: { + assert: false, + }, }, output: { path: path.resolve(__dirname, "build"), @@ -98,15 +102,6 @@ module.exports = { }, node: false, plugins: [ - new BannedModulesPlugin({ - bannedModules: [ - { - moduleName: "assert", - reason: - "This module is very large when bundled in browser facing Javascript, instead use the assert API in @fluidframework/common-utils", - }, - ], - }), new DuplicatePackageCheckerPlugin({ // Also show module that is requiring each duplicate package verbose: true, diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 3dbc01c31fb1..e17d91dee356 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -15,9 +15,6 @@ catalogs: '@fluidframework/build-tools': specifier: ^0.65.0 version: 0.65.0 - '@fluidframework/bundle-size-tools': - specifier: ^0.65.0 - version: 0.65.0 eslint: '@fluidframework/eslint-config-fluid': specifier: ^9.0.0 @@ -4850,9 +4847,6 @@ importers: '@fluidframework/build-tools': specifier: catalog:buildTools version: 0.65.0(@types/node@22.19.17) - '@fluidframework/bundle-size-tools': - specifier: catalog:buildTools - version: 0.65.0(webpack-cli@5.1.4) '@fluidframework/eslint-config-fluid': specifier: catalog:eslint version: 9.0.0(@typescript-eslint/utils@8.56.1(eslint@9.39.1(jiti@2.6.1))(typescript@5.4.5))(eslint@9.39.1(jiti@2.6.1))(typescript@5.4.5) @@ -33588,7 +33582,7 @@ snapshots: fast-glob: 3.3.3 is-glob: 4.0.3 minimatch: 9.0.9 - semver: 7.7.3 + semver: 7.7.4 ts-api-utils: 2.4.0(typescript@5.4.5) typescript: 5.4.5 transitivePeerDependencies: @@ -33604,7 +33598,7 @@ snapshots: fast-glob: 3.3.3 is-glob: 4.0.3 minimatch: 9.0.9 - semver: 7.7.3 + semver: 7.7.4 ts-api-utils: 2.4.0(typescript@5.9.3) typescript: 5.9.3 transitivePeerDependencies: @@ -37454,7 +37448,7 @@ snapshots: '@babel/parser': 7.28.4 '@istanbuljs/schema': 0.1.3 istanbul-lib-coverage: 3.2.2 - semver: 7.7.3 + semver: 7.7.4 transitivePeerDependencies: - supports-color @@ -38371,7 +38365,7 @@ snapshots: make-dir@4.0.0: dependencies: - semver: 7.7.3 + semver: 7.7.4 make-error@1.3.6: {} @@ -39139,7 +39133,7 @@ snapshots: node-abi@3.71.0: dependencies: - semver: 7.7.3 + semver: 7.7.4 node-addon-api@4.3.0: {}