Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ legacy.d.ts

# Bundle analysis artifacts
bundleAnalysis
bundleAnalyzerJson

# Misc pipeline artifacts
artifacts
Expand Down
4 changes: 2 additions & 2 deletions build-tools/packages/build-cli/docs/generate.md
Original file line number Diff line number Diff line change
Expand Up @@ -128,8 +128,8 @@ USAGE
$ flub generate bundleSizeDiff [--json] [-v | --quiet] [--localReportPath <value>] [--outputDir <value>]

FLAGS
--localReportPath=<value> [default: ./artifacts/bundleAnalysis] Path to the locally-collected bundle reports for the
PR (as produced by `flub generate bundleStats`).
--localReportPath=<value> [default: ./artifacts/bundleAnalyzerJson] Path to the locally-collected bundle reports for
the PR (as produced by `flub generate bundleStats`).
--outputDir=<value> [default: ./artifacts/bundleSizeDiff] Directory to write result.json or error.json into.

LOGGING FLAGS
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,12 @@ const adoConstants = {
orgUrl: "https://dev.azure.com/fluidframework",
projectName: "public",
ciBuildDefinitionId: 48,
bundleAnalysisArtifactName: "bundleAnalysis",
artifactName: "bundleAnalyzerJson",
} as const;

// Default path to the PR's locally-collected bundle reports.
// Default path to the PR's locally-collected analyzer.json files.
// Matches where `flub generate bundleStats` (invoked via `npm run bundle-analysis:collect`) writes.
const defaultLocalReportPath = "./artifacts/bundleAnalysis";
const defaultLocalReportPath = "./artifacts/bundleAnalyzerJson";

// Default output directory. The pipeline publishes this directory as the `bundleSizeDiff`
// artifact.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,15 @@ export default class GenerateBundlestats extends BaseCommand<typeof GenerateBund
this.error("failed to get package information");
}

// Check each package location for a bundleAnalysis folder
// and copy it to a central location
// Check each package location for a bundleAnalysis folder (telemetry-feeding
// artifact: report.html, report.json, bundleStats.msp.gz) and a sibling
// bundleAnalyzerJson folder (PR-comparison-feeding artifact: analyzer.json),
// and copy each to its own central staging location. The two artifacts are
// kept separate because the FF-internal telemetry handler walks every .json
// under the bundleAnalysis artifact and would mis-parse analyzer.json.
let hasSmallAssetError = false;
const analysesDestPath = path.join(process.cwd(), "artifacts/bundleAnalysis");
const analyzerJsonDestPath = path.join(process.cwd(), "artifacts/bundleAnalyzerJson");

for (const pkg of pkgList) {
if (pkg.path === undefined) {
Expand Down Expand Up @@ -82,6 +87,12 @@ export default class GenerateBundlestats extends BaseCommand<typeof GenerateBund

copySync(packageAnalysisPath, path.join(analysesDestPath, pkg.name));
}

const packageAnalyzerJsonPath = path.join(pkg.path, "bundleAnalyzerJson");
if (existsSync(packageAnalyzerJsonPath)) {
this.log(`found bundleAnalyzerJson for ${pkg.name}`);
copySync(packageAnalyzerJsonPath, path.join(analyzerJsonDestPath, pkg.name));
}
}

if (hasSmallAssetError) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,14 +142,14 @@ export interface GetBundleSummariesFromAnalyzerArgs {
export function getPriorCommit(baseCommit: string): string;

// @public
export function getZipObjectFromArtifact(adoConnection: WebApi, projectName: string, buildNumber: number, bundleAnalysisArtifactName: string): Promise<JSZip>;
export function getZipObjectFromArtifact(adoConnection: WebApi, projectName: string, buildNumber: number, artifactName: string): Promise<JSZip>;

// @public (undocumented)
export interface IADOConstants {
// (undocumented)
buildsToSearch?: number;
artifactName: string;
// (undocumented)
bundleAnalysisArtifactName: string;
buildsToSearch?: number;
// (undocumented)
ciBuildDefinitionId: number;
// (undocumented)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export async function getZipObjectFromArtifact(
adoConnection: WebApi,
projectName: string,
buildNumber: number,
bundleAnalysisArtifactName: string,
artifactName: string,
): Promise<JSZip> {
const buildApi = await adoConnection.getBuildApi();

Expand All @@ -51,17 +51,14 @@ export async function getZipObjectFromArtifact(
const artifactStream = await buildApi.getArtifactContentZip(
projectName,
buildNumber,
bundleAnalysisArtifactName,
artifactName,
);
// 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);
assert(
result,
`getZipObjectFromArtifact could not find the folder ${bundleAnalysisArtifactName}`,
);
const result = (await unzipStream(artifactStream)).folder(artifactName);
assert(result, `getZipObjectFromArtifact could not find the folder ${artifactName}`);

return result;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,15 +165,13 @@ export class ADOSizeComparator {
// Baseline build succeeded
console.log(`Found baseline build with id: ${baselineBuild.id}`);
console.log(`projectName: ${this.adoConstants.projectName}`);
console.log(
`bundleAnalysisArtifactName: ${this.adoConstants.bundleAnalysisArtifactName}`,
);
console.log(`artifactName: ${this.adoConstants.artifactName}`);

baselineZip = await getZipObjectFromArtifact(
this.adoConnection,
this.adoConstants.projectName,
baselineBuild.id,
this.adoConstants.bundleAnalysisArtifactName,
this.adoConstants.artifactName,
).catch((error) => {
console.log(`Error unzipping object from artifact: ${error.message}`);
console.log(`Error stack: ${error.stack}`);
Expand Down
4 changes: 2 additions & 2 deletions build-tools/packages/bundle-size-tools/src/ADO/Constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ export interface IADOConstants {
// 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 name of the build artifact that contains the bundle size data
artifactName: string;

// The guid of the repo
// Used to post/update comments in ADO
Expand Down
2 changes: 1 addition & 1 deletion examples/utils/bundle-size-tests/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
"build:test:esm": "tsc --project ./src/test/tsconfig.json",
"check:biome": "biome check .",
"check:format": "npm run check:biome",
"clean": "rimraf --glob build dist lib bundleAnalysis \"**/*.tsbuildinfo\" \"**/*.build.log\" nyc",
"clean": "rimraf --glob build dist lib bundleAnalysis bundleAnalyzerJson \"**/*.tsbuildinfo\" \"**/*.build.log\" nyc",
"eslint": "eslint --quiet --format stylish src",
"eslint:fix": "eslint --quiet --format stylish src --fix --fix-type problem,suggestion,layout",
"explore:tree": "fluid-build . --task webpack && source-map-explorer ./build/sharedTree.js --html bundleAnalysis/reportTree.html",
Expand Down
6 changes: 5 additions & 1 deletion examples/utils/bundle-size-tests/webpack.config.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -130,9 +130,13 @@ module.exports = {
}),
// Generates analyzer.json with per-asset statSize/parsedSize/gzipSize that
// `flub generate bundleSizeDiff` consumes to compute the bundle-size diff.
// Emitted to a sibling folder so the file ends up in its own artifact
// (bundleAnalyzerJson) rather than alongside bundleAnalysis content; the
// FF-internal telemetry handler walks every .json under the bundleAnalysis
// artifact and would otherwise try to parse this file as webpack stats.
new BundleAnalyzerPlugin({
analyzerMode: "json",
reportFilename: path.resolve(process.cwd(), "bundleAnalysis/analyzer.json"),
reportFilename: path.resolve(process.cwd(), "bundleAnalyzerJson/analyzer.json"),
}),
// Generates bundleStats.msp.gz (compressed webpack stats; consumed by the
// FF-internal telemetry handler).
Expand Down
46 changes: 39 additions & 7 deletions tools/pipelines/templates/build-npm-client-package.yml
Original file line number Diff line number Diff line change
Expand Up @@ -403,23 +403,41 @@ extends:

# Bundle analysis
- ${{ if eq(parameters.taskBundleAnalysis, true) }}:
- task: Npm@1
# Bash@3 (not Npm@1) so `flub` resolves via PATH to the globally-linked
# workspace build; `npm run` would shadow it with the catalog-pinned
# flub, which currently lacks the bundleAnalyzerJson aggregation.
- task: Bash@3
displayName: 'Calculate bundle sizes'
inputs:
command: custom
workingDir: '$(Pipeline.Workspace)/${{ parameters.buildDirectory }}'
customCommand: 'run bundle-analysis:collect'
targetType: inline
workingDirectory: '$(Pipeline.Workspace)/${{ parameters.buildDirectory }}'
script: |
set -eu -o pipefail
pnpm run webpack:profile
flub generate bundleStats

# Copy files so all artifacts we publish end up under the same parent folder.
# The sourceFolder should be wherever the 'npm run bundle-analysis:collect' task places its output.
# The sourceFolder should be wherever the 'Calculate bundle sizes' task places its output.
- task: CopyFiles@2
displayName: Copy bundle size files to artifact staging directory
inputs:
sourceFolder: '$(Pipeline.Workspace)/${{ parameters.buildDirectory }}/artifacts/bundleAnalysis'
targetFolder: $(Build.ArtifactStagingDirectory)/bundleAnalysis

# At this point we want to publish the bundleAnalysis artifact,
# but as part of 1ES migration that's now part of templateContext.outputs below.
# bundleAnalyzerJson is a parallel artifact carrying analyzer.json
# (consumed by `flub generate bundleSizeDiff`). It's separate from the
# bundleAnalysis artifact so the FF-internal telemetry handler — which
# walks every .json under bundleAnalysis — doesn't trip over a file
# whose shape it doesn't recognize.
- task: CopyFiles@2
displayName: Copy bundle analyzer JSON files to artifact staging directory
inputs:
sourceFolder: '$(Pipeline.Workspace)/${{ parameters.buildDirectory }}/artifacts/bundleAnalyzerJson'
targetFolder: $(Build.ArtifactStagingDirectory)/bundleAnalyzerJson

# At this point we want to publish the bundleAnalysis and
# bundleAnalyzerJson artifacts, but as part of 1ES migration that's
# now part of templateContext.outputs below.

# Computes the bundle size diff against the baseline CI build (definitionId 48 in the public
# project) and writes either artifacts/bundleSizeDiff/result.json (on success) or
Expand Down Expand Up @@ -579,6 +597,20 @@ extends:
sbomEnabled: false
publishLocation: pipeline

# Parallel artifact for analyzer.json. Kept separate from
# bundleAnalysis so the FF-internal telemetry handler (which walks
# every .json file under that artifact) doesn't pick this up.
# Only published on non-PR (CI) builds so it can serve as the baseline
# that PR builds compare against; PRs read their own analyzer.json
# from artifacts/bundleAnalyzerJson locally and don't need to publish it.
- output: pipelineArtifact
displayName: Publish Artifacts - bundle-analyzer-json
condition: and( succeeded(), ne(variables['Build.Reason'], 'PullRequest') )
targetPath: $(Build.ArtifactStagingDirectory)/bundleAnalyzerJson
artifactName: bundleAnalyzerJson
sbomEnabled: false
publishLocation: pipeline
Comment thread
ChumpChief marked this conversation as resolved.

# The artifactName is a public-facing pipeline contract; downstream consumers
# download it by name. Any rename here requires updating those consumers in lockstep.
- output: pipelineArtifact
Expand Down
46 changes: 39 additions & 7 deletions tools/pipelines/templates/build-npm-package.yml
Original file line number Diff line number Diff line change
Expand Up @@ -471,23 +471,41 @@ extends:

# Bundle analysis
- ${{ if eq(parameters.taskBundleAnalysis, true) }}:
- task: Npm@1
# Bash@3 (not Npm@1) so `flub` resolves via PATH to the globally-linked
# workspace build; `npm run` would shadow it with the catalog-pinned
# flub, which currently lacks the bundleAnalyzerJson aggregation.
- task: Bash@3
displayName: 'Calculate bundle sizes'
inputs:
command: custom
workingDir: '$(Pipeline.Workspace)/${{ parameters.buildDirectory }}'
customCommand: 'run bundle-analysis:collect'
targetType: inline
workingDirectory: '$(Pipeline.Workspace)/${{ parameters.buildDirectory }}'
script: |
set -eu -o pipefail
pnpm run webpack:profile
flub generate bundleStats

# Copy files so all artifacts we publish end up under the same parent folder.
# The sourceFolder should be wherever the 'npm run bundle-analysis:collect' task places its output.
# The sourceFolder should be wherever the 'Calculate bundle sizes' task places its output.
- task: CopyFiles@2
displayName: Copy bundle size files to artifact staging directory
inputs:
sourceFolder: '$(Pipeline.Workspace)/${{ parameters.buildDirectory }}/artifacts/bundleAnalysis'
targetFolder: $(Build.ArtifactStagingDirectory)/bundleAnalysis

# At this point we want to publish the bundleAnalysis artifact,
# but as part of 1ES migration that's now part of templateContext.outputs below.
# bundleAnalyzerJson is a parallel artifact carrying analyzer.json
# (consumed by `flub generate bundleSizeDiff`). It's separate from the
# bundleAnalysis artifact so the FF-internal telemetry handler — which
# walks every .json under bundleAnalysis — doesn't trip over a file
# whose shape it doesn't recognize.
- task: CopyFiles@2
displayName: Copy bundle analyzer JSON files to artifact staging directory
Comment thread
ChumpChief marked this conversation as resolved.
inputs:
sourceFolder: '$(Pipeline.Workspace)/${{ parameters.buildDirectory }}/artifacts/bundleAnalyzerJson'
targetFolder: $(Build.ArtifactStagingDirectory)/bundleAnalyzerJson

# At this point we want to publish the bundleAnalysis and
# bundleAnalyzerJson artifacts, but as part of 1ES migration that's
# now part of templateContext.outputs below.

# Docs
- ${{ if ne(parameters.taskBuildDocs, false) }}:
Expand Down Expand Up @@ -574,6 +592,20 @@ extends:
sbomEnabled: false
publishLocation: pipeline

# Parallel artifact for analyzer.json. Kept separate from
# bundleAnalysis so the FF-internal telemetry handler (which walks
# every .json file under that artifact) doesn't pick this up.
# Only published on non-PR (CI) builds so it can serve as the baseline
# that PR builds compare against; PRs read their own analyzer.json
# from artifacts/bundleAnalyzerJson locally and don't need to publish it.
- output: pipelineArtifact
displayName: Publish Artifacts - bundle-analyzer-json
condition: and( succeeded(), ne(variables['Build.Reason'], 'PullRequest'), eq(${{ parameters.isBundleSizeArtifactsPipeline }}, true) )
targetPath: $(Build.ArtifactStagingDirectory)/bundleAnalyzerJson
artifactName: bundleAnalyzerJson
sbomEnabled: false
publishLocation: pipeline

# Publish the Code Coverage artifact in this format as it is easier for devs to find coverage html for each file.
- ${{ if eq(parameters.testCoverage, true) }}:
- output: pipelineArtifact
Expand Down
Loading