fix(bundle-analysis): publish analyzer.json as a separate bundleAnalyzerJson artifact#27246
Conversation
The FF-internal telemetry handler walks every .json file under the bundleAnalysis pipeline artifact and tries to parse each one as a webpack stats file (looking for `.assets`). After microsoft#27224 landed and added analyzer.json to that artifact (a top-level array, not a stats object), the handler crashes with `TypeError: fileData.assets is not iterable`, breaking the build-client telemetry upload step. Move analyzer.json out of the bundleAnalysis artifact entirely. It now travels in its own parallel pipeline artifact, bundleAnalyzerJson, with its own staging directory at every stage of the producer/collect/publish pipeline. The split also reflects a real distinction in audience and lifecycle: - bundleAnalysis: feeds the FF-internal size-telemetry dashboards. Contents (report.html, report.json, bundleStats.msp.gz) and consumer contract are owned by the internal telemetry handler. - bundleAnalyzerJson: feeds `flub generate bundleSizeDiff` (and the upcoming GH Actions PR-comparison workflow). Contents and shape are owned in this repo. Keeping them in separate artifacts means we can iterate freely on the PR-comparison feed without coordinating with ff_internal each time. Changes: - examples/utils/bundle-size-tests/webpack.config.cjs: emit analyzer.json to bundleAnalyzerJson/ instead of bundleAnalysis/. - examples/utils/bundle-size-tests/package.json: clean script also wipes bundleAnalyzerJson/. - .gitignore: ignore bundleAnalyzerJson/ alongside bundleAnalysis/. - build-cli/src/commands/generate/bundleStats.ts: also collect any bundleAnalyzerJson/ folders from each package into artifacts/bundleAnalyzerJson/<package>/. - build-cli/src/commands/generate/bundleSizeDiff.ts: switch the artifact name and local report path to bundleAnalyzerJson. - tools/pipelines/templates/build-npm-{client-,}package.yml: copy the new staging dir and publish bundleAnalyzerJson as a parallel pipeline artifact (only on non-PR builds, matching bundleAnalysis). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Hi! Thank you for opening this PR. Want me to review it? Based on the diff (153 lines, 12 files), I've queued these reviewers:
How this works
|
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Splits analyzer.json out of the existing bundleAnalysis pipeline artifact into a new parallel bundleAnalyzerJson artifact to prevent the FF-internal telemetry handler from mis-parsing it and breaking telemetry upload.
Changes:
- Emit and clean
analyzer.jsonunderbundleAnalyzerJson/(producer side). - Collect
bundleAnalyzerJson/intoartifacts/bundleAnalyzerJson/<package>/and updatebundleSizeDiffto consume that artifact. - Update Azure Pipelines templates to stage and publish the new
bundleAnalyzerJsonpipeline artifact.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/pipelines/templates/build-npm-package.yml | Stages and publishes new bundleAnalyzerJson artifact in CI builds. |
| tools/pipelines/templates/build-npm-client-package.yml | Stages and publishes new bundleAnalyzerJson artifact in CI builds (client template). |
| examples/utils/bundle-size-tests/webpack.config.cjs | Writes BundleAnalyzerPlugin output to bundleAnalyzerJson/analyzer.json. |
| examples/utils/bundle-size-tests/package.json | Extends clean script to remove bundleAnalyzerJson/. |
| build-tools/packages/build-cli/src/commands/generate/bundleStats.ts | Collects bundleAnalyzerJson/ into central artifacts directory. |
| build-tools/packages/build-cli/src/commands/generate/bundleSizeDiff.ts | Switches baseline artifact + local path from bundleAnalysis to bundleAnalyzerJson. |
| .gitignore | Ignores bundleAnalyzerJson/. |
Per review feedback (microsoft#27246): the field is no longer specific to bundle analysis (it now points at bundleAnalyzerJson) and the surrounding adoConstants object already provides scope, so the bundleAnalysis prefix just reads as out-of-date. Aligns with the naming used by PR2's planned library extraction (e.g. library/azureDevops/constants.ts). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two now-shorter strings collapse onto a single line. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Build 398312 failed the new "Copy bundle analyzer JSON files" step with "Not found SourceFolder: .../artifacts/bundleAnalyzerJson". The bundle- analysis:collect step was invoked via Npm@1, where `npm run` prepends node_modules/.bin to PATH. That points at the catalog-pinned build-cli, which doesn't have the bundleAnalyzerJson aggregation — so the directory never gets created. Switch to Bash@3 (matching the pattern already used by the bundleSizeDiff step) so `flub` resolves through the system PATH to the globally-linked workspace build, which has the new collect logic. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CI's "Check for extraneous modified files" guard caught two generated files that hadn't been re-committed after recent renames: - build-cli/docs/generate.md: oclif readme reflects the bundleSizeDiff command's new --localReportPath default (./artifacts/bundleAnalyzerJson). - bundle-size-tools/api-report/bundle-size-tools.api.md: api-extractor reflects the IADOConstants.artifactName rename and the same field on getZipObjectFromArtifact. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Description
#27224 added
analyzer.jsonto the existingbundleAnalysispipeline artifact. The FF-internal telemetry handler (@ff-internal/telemetry-generator/dist/handlers/bundleSizeHandler.js) walks every.jsonfile under that artifact and tries to parse each as a webpack stats file ({ assets: [...] }).analyzer.jsonis a top-level array, so the handler crashes withTypeError: fileData.assets is not iterableand breaks the build-client telemetry upload step.Fix on our side instead of in the handler: move
analyzer.jsonout of thebundleAnalysisartifact entirely. It now travels in its own parallel pipeline artifact,bundleAnalyzerJson, with its own staging directory at every stage of the producer/collect/publish pipeline.The split also reflects a real distinction in audience and lifecycle:
bundleAnalysisfeeds the FF-internal size-telemetry dashboards. Contents (report.html,report.json,bundleStats.msp.gz) and consumer contract are owned by the internal telemetry handler.bundleAnalyzerJsonfeedsflub generate bundleSizeDiff(and the upcoming GH Actions PR-comparison workflow). Contents and shape are owned in this repo.Keeping them in separate artifacts means we can iterate freely on the PR-comparison feed without coordinating with the FF-internal handler each time we want to change shape, add a file, etc.
Changes
examples/utils/bundle-size-tests/webpack.config.cjs: emitanalyzer.jsontobundleAnalyzerJson/instead ofbundleAnalysis/.examples/utils/bundle-size-tests/package.json: clean script also wipesbundleAnalyzerJson/..gitignore: ignorebundleAnalyzerJson/alongsidebundleAnalysis/.build-cli/src/commands/generate/bundleStats.ts: also collect anybundleAnalyzerJson/folders from each package intoartifacts/bundleAnalyzerJson/<package>/.build-cli/src/commands/generate/bundleSizeDiff.ts: switch the artifact name and local report path frombundleAnalysis→bundleAnalyzerJson.tools/pipelines/templates/build-npm-{client-,}package.yml: copy the new staging dir and publishbundleAnalyzerJsonas a parallel pipeline artifact (only on non-PR builds, matchingbundleAnalysis).Validation
pnpm webpackinbundle-size-testsemits to bothbundleAnalysis/andbundleAnalyzerJson/directories correctly.flub generate bundleStatspopulates bothartifacts/bundleAnalysis/<scope>/<pkg>/andartifacts/bundleAnalyzerJson/<scope>/<pkg>/.Reviewer Guidance
The review process is outlined on this wiki page.
The conceptual flow is straightforward — there's just a lot of mechanical plumbing the move has to thread through. Reviewing in this order may help: producer → collect → pipeline publish → consumer.