From e1115e3fdad5d62369b158f17de4c484d00498ab Mon Sep 17 00:00:00 2001 From: Cindy Zhang Date: Sat, 24 Oct 2026 11:06:37 -0700 Subject: [PATCH 1/3] fix(ci): the PR size report measures ESM entries and fills Before and Delta from the merge base analyze-pr.js resolves dist entries by module type: Babel writes ESM as .js in "type": "module" packages, so ESM was always N/A and the ESM file was labeled CJS. A new size-base job builds the PR's merge base beside build-storybook and runs analyze-pr.js --base-root --base-only; the PR comment matches its head and renders Before and Delta for modified components and package gzip size. --- .github/scripts/analyze-pr.js | 110 ++++++++++++++---- .github/scripts/analyze-pr.test.mjs | 116 +++++++++++++++++++ .github/scripts/generate-pr-comment.js | 40 ++++++- .github/scripts/generate-pr-comment.test.mjs | 52 +++++++++ .github/scripts/lib/pr-preview.mjs | 24 ++++ .github/scripts/lib/pr-preview.test.mjs | 26 +++++ .github/workflows/ci.yml | 70 +++++++++++ .github/workflows/pr-comment.yml | 10 ++ 8 files changed, 419 insertions(+), 29 deletions(-) diff --git a/.github/scripts/analyze-pr.js b/.github/scripts/analyze-pr.js index 6fde566b6de38..49fe3b0f704b2 100644 --- a/.github/scripts/analyze-pr.js +++ b/.github/scripts/analyze-pr.js @@ -4,8 +4,11 @@ /** * @description Analyzes PR changes to detect new/modified components and gather stats - * @input --base --head --output - * @output JSON file with component analysis; exits non-zero if the base...head diff fails (e.g. shallow clone without a merge base) + * @input --base --head --output [--base-root [--base-only]] + * @output JSON file with component analysis; exits non-zero if the base...head diff fails (e.g. shallow clone without a merge base). + * With --base-root (a built checkout of the merge base), the analysis also + * records baseComponentStats and baseBundlePackages measured in that tree; + * --base-only writes just those, with the head and base commits they belong to. */ const fs = require('node:fs'); @@ -28,6 +31,12 @@ const getArg = (name) => { const baseBranch = getArg('base') || 'main'; const headRef = getArg('head') || 'HEAD'; const outputFile = getArg('output') || 'analysis.json'; +const baseRoot = getArg('base-root'); +const baseOnly = args.includes('--base-only'); +if (baseOnly && !baseRoot) { + console.error('::error::--base-only needs --base-root'); + process.exit(1); +} const STORYBOOK_STORIES = 'apps/storybook/stories'; @@ -236,32 +245,50 @@ function getFileSizeBytes(filePath) { } } -// Get gzipped size of a file. Returns null (→ "N/A") when the file is absent, -// instead of the old behavior of gzipping a nonexistent path and reporting a -// bogus "0B". -function getGzipSize(filePath) { +// Gzipped byte count of a file, or null when the file is absent — never a +// bogus 0 for a path that does not exist. +function getGzipBytes(filePath) { if (!filePath || !fs.existsSync(filePath)) return null; try { const output = execSync(`gzip -c "${filePath}" | wc -c`, { encoding: 'utf8' }); const bytes = parseInt(output.trim(), 10); - if (!Number.isFinite(bytes) || bytes <= 0) return null; - if (bytes < 1024) return `${bytes}B`; - if (bytes < 1024 * 1024) return `${(bytes / 1024).toFixed(1)}KB`; - return `${(bytes / (1024 * 1024)).toFixed(2)}MB`; + return Number.isFinite(bytes) && bytes > 0 ? bytes : null; } catch { return null; } } -// Resolve a package/component's dist entry files. The build emits CJS-style -// `.js` today (no `.mjs`); we probe for both so the report stays correct if an -// ESM artifact is added later, and never measures a file that isn't there. -function resolveEntries(distDir) { - const esm = path.join(distDir, 'index.mjs'); - const cjs = path.join(distDir, 'index.js'); +function formatBytes(bytes) { + if (bytes === null) return null; + if (bytes < 1024) return `${bytes}B`; + if (bytes < 1024 * 1024) return `${(bytes / 1024).toFixed(1)}KB`; + return `${(bytes / (1024 * 1024)).toFixed(2)}MB`; +} + +// Whether a package's `.js` files are ES modules (`"type": "module"`). +function packageIsModule(pkg) { + try { + const manifest = path.join(process.cwd(), pkg.dir, 'package.json'); + return JSON.parse(fs.readFileSync(manifest, 'utf8')).type === 'module'; + } catch { + return false; + } +} + +// Resolve a package/component's dist entry files by module format. Babel +// writes ES modules with a `.js` extension in `"type": "module"` packages, so +// there `index.js` is the ESM entry; a package that is not a module ships ESM +// as `index.mjs` and CommonJS as `index.js`. `index.mjs`/`index.cjs` are ESM +// and CommonJS wherever they exist. A missing file is never measured. +function resolveEntries(distDir, pkg) { + const existing = (name) => { + const file = path.join(distDir, name); + return fs.existsSync(file) ? file : null; + }; + const isModule = packageIsModule(pkg); return { - esmPath: fs.existsSync(esm) ? esm : null, - cjsPath: fs.existsSync(cjs) ? cjs : null, + esmPath: existing('index.mjs') || (isModule ? existing('index.js') : null), + cjsPath: existing('index.cjs') || (isModule ? null : existing('index.js')), }; } @@ -482,7 +509,7 @@ function getComponentStats(pkg, componentName) { pkg.layout === 'flat' ? null : path.join(process.cwd(), pkgDist(pkg), componentName); - const entries = distDir ? resolveEntries(distDir) : { esmPath: null, cjsPath: null }; + const entries = distDir ? resolveEntries(distDir, pkg) : { esmPath: null, cjsPath: null }; const loc = getComponentLOC(pkg, componentName); const complexity = getComplexity(pkg, componentName); @@ -507,17 +534,17 @@ function getComponentStats(pkg, componentName) { // Get total bundle stats for a package's published entry. function getPackageBundleStats(pkg) { const distDir = path.join(process.cwd(), pkgDist(pkg)); - const { esmPath, cjsPath } = resolveEntries(distDir); - // Gzip the primary published entry — prefer ESM if it exists, else the CJS - // `.js` the build actually emits today. Never gzip a missing path. - const primary = esmPath || cjsPath; + const { esmPath, cjsPath } = resolveEntries(distDir, pkg); + // Gzip the primary published entry: ESM when it exists, else CommonJS. + const gzipBytes = getGzipBytes(esmPath || cjsPath); return { package: pkg.name, esmSize: getFileSize(esmPath), esmBytes: getFileSizeBytes(esmPath), cjsSize: getFileSize(cjsPath), cjsBytes: getFileSizeBytes(cjsPath), - gzipSize: getGzipSize(primary), + gzipSize: formatBytes(gzipBytes), + gzipBytes, }; } @@ -624,6 +651,40 @@ function analyze() { getPackageBundleStats(p), ); + // Base measurements come from a built checkout of the merge base. Component + // and package lists stay the head's; only the stats are read in the base tree. + let baseComponentStats = null; + let baseBundlePackages = null; + if (baseRoot) { + const headDir = process.cwd(); + process.chdir(path.resolve(headDir, baseRoot)); + try { + baseComponentStats = {}; + for (const key of modifiedComponents) { + const pkg = PACKAGES.find((p) => p.name === componentStats[key].package); + baseComponentStats[key] = getComponentStats(pkg, key); + } + baseBundlePackages = PACKAGES.filter((p) => changedPackages.has(p.name)).map((p) => + getPackageBundleStats(p), + ); + } finally { + process.chdir(headDir); + } + } + + if (baseOnly) { + const baseResult = { + headSha: execSync('git rev-parse HEAD', { encoding: 'utf8' }).trim(), + baseSha: execSync('git rev-parse HEAD', { cwd: baseRoot, encoding: 'utf8' }).trim(), + baseComponentStats, + baseBundlePackages, + analyzedAt: new Date().toISOString(), + }; + fs.writeFileSync(outputFile, JSON.stringify(baseResult, null, 2)); + console.log(`Base stats written to ${outputFile}`); + return baseResult; + } + const result = { newComponents, modifiedComponents, @@ -640,6 +701,7 @@ function analyze() { // or render it in the report should caveat when this is 'two-dot'. diffMode, bundlePackages, + ...(baseRoot ? { baseComponentStats, baseBundlePackages } : {}), // Back-compat: keep totalBundle pointed at core when core changed, so any // older consumer of this field still resolves. totalBundle: bundlePackages.find((b) => b.package === '@astryxdesign/core') || null, diff --git a/.github/scripts/analyze-pr.test.mjs b/.github/scripts/analyze-pr.test.mjs index a13c7554d1f0f..df62f458587bf 100644 --- a/.github/scripts/analyze-pr.test.mjs +++ b/.github/scripts/analyze-pr.test.mjs @@ -279,3 +279,119 @@ describe('analyze-pr component ownership', () => { } }); }); + +describe('analyze-pr base and head sizes', () => { + it('measures ESM entries by module type and reads base sizes from a merge-base build', () => { + const repo = fs.mkdtempSync(path.join(os.tmpdir(), 'astryx-analyze-size-')); + const baseRoot = path.join(repo, '.size-base'); + const write = (root, file, content) => { + fs.mkdirSync(path.dirname(path.join(root, file)), {recursive: true}); + fs.writeFileSync(path.join(root, file), content); + }; + try { + write(repo, 'package.json', '{}'); + write(repo, '.gitignore', 'dist/\n.size-base/\n*.json.out\n'); + write(repo, 'packages/core/package.json', '{"type": "module"}\n'); + write( + repo, + 'packages/core/src/index.ts', + "export {Card} from './Card';\n", + ); + write( + repo, + 'packages/core/src/Card/index.ts', + "export {Card} from './Card';\n", + ); + write( + repo, + 'packages/core/src/Card/Card.tsx', + 'export function Card() {}\n', + ); + write( + repo, + 'packages/core/src/Card/Card.doc.mjs', + "export const docs = {\n name: 'Card',\n};\n", + ); + git(repo, ['init', '-q', '-b', 'main']); + git(repo, ['config', 'user.email', 'test@test.co']); + git(repo, ['config', 'user.name', 'Test']); + git(repo, ['add', '-A']); + git(repo, ['commit', '-qm', 'base']); + git(repo, ['worktree', 'add', '-q', '--detach', baseRoot, 'HEAD']); + git(repo, ['checkout', '-q', '-b', 'feature']); + fs.appendFileSync( + path.join(repo, 'packages/core/src/Card/Card.tsx'), + 'export const changed = true;\n', + ); + git(repo, ['commit', '-qam', 'touch Card']); + + // Babel writes ES modules as `.js` in a "type": "module" package. + write(baseRoot, 'packages/core/dist/Card/index.js', 'x'.repeat(100)); + write( + baseRoot, + 'packages/core/dist/index.js', + 'export const base = 1;\n'.repeat(10), + ); + write(repo, 'packages/core/dist/Card/index.js', 'x'.repeat(160)); + write( + repo, + 'packages/core/dist/index.js', + 'export const head = 2;\n'.repeat(40), + ); + + execFileSync( + process.execPath, + [ + SCRIPT, + '--base', + 'main', + '--head', + 'HEAD', + '--base-root', + '.size-base', + '--output', + 'analysis.json', + ], + {cwd: repo, encoding: 'utf8'}, + ); + const analysis = JSON.parse( + fs.readFileSync(path.join(repo, 'analysis.json'), 'utf8'), + ); + expect(analysis.componentStats.Card.esmBytes).toBe(160); + expect(analysis.componentStats.Card.cjsBytes).toBeNull(); + expect(analysis.baseComponentStats.Card.esmBytes).toBe(100); + const [headBundle] = analysis.bundlePackages; + const [baseBundle] = analysis.baseBundlePackages; + expect(headBundle.package).toBe('@astryxdesign/core'); + expect(baseBundle.package).toBe('@astryxdesign/core'); + expect(headBundle.gzipBytes).toBeGreaterThan(0); + expect(baseBundle.gzipBytes).toBeGreaterThan(0); + + execFileSync( + process.execPath, + [ + SCRIPT, + '--base', + 'main', + '--head', + 'HEAD', + '--base-root', + '.size-base', + '--base-only', + '--output', + 'size-base.json', + ], + {cwd: repo, encoding: 'utf8'}, + ); + const baseOnly = JSON.parse( + fs.readFileSync(path.join(repo, 'size-base.json'), 'utf8'), + ); + expect(baseOnly.headSha).toBe(git(repo, ['rev-parse', 'HEAD'])); + expect(baseOnly.baseSha).toBe(git(repo, ['rev-parse', 'main'])); + expect(baseOnly.baseComponentStats.Card.esmBytes).toBe(100); + expect(baseOnly.componentStats).toBeUndefined(); + } finally { + fs.rmSync(repo, {recursive: true, force: true}); + } + }); +}); diff --git a/.github/scripts/generate-pr-comment.js b/.github/scripts/generate-pr-comment.js index 6ed004cc48bdc..8004550e5f9fd 100644 --- a/.github/scripts/generate-pr-comment.js +++ b/.github/scripts/generate-pr-comment.js @@ -3,7 +3,7 @@ /** * @description Generates and posts PR comment with analysis results - * @input --analysis --a11y --visual --storybook-url --sandbox-url --preview-state --source-conclusion --run-url + * @input --analysis --base-stats --a11y --visual --storybook-url --sandbox-url --preview-state --source-conclusion --run-url * @output Formatted markdown comment body to stdout */ @@ -21,6 +21,7 @@ const getArg = name => { }; const analysisFile = getArg('analysis') || 'analysis.json'; +const baseStatsFile = getArg('base-stats'); const a11yFile = getArg('a11y') || 'a11y-report.json'; const visualFile = getArg('visual'); const runUrl = getArg('run-url') || ''; @@ -44,6 +45,18 @@ try { console.error('Warning: Could not read analysis file:', e.message); } +// Base measurements from a built checkout of the merge base, written by +// `analyze-pr.js --base-only`. The caller has already matched their head. +if (baseStatsFile) { + try { + const base = JSON.parse(fs.readFileSync(baseStatsFile, 'utf8')); + analysis.baseComponentStats = base.baseComponentStats || undefined; + analysis.baseBundlePackages = base.baseBundlePackages || undefined; + } catch (e) { + console.error('Warning: Could not read base stats:', e.message); + } +} + try { a11yReport = JSON.parse(fs.readFileSync(a11yFile, 'utf8')); } catch (e) { @@ -86,8 +99,9 @@ if (analysis.newComponents && analysis.newComponents.length > 0) { : ''; componentSection += `
\n${inline(comp)}${pkgBadge}${sbBadge}\n\n`; componentSection += `| Metric | Value |\n|--------|-------|\n`; - componentSection += `| Bundle Size (ESM) | ${inline(stats.esmSize) || 'N/A'} |\n`; - componentSection += `| Bundle Size (CJS) | ${inline(stats.cjsSize) || 'N/A'} |\n`; + componentSection += `| Entry File (ESM) | ${inline(stats.esmSize) || 'N/A'} |\n`; + if (stats.cjsSize) + componentSection += `| Entry File (CJS) | ${inline(stats.cjsSize)} |\n`; componentSection += `| Lines of Code | ${inline(stats.linesOfCode) || 'N/A'} |\n`; componentSection += `| Source Files | ${inline(stats.fileCount) || 'N/A'} |\n`; componentSection += `| Complexity | ${inline(stats.complexityRating) || 'N/A'} (${num(stats.complexity)}) |\n`; @@ -132,7 +146,7 @@ if (analysis.modifiedComponents && analysis.modifiedComponents.length > 0) { : `${esmDelta}B` : 'N/A'; - componentSection += `| Bundle Size (ESM) | ${inline(baseStats.esmSize) || 'N/A'} | ${inline(stats.esmSize) || 'N/A'} | ${esmDeltaStr} |\n`; + componentSection += `| Entry File (ESM) | ${inline(baseStats.esmSize) || 'N/A'} | ${inline(stats.esmSize) || 'N/A'} | ${esmDeltaStr} |\n`; componentSection += `| Lines of Code | ${inline(baseStats.linesOfCode) || 'N/A'} | ${inline(stats.linesOfCode) || 'N/A'} | - |\n`; componentSection += `| Complexity | ${inline(baseStats.complexityRating) || 'N/A'} | ${inline(stats.complexityRating) || 'N/A'} (${num(stats.complexity)}) | - |\n`; componentSection += `\n
\n\n`; @@ -169,7 +183,23 @@ const bundlePackages = ? [{package: '@astryxdesign/core', ...analysis.totalBundle}] : []; -if (bundlePackages.length > 0) { +const baseBundles = analysis.baseBundlePackages; +const signedBytes = delta => + delta === null ? 'N/A' : delta > 0 ? `+${delta}B` : `${delta}B`; +if (bundlePackages.length > 0 && Array.isArray(baseBundles)) { + bundleSection += `| Package | Entry (ESM) | Gzipped before | Gzipped after | Delta |\n`; + bundleSection += `|---------|-------------|----------------|---------------|-------|\n`; + for (const b of bundlePackages) { + const base = baseBundles.find(entry => entry.package === b.package) || {}; + const delta = + Number.isFinite(Number(b.gzipBytes)) && + Number.isFinite(Number(base.gzipBytes)) + ? Number(b.gzipBytes) - Number(base.gzipBytes) + : null; + bundleSection += `| ${inline(b.package)} | ${inline(b.esmSize) || 'N/A'} | ${inline(base.gzipSize) || 'N/A'} | ${inline(b.gzipSize) || 'N/A'} | ${signedBytes(delta)} |\n`; + } + bundleSection += `\n`; +} else if (bundlePackages.length > 0) { bundleSection += `| Package | Size (ESM) | Size (CJS) | Gzipped |\n`; bundleSection += `|---------|------------|------------|----------|\n`; for (const b of bundlePackages) { diff --git a/.github/scripts/generate-pr-comment.test.mjs b/.github/scripts/generate-pr-comment.test.mjs index 79b996205285d..567e657625151 100644 --- a/.github/scripts/generate-pr-comment.test.mjs +++ b/.github/scripts/generate-pr-comment.test.mjs @@ -125,3 +125,55 @@ describe('generate-pr-comment preview availability', () => { expect(stdout).not.toContain('View Sandbox for this PR'); }); }); + +describe('generate-pr-comment base sizes', () => { + const sized = { + ...baseAnalysis, + componentStats: { + Card: {package: '@astryxdesign/core', esmSize: '160B', esmBytes: 160}, + }, + bundlePackages: [ + { + package: '@astryxdesign/core', + esmSize: '900B', + gzipSize: '300B', + gzipBytes: 300, + }, + ], + }; + + it('fills Before and Delta from base stats', () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'astryx-base-stats-')); + try { + const baseFile = path.join(dir, 'size-base.json'); + fs.writeFileSync( + baseFile, + JSON.stringify({ + baseComponentStats: { + Card: { + package: '@astryxdesign/core', + esmSize: '100B', + esmBytes: 100, + }, + }, + baseBundlePackages: [ + {package: '@astryxdesign/core', gzipSize: '280B', gzipBytes: 280}, + ], + }), + ); + const out = runComment(sized, ['--base-stats', baseFile]); + expect(out).toContain('| Entry File (ESM) | 100B | 160B | +60B |'); + expect(out).toContain( + '| @astryxdesign/core | 900B | 280B | 300B | +20B |', + ); + } finally { + fs.rmSync(dir, {recursive: true, force: true}); + } + }); + + it('keeps N/A without base stats', () => { + const out = runComment(sized); + expect(out).toContain('| Entry File (ESM) | N/A | 160B | N/A |'); + expect(out).toContain('| Package | Size (ESM) | Size (CJS) | Gzipped |'); + }); +}); diff --git a/.github/scripts/lib/pr-preview.mjs b/.github/scripts/lib/pr-preview.mjs index 1af647a22100e..edaca230c33a5 100644 --- a/.github/scripts/lib/pr-preview.mjs +++ b/.github/scripts/lib/pr-preview.mjs @@ -446,6 +446,26 @@ function trustedAnalysis({analysisPath, metadataPath, identity, core}) { } } +/** + * Base measurements come from the same CI run's merge-base build. They are + * used only when they name the exact head this comment reports. + */ +function trustedBaseStats({baseStatsPath, identity, core}) { + try { + if (!fs.existsSync(baseStatsPath)) return false; + const stats = readJSON(baseStatsPath); + if ( + !FULL_SHA.test(String(stats.headSha)) || + stats.headSha !== identity.headSha + ) + refuse('base stats head does not match'); + return true; + } catch (error) { + core.warning(`Ignoring untrusted or stale base stats: ${error.message}`); + return false; + } +} + async function allComments(github, owner, repo, prNumber) { return github.paginate(github.rest.issues.listComments, { owner, @@ -462,6 +482,7 @@ export async function reconcilePrComment({ expectedIdentity, analysisPath = 'pr-analysis/analysis.json', metadataPath = 'pr-analysis/pr-meta.json', + baseStatsPath = 'pr-size-base/size-base.json', a11yPath = 'a11y/a11y-report.json', visualPath = 'trusted-visual/verdict.json', createIfMissing = true, @@ -549,6 +570,9 @@ export async function reconcilePrComment({ generator, '--analysis', analysisPath, + ...(trustedBaseStats({baseStatsPath, identity, core}) + ? ['--base-stats', baseStatsPath] + : []), '--a11y', resolvedA11yPath, ...(fs.existsSync(visualPath) ? ['--visual', visualPath] : []), diff --git a/.github/scripts/lib/pr-preview.test.mjs b/.github/scripts/lib/pr-preview.test.mjs index c40b8b03b7d6f..51baf296d93e5 100644 --- a/.github/scripts/lib/pr-preview.test.mjs +++ b/.github/scripts/lib/pr-preview.test.mjs @@ -208,8 +208,14 @@ async function reconcile({ createIfMissing, fallbackMessage, execute, + baseStats, } = {}) { const paths = fixture(value, {analysis}); + const baseStatsPath = path.join( + path.dirname(paths.analysis), + 'size-base.json', + ); + if (baseStats) fs.writeFileSync(baseStatsPath, JSON.stringify(baseStats)); const {github, state} = githubFixture({ value: githubFixtureValue, comments, @@ -225,6 +231,7 @@ async function reconcile({ expectedIdentity: value, analysisPath: paths.analysis, metadataPath: paths.metadata, + baseStatsPath, a11yPath: paths.a11y, lookupPreview: async () => (previewAvailable ? VERCEL_ORIGIN : null), probePreview: async () => true, @@ -894,6 +901,25 @@ describe('PR comment preview reconciliation', () => { expect(withPrior.state.updated).toHaveLength(1); }); + it.each([ + ['the exact head', HEAD, true], + ['another head', 'b'.repeat(40), false], + ])( + 'passes base size stats only when they name %s', + async (_label, headSha, used) => { + const seen = []; + await reconcile({ + baseStats: {headSha, baseComponentStats: {}, baseBundlePackages: []}, + execute: (_node, args) => { + seen.push(args); + return '## PR Analysis Report\n'; + }, + }); + expect(seen).toHaveLength(1); + expect(seen[0].includes('--base-stats')).toBe(used); + }, + ); + it('falls back to a safe current message when analysis rendering fails', async () => { const stale = { id: 78, diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index dfd0139b3129d..524f160d9deb9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1073,6 +1073,76 @@ jobs: pr-meta.json retention-days: 1 + # Measures the PR's merge base so the PR comment can show component and + # package sizes before and after. It runs beside build-storybook rather than + # inside it, so no browser lane waits for the second build; the head sizes + # still come from build-storybook's analysis. + size-base: + needs: [check-scope] + if: github.event_name == 'pull_request' && needs.check-scope.result == 'success' && needs.check-scope.outputs.docsite_only != 'true' && needs.check-scope.outputs.spec_only != 'true' && needs.check-scope.outputs.tooling_only != 'true' + runs-on: ubuntu-latest + permissions: + contents: read + steps: + - name: Checkout + uses: actions/checkout@v7 + with: + ref: ${{ github.event.pull_request.head.sha }} + fetch-depth: 50 + + - name: Fetch base branch + run: git fetch origin ${{ github.base_ref }} --depth=50 + + - name: Setup Node and pnpm + uses: ./.github/actions/setup + + - name: Check out the merge base + id: merge-base + env: + BASE_REF: ${{ github.base_ref }} + run: | + if ! BASE_SHA=$(git merge-base "origin/$BASE_REF" HEAD); then + git fetch --deepen=200 origin "$BASE_REF:refs/remotes/origin/$BASE_REF" + BASE_SHA=$(git merge-base "origin/$BASE_REF" HEAD) + fi + git worktree add --detach .size-base "$BASE_SHA" + echo "sha=$BASE_SHA" >> "$GITHUB_OUTPUT" + + # Core and its build tooling always build, because the other component + # packages depend on them; another package builds only when the PR + # changes its source. + - name: Build the merge base's component packages + working-directory: .size-base + env: + BASE_SHA: ${{ steps.merge-base.outputs.sha }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + run: | + pnpm install --frozen-lockfile + pnpm -F @astryxdesign/build build + pnpm -F @astryxdesign/core build + for pkg in lab charts richtext vega; do + if git diff --quiet "$BASE_SHA" "$HEAD_SHA" -- "packages/$pkg/src"; then + continue + fi + pnpm -F "@astryxdesign/$pkg" build + done + + - name: Measure the merge base + run: | + node .github/scripts/analyze-pr.js \ + --base origin/${{ github.base_ref }} \ + --head HEAD \ + --base-root .size-base \ + --base-only \ + --output size-base.json + + - name: Upload base size stats + uses: actions/upload-artifact@v7 + with: + name: pr-size-base + path: size-base.json + retention-days: 1 + # Typechecks run beside build-storybook instead of before its Storybook # build, so the browser checks that wait on the Storybook artifact start # ~1.5 minutes sooner. The build and release-check joins require this job, diff --git a/.github/workflows/pr-comment.yml b/.github/workflows/pr-comment.yml index 532a200ccf581..ada3a83bc6288 100644 --- a/.github/workflows/pr-comment.yml +++ b/.github/workflows/pr-comment.yml @@ -393,6 +393,16 @@ jobs: github-token: ${{ secrets.GITHUB_TOKEN }} continue-on-error: true + - name: Download base size stats from the CI run + if: steps.identity.outputs.valid == 'true' + uses: actions/download-artifact@v8 + with: + name: pr-size-base + path: pr-size-base + run-id: ${{ github.event.workflow_run.id }} + github-token: ${{ secrets.GITHUB_TOKEN }} + continue-on-error: true + - name: Download a11y artifact from the CI run if: steps.identity.outputs.valid == 'true' uses: actions/download-artifact@v8 From 38fed66e10e0677b705d1e726ce735f350cadc3b Mon Sep 17 00:00:00 2001 From: Cindy Zhang Date: Sat, 24 Oct 2026 11:39:56 -0700 Subject: [PATCH 2/3] fix(ci): no delta from an unmeasured size; skip base measurement without a merge base; label entry-file gzip --- .github/scripts/generate-pr-comment.js | 28 +++++++----- .github/scripts/generate-pr-comment.test.mjs | 48 ++++++++++++++++++++ .github/workflows/ci.yml | 15 +++++- 3 files changed, 77 insertions(+), 14 deletions(-) diff --git a/.github/scripts/generate-pr-comment.js b/.github/scripts/generate-pr-comment.js index 8004550e5f9fd..225da03e204d5 100644 --- a/.github/scripts/generate-pr-comment.js +++ b/.github/scripts/generate-pr-comment.js @@ -86,6 +86,17 @@ function extLink(text, url) { return `${text}`; } +// Difference of two measured byte counts, or null when either side was not +// measured. A missing size is never treated as 0. +function bytesDelta(after, before) { + return typeof after === 'number' && + Number.isFinite(after) && + typeof before === 'number' && + Number.isFinite(before) + ? after - before + : null; +} + // Build component stats section let componentSection = ''; if (analysis.newComponents && analysis.newComponents.length > 0) { @@ -134,11 +145,7 @@ if (analysis.modifiedComponents && analysis.modifiedComponents.length > 0) { componentSection += `
\n${inline(comp)}${pkgBadge}${sbBadge}${newBadge}\n\n`; componentSection += `| Metric | Before | After | Delta |\n|--------|--------|-------|-------|\n`; - const esmDelta = - Number.isFinite(Number(stats.esmBytes)) && - Number.isFinite(Number(baseStats.esmBytes)) - ? Number(stats.esmBytes) - Number(baseStats.esmBytes) - : null; + const esmDelta = bytesDelta(stats.esmBytes, baseStats.esmBytes); const esmDeltaStr = esmDelta !== null ? esmDelta > 0 @@ -187,15 +194,12 @@ const baseBundles = analysis.baseBundlePackages; const signedBytes = delta => delta === null ? 'N/A' : delta > 0 ? `+${delta}B` : `${delta}B`; if (bundlePackages.length > 0 && Array.isArray(baseBundles)) { - bundleSection += `| Package | Entry (ESM) | Gzipped before | Gzipped after | Delta |\n`; - bundleSection += `|---------|-------------|----------------|---------------|-------|\n`; + bundleSection += `Gzip figures measure each package's entry file, not the code an import pulls in.\n\n`; + bundleSection += `| Package | Entry (ESM) | Entry gzip before | Entry gzip after | Delta |\n`; + bundleSection += `|---------|-------------|-------------------|------------------|-------|\n`; for (const b of bundlePackages) { const base = baseBundles.find(entry => entry.package === b.package) || {}; - const delta = - Number.isFinite(Number(b.gzipBytes)) && - Number.isFinite(Number(base.gzipBytes)) - ? Number(b.gzipBytes) - Number(base.gzipBytes) - : null; + const delta = bytesDelta(b.gzipBytes, base.gzipBytes); bundleSection += `| ${inline(b.package)} | ${inline(b.esmSize) || 'N/A'} | ${inline(base.gzipSize) || 'N/A'} | ${inline(b.gzipSize) || 'N/A'} | ${signedBytes(delta)} |\n`; } bundleSection += `\n`; diff --git a/.github/scripts/generate-pr-comment.test.mjs b/.github/scripts/generate-pr-comment.test.mjs index 567e657625151..0ce8d5a3f7024 100644 --- a/.github/scripts/generate-pr-comment.test.mjs +++ b/.github/scripts/generate-pr-comment.test.mjs @@ -171,6 +171,54 @@ describe('generate-pr-comment base sizes', () => { } }); + it('never reports a delta for an unmeasured size', () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'astryx-base-null-')); + try { + const baseFile = path.join(dir, 'size-base.json'); + fs.writeFileSync( + baseFile, + JSON.stringify({ + baseComponentStats: { + Card: { + package: '@astryxdesign/charts', + esmSize: null, + esmBytes: null, + }, + }, + baseBundlePackages: [ + {package: '@astryxdesign/core', gzipSize: null, gzipBytes: null}, + ], + }), + ); + const out = runComment( + { + ...sized, + componentStats: { + Card: { + package: '@astryxdesign/charts', + esmSize: null, + esmBytes: null, + }, + }, + bundlePackages: [ + { + package: '@astryxdesign/core', + esmSize: '900B', + gzipSize: '300B', + gzipBytes: 300, + }, + ], + }, + ['--base-stats', baseFile], + ); + expect(out).toContain('| Entry File (ESM) | N/A | N/A | N/A |'); + expect(out).toContain('| @astryxdesign/core | 900B | N/A | 300B | N/A |'); + expect(out).not.toMatch(/\| [+-]?0B \|/); + } finally { + fs.rmSync(dir, {recursive: true, force: true}); + } + }); + it('keeps N/A without base stats', () => { const out = runComment(sized); expect(out).toContain('| Entry File (ESM) | N/A | 160B | N/A |'); diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 524f160d9deb9..2fbc9ddd063b3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1096,22 +1096,31 @@ jobs: - name: Setup Node and pnpm uses: ./.github/actions/setup + # A branch far behind main may have no merge base within the fetched + # history even after deepening. The report then keeps Before and Delta + # as N/A rather than failing the run. - name: Check out the merge base id: merge-base env: BASE_REF: ${{ github.base_ref }} run: | if ! BASE_SHA=$(git merge-base "origin/$BASE_REF" HEAD); then - git fetch --deepen=200 origin "$BASE_REF:refs/remotes/origin/$BASE_REF" - BASE_SHA=$(git merge-base "origin/$BASE_REF" HEAD) + git fetch --deepen=200 origin "$BASE_REF:refs/remotes/origin/$BASE_REF" || true + if ! BASE_SHA=$(git merge-base "origin/$BASE_REF" HEAD); then + echo "::warning::no merge base within the fetched history; skipping base size measurement" + echo "found=false" >> "$GITHUB_OUTPUT" + exit 0 + fi fi git worktree add --detach .size-base "$BASE_SHA" echo "sha=$BASE_SHA" >> "$GITHUB_OUTPUT" + echo "found=true" >> "$GITHUB_OUTPUT" # Core and its build tooling always build, because the other component # packages depend on them; another package builds only when the PR # changes its source. - name: Build the merge base's component packages + if: steps.merge-base.outputs.found == 'true' working-directory: .size-base env: BASE_SHA: ${{ steps.merge-base.outputs.sha }} @@ -1128,6 +1137,7 @@ jobs: done - name: Measure the merge base + if: steps.merge-base.outputs.found == 'true' run: | node .github/scripts/analyze-pr.js \ --base origin/${{ github.base_ref }} \ @@ -1137,6 +1147,7 @@ jobs: --output size-base.json - name: Upload base size stats + if: steps.merge-base.outputs.found == 'true' uses: actions/upload-artifact@v7 with: name: pr-size-base From a5e6e5fd334723d880db7f3cb4c9667da7979238 Mon Sep 17 00:00:00 2001 From: Cindy Zhang Date: Sat, 24 Oct 2026 12:19:25 -0700 Subject: [PATCH 3/3] test(ci): the size-base job stays off the Node-tooling lane --- .github/scripts/ci-tooling-routing.test.mjs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/scripts/ci-tooling-routing.test.mjs b/.github/scripts/ci-tooling-routing.test.mjs index 6205b00fba9cb..5561eb4f5add6 100644 --- a/.github/scripts/ci-tooling-routing.test.mjs +++ b/.github/scripts/ci-tooling-routing.test.mjs @@ -108,6 +108,9 @@ describe('Node-tooling CI routing', () => { step(ci.jobs['test-build'], 'Run Build CSS-layer cascade guard').run, ).toBe('node .github/scripts/theme-layer-cascade.js'); expect(ci.jobs['fixture-contrast'].if).toContain(TOOLING_FALSE); + // The merge-base size build is heavy build work and stays off the + // Node-tooling, docsite, and spec lanes. + expect(ci.jobs['size-base'].if).toContain(BROAD_ONLY); const familyCommands = new Map([ [ 'Install Playwright for theme-family contract',