diff --git a/.github/scripts/analyze-pr.js b/.github/scripts/analyze-pr.js index 49fe3b0f704b2..ee2365b36f56d 100644 --- a/.github/scripts/analyze-pr.js +++ b/.github/scripts/analyze-pr.js @@ -14,6 +14,12 @@ const fs = require('node:fs'); const path = require('node:path'); const { execSync } = require('node:child_process'); +const { + gzipBytes: gzipTextBytes, + measureComponentImport, + bundleInputsByEntry, + packageStylesheetPath, +} = require('./lib/component-size'); const { COMPONENT_PACKAGES, componentExportsFromBarrel, @@ -194,7 +200,17 @@ function componentExistsInBase(pkg, componentName) { }); return true; } catch { - return false; + // A nested component folder without an index barrel still exists in base. + if (pkg.layout === 'flat') return false; + try { + const listing = execSync( + `git ls-tree --name-only ${baseBranch} -- ${pkgSrc(pkg)}/${componentName}/`, + { encoding: 'utf8', stdio: 'pipe' }, + ); + return listing.trim().length > 0; + } catch { + return false; + } } } @@ -500,6 +516,113 @@ function getComplexity(pkg, componentName) { } } +// A package's prebuilt stylesheet text, read once per tree. +const stylesheetCache = new Map(); +function packageStylesheet(pkg) { + const file = packageStylesheetPath(path.join(process.cwd(), pkg.dir)); + if (!file) return null; + if (!stylesheetCache.has(file)) stylesheetCache.set(file, fs.readFileSync(file, 'utf8')); + return stylesheetCache.get(file); +} + +// Every canonical public component of a package, with the dist module that +// defines its export. +function packageTargets(pkg) { + if (pkg.layout === 'flat') { + return getComponentNames(pkg).map((name) => ({ + key: `${pkg.packageName}/${name}`, + pkg, + name, + distModule: `${name}.js`, + })); + } + const sourceDir = path.join(process.cwd(), pkgSrc(pkg)); + const exports = componentExportsFromBarrel(path.join(sourceDir, 'index.ts'), sourceDir); + const targets = []; + for (const name of nestedPackageComponentNames(process.cwd(), pkg)) { + const file = exports.get(name); + if (!file) continue; + targets.push({ + key: `${pkg.packageName}/${name}`, + pkg, + name, + distModule: path.relative(sourceDir, file).replace(/\.tsx?$/, '.js'), + }); + } + return targets; +} + +// Every canonical public component a change can affect: its own folder or +// file changed, or its bundle, import() targets included, reaches a changed +// module of its package, such as a shared hook or style module. Family folders +// that hold several components yield each of them. +function sizeTargets(changedFiles) { + const withoutExtension = (file) => file.replace(/\.(tsx?|jsx?|mjs)$/, ''); + const targets = []; + for (const pkg of PACKAGES) { + const prefix = pkgSrc(pkg) + '/'; + const changedModules = new Set( + changedFiles + .filter((file) => file.startsWith(prefix)) + .map((file) => withoutExtension(file.slice(prefix.length))), + ); + if (changedModules.size === 0) continue; + const changedTops = new Set([...changedModules].map((file) => file.split('/')[0])); + const dist = path.join(process.cwd(), pkgDist(pkg)); + // esbuild reports inputs by real path. + const distReal = fs.existsSync(dist) ? fs.realpathSync(dist) : dist; + const all = packageTargets(pkg); + const reach = bundleInputsByEntry(all.map((target) => path.join(dist, target.distModule))); + for (const target of all) { + const top = withoutExtension(target.distModule.split('/')[0]); + const inputs = reach?.get(path.join(dist, target.distModule)) ?? []; + const reachesChange = inputs.some((input) => { + const relative = path.relative(distReal, input); + return !relative.startsWith('..') && changedModules.has(withoutExtension(relative)); + }); + if (changedTops.has(top) || reachesChange) targets.push(target); + } + } + return targets; +} + +// Targets from either tree, once each by key. +function unionTargets(first, second) { + const byKey = new Map(first.map((target) => [target.key, target])); + for (const target of second) if (!byKey.has(target.key)) byKey.set(target.key, target); + return [...byKey.values()]; +} + +// What each target's import costs in the current tree: the gzip JavaScript it +// loads eagerly, and the gzip CSS of the package stylesheet rules referenced by +// its own package's modules it can load, import() targets included (spec:AST-080 FR8, +// architecture:component-style-authoring INV6). +function measureSizes(targets) { + const sizes = {}; + for (const target of targets) { + const dist = path.join(process.cwd(), pkgDist(target.pkg)); + let size = measureComponentImport( + path.join(dist, target.distModule), + packageStylesheet(target.pkg), + dist, + target.name, + ); + // An alias (`export {A as B}`) is not exported by its defining module; + // measure it through the package entry instead. + if (size.importGzipBytes === null) { + const {esmPath} = resolveEntries(dist, target.pkg); + size = measureComponentImport( + esmPath, + packageStylesheet(target.pkg), + dist, + target.name, + ); + } + sizes[target.key] = {package: target.pkg.name, ...size}; + } + return sizes; +} + // Get component stats function getComponentStats(pkg, componentName) { // Per-component dist sizing only applies to nested packages that emit a @@ -512,7 +635,6 @@ function getComponentStats(pkg, componentName) { const entries = distDir ? resolveEntries(distDir, pkg) : { esmPath: null, cjsPath: null }; const loc = getComponentLOC(pkg, componentName); const complexity = getComplexity(pkg, componentName); - return { package: pkg.name, esmSize: getFileSize(entries.esmPath), @@ -545,6 +667,10 @@ function getPackageBundleStats(pkg) { cjsBytes: getFileSizeBytes(cjsPath), gzipSize: formatBytes(gzipBytes), gzipBytes, + stylesheetGzipBytes: (() => { + const css = packageStylesheet(pkg); + return css === null ? null : gzipTextBytes(css); + })(), }; } @@ -655,6 +781,12 @@ function analyze() { // and package lists stay the head's; only the stats are read in the base tree. let baseComponentStats = null; let baseBundlePackages = null; + // Components a change can affect, by reach in the head build and, when the + // merge base is built, in the base build too: CI's size-base job builds only + // the base, and a module change can add or remove reach. + const headTargets = sizeTargets(changedFiles); + let targets = headTargets; + let baseComponentSizes = null; if (baseRoot) { const headDir = process.cwd(); process.chdir(path.resolve(headDir, baseRoot)); @@ -667,10 +799,16 @@ function analyze() { baseBundlePackages = PACKAGES.filter((p) => changedPackages.has(p.name)).map((p) => getPackageBundleStats(p), ); + // Each tree is measured through its own defining modules, so a renamed + // module is not mistaken for a missing one. + const baseTargets = sizeTargets(changedFiles); + baseComponentSizes = measureSizes(unionTargets(baseTargets, headTargets)); + targets = unionTargets(headTargets, baseTargets); } finally { process.chdir(headDir); } } + const componentSizes = baseOnly ? null : measureSizes(targets); if (baseOnly) { const baseResult = { @@ -678,6 +816,7 @@ function analyze() { baseSha: execSync('git rev-parse HEAD', { cwd: baseRoot, encoding: 'utf8' }).trim(), baseComponentStats, baseBundlePackages, + baseComponentSizes, analyzedAt: new Date().toISOString(), }; fs.writeFileSync(outputFile, JSON.stringify(baseResult, null, 2)); @@ -701,7 +840,8 @@ function analyze() { // or render it in the report should caveat when this is 'two-dot'. diffMode, bundlePackages, - ...(baseRoot ? { baseComponentStats, baseBundlePackages } : {}), + componentSizes, + ...(baseRoot ? { baseComponentStats, baseBundlePackages, baseComponentSizes } : {}), // 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 df62f458587bf..7f9e433add79a 100644 --- a/.github/scripts/analyze-pr.test.mjs +++ b/.github/scripts/analyze-pr.test.mjs @@ -281,6 +281,138 @@ describe('analyze-pr component ownership', () => { }); describe('analyze-pr base and head sizes', () => { + it('sizes a component whose bundle reaches a changed shared module', () => { + const repo = fs.mkdtempSync(path.join(os.tmpdir(), 'astryx-analyze-reach-')); + const write = (file, content) => { + fs.mkdirSync(path.dirname(path.join(repo, file)), {recursive: true}); + fs.writeFileSync(path.join(repo, file), content); + }; + try { + write('package.json', '{}'); + write('.gitignore', 'dist/\n*.json.out\nanalysis.json\n'); + write('packages/core/package.json', '{"type": "module"}\n'); + write('packages/core/src/index.ts', "export {Card} from './Card';\nexport {Badge} from './Badge';\n"); + write('packages/core/src/Card/index.ts', "export {Card} from './Card';\n"); + write('packages/core/src/Card/Card.tsx', "import {useHold} from '../hooks/useHold';\nexport function Card() { return useHold(); }\n"); + write('packages/core/src/Card/Card.doc.mjs', "export const docs = {\n name: 'Card',\n};\n"); + write('packages/core/src/Badge/index.ts', "export {Badge} from './Badge';\n"); + write('packages/core/src/Badge/Badge.tsx', 'export function Badge() {}\n'); + write('packages/core/src/Badge/Badge.doc.mjs', "export const docs = {\n name: 'Badge',\n};\n"); + write('packages/core/src/hooks/useHold.ts', 'export function useHold() { return 1; }\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, ['checkout', '-q', '-b', 'feature']); + write('packages/core/src/hooks/useHold.ts', 'export function useHold() { return 2; }\n'); + git(repo, ['commit', '-qam', 'touch the shared hook only']); + write('packages/core/dist/Card/Card.js', "import {useHold} from '../hooks/useHold.js';\nexport function Card() { return useHold(); }\n"); + write('packages/core/dist/Badge/Badge.js', 'export function Badge() { return 0; }\n'); + write('packages/core/dist/hooks/useHold.js', 'export function useHold() { return 2; }\n'); + write('packages/core/dist/index.js', "export {Card} from './Card/Card.js';\nexport {Badge} from './Badge/Badge.js';\n"); + + execFileSync( + process.execPath, + [SCRIPT, '--base', 'main', '--head', 'HEAD', '--output', 'analysis.json'], + {cwd: repo, encoding: 'utf8'}, + ); + const analysis = JSON.parse(fs.readFileSync(path.join(repo, 'analysis.json'), 'utf8')); + expect(Object.keys(analysis.componentSizes)).toEqual(['core/Card']); + } finally { + fs.rmSync(repo, {recursive: true, force: true}); + } + }); + + it('decides reach in the merge-base build when only the base is built', () => { + const repo = fs.mkdtempSync(path.join(os.tmpdir(), 'astryx-analyze-basereach-')); + 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/\nbase.json\n'); + write(repo, 'packages/core/package.json', '{"type": "module"}\n'); + write(repo, 'packages/core/src/index.ts', "export {Card} from './Card';\nexport {Badge} from './Badge';\n"); + write(repo, 'packages/core/src/Card/index.ts', "export {Card} from './Card';\n"); + write(repo, 'packages/core/src/Card/Card.tsx', "import {useHold} from '../hooks/useHold';\nexport function Card() { return useHold(); }\n"); + write(repo, 'packages/core/src/Card/Card.doc.mjs', "export const docs = {\n name: 'Card',\n};\n"); + write(repo, 'packages/core/src/Badge/index.ts', "export {Badge} from './Badge';\n"); + write(repo, 'packages/core/src/Badge/Badge.tsx', 'export function Badge() {}\n'); + write(repo, 'packages/core/src/Badge/Badge.doc.mjs', "export const docs = {\n name: 'Badge',\n};\n"); + write(repo, 'packages/core/src/hooks/useHold.ts', 'export function useHold() { return 1; }\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']); + write(repo, 'packages/core/src/hooks/useHold.ts', 'export function useHold() { return 2; }\n'); + git(repo, ['commit', '-qam', 'touch the shared hook only']); + // Only the merge base is built, as in the size-base job. + write(baseRoot, 'packages/core/dist/Card/Card.js', "import {useHold} from '../hooks/useHold.js';\nexport function Card() { return useHold(); }\n"); + write(baseRoot, 'packages/core/dist/Badge/Badge.js', 'export function Badge() { return 0; }\n'); + write(baseRoot, 'packages/core/dist/hooks/useHold.js', 'export function useHold() { return 1; }\n'); + write(baseRoot, 'packages/core/dist/index.js', "export {Card} from './Card/Card.js';\nexport {Badge} from './Badge/Badge.js';\n"); + + execFileSync( + process.execPath, + [SCRIPT, '--base', 'main', '--head', 'HEAD', '--base-root', '.size-base', '--base-only', '--output', 'base.json'], + {cwd: repo, encoding: 'utf8'}, + ); + const base = JSON.parse(fs.readFileSync(path.join(repo, 'base.json'), 'utf8')); + expect(Object.keys(base.baseComponentSizes)).toEqual(['core/Card']); + expect(base.baseComponentSizes['core/Card'].importGzipBytes).toBeGreaterThan(0); + } finally { + fs.rmSync(repo, {recursive: true, force: true}); + } + }); + + it('measures the merge base through its own defining module when the pull request renames it', () => { + const repo = fs.mkdtempSync(path.join(os.tmpdir(), 'astryx-analyze-rename-')); + 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/\nbase.json\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']); + git(repo, ['mv', 'packages/core/src/Card/Card.tsx', 'packages/core/src/Card/CardView.tsx']); + write(repo, 'packages/core/src/Card/index.ts', "export {Card} from './CardView';\n"); + git(repo, ['commit', '-qam', 'rename the defining module']); + // Only the merge base is built; its package entry does not export Card, + // so a figure proves the defining module was measured. + write(baseRoot, 'packages/core/dist/Card/Card.js', 'export function Card() { return 1; }\n'); + write(baseRoot, 'packages/core/dist/index.js', 'export const other = 1;\n'); + + execFileSync( + process.execPath, + [SCRIPT, '--base', 'main', '--head', 'HEAD', '--base-root', '.size-base', '--base-only', '--output', 'base.json'], + {cwd: repo, encoding: 'utf8'}, + ); + const base = JSON.parse(fs.readFileSync(path.join(repo, 'base.json'), 'utf8')); + expect(base.baseComponentSizes['core/Card'].importGzipBytes).toBeGreaterThan(0); + } finally { + fs.rmSync(repo, {recursive: true, force: true}); + } + }); + 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'); @@ -295,7 +427,20 @@ describe('analyze-pr base and head sizes', () => { write( repo, 'packages/core/src/index.ts', - "export {Card} from './Card';\n", + "export {Card} from './Card';\nexport {Chat, ChatComposer} from './Chat';\n", + ); + write( + repo, + 'packages/core/src/Chat/index.ts', + "export {Chat} from './Chat';\nexport {ChatComposer} from './ChatComposer';\n", + ); + write(repo, 'packages/core/src/Chat/Chat.tsx', 'export function Chat() {}\n'); + write(repo, 'packages/core/src/Chat/ChatComposer.tsx', 'export function ChatComposer() {}\n'); + write(repo, 'packages/core/src/Chat/Chat.doc.mjs', "export const docs = {\n name: 'Chat',\n};\n"); + write( + repo, + 'packages/core/src/Chat/ChatComposer.doc.mjs', + "export const docs = {\n name: 'ChatComposer',\n};\n", ); write( repo, @@ -323,7 +468,11 @@ describe('analyze-pr base and head sizes', () => { path.join(repo, 'packages/core/src/Card/Card.tsx'), 'export const changed = true;\n', ); - git(repo, ['commit', '-qam', 'touch Card']); + fs.appendFileSync( + path.join(repo, 'packages/core/src/Chat/ChatComposer.tsx'), + 'export const changed = true;\n', + ); + git(repo, ['commit', '-qam', 'touch Card and the Chat family']); // Babel writes ES modules as `.js` in a "type": "module" package. write(baseRoot, 'packages/core/dist/Card/index.js', 'x'.repeat(100)); @@ -333,6 +482,19 @@ describe('analyze-pr base and head sizes', () => { 'export const base = 1;\n'.repeat(10), ); write(repo, 'packages/core/dist/Card/index.js', 'x'.repeat(160)); + write(baseRoot, 'packages/core/dist/Card/Card.js', 'export function Card() {}\n'); + write( + repo, + 'packages/core/dist/Card/Card.js', + 'export function Card() { return "a longer head body"; }\nexport const changed = true;\n', + ); + write(baseRoot, 'packages/core/dist/Chat/Chat.js', 'export function Chat() {}\n'); + write(repo, 'packages/core/dist/Chat/Chat.js', 'export function Chat() {}\n'); + write( + repo, + 'packages/core/dist/Chat/ChatComposer.js', + 'export function ChatComposer() { return "new"; }\n', + ); write( repo, 'packages/core/dist/index.js', @@ -360,6 +522,18 @@ describe('analyze-pr base and head sizes', () => { expect(analysis.componentStats.Card.esmBytes).toBe(160); expect(analysis.componentStats.Card.cjsBytes).toBeNull(); expect(analysis.baseComponentStats.Card.esmBytes).toBe(100); + // Every public component in a changed folder is sized, family folders + // included, at head and at the merge base. + expect(Object.keys(analysis.componentSizes).sort()).toEqual([ + 'core/Card', + 'core/Chat', + 'core/ChatComposer', + ]); + expect(analysis.componentSizes['core/Card'].importGzipBytes).toBeGreaterThan( + analysis.baseComponentSizes['core/Card'].importGzipBytes, + ); + expect(analysis.componentSizes['core/ChatComposer'].importGzipBytes).toBeGreaterThan(0); + expect(analysis.baseComponentSizes['core/ChatComposer'].importGzipBytes).toBeNull(); const [headBundle] = analysis.bundlePackages; const [baseBundle] = analysis.baseBundlePackages; expect(headBundle.package).toBe('@astryxdesign/core'); @@ -390,6 +564,7 @@ describe('analyze-pr base and head sizes', () => { expect(baseOnly.baseSha).toBe(git(repo, ['rev-parse', 'main'])); expect(baseOnly.baseComponentStats.Card.esmBytes).toBe(100); expect(baseOnly.componentStats).toBeUndefined(); + expect(baseOnly.baseComponentSizes['core/Chat'].importGzipBytes).toBeGreaterThan(0); } 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 225da03e204d5..a5b87c81c4185 100644 --- a/.github/scripts/generate-pr-comment.js +++ b/.github/scripts/generate-pr-comment.js @@ -52,6 +52,7 @@ if (baseStatsFile) { const base = JSON.parse(fs.readFileSync(baseStatsFile, 'utf8')); analysis.baseComponentStats = base.baseComponentStats || undefined; analysis.baseBundlePackages = base.baseBundlePackages || undefined; + analysis.baseComponentSizes = base.baseComponentSizes || undefined; } catch (e) { console.error('Warning: Could not read base stats:', e.message); } @@ -97,6 +98,26 @@ function bytesDelta(after, before) { : null; } +// A measured byte count as text, or N/A. Only numbers reach the output. +function sizeText(bytes) { + if (typeof bytes !== 'number' || !Number.isFinite(bytes)) return 'N/A'; + if (Math.abs(bytes) < 1024) return `${bytes}B`; + return `${(bytes / 1024).toFixed(1)}KB`; +} + +function signedSizeText(delta) { + if (delta === null) return 'N/A'; + return delta > 0 ? `+${sizeText(delta)}` : sizeText(delta); +} + +// Touch-only budget per component (architecture:component-style-authoring +// INV6, spec:AST-080 FR8): 2 KB gzip JavaScript, 1 KB gzip CSS. The report +// cannot tell whether growth runs only on a coarse pointer, so it flags any +// growth over budget and asks the author. +const JS_BUDGET_BYTES = 2 * 1024; +const CSS_BUDGET_BYTES = 1024; +const budgetFlags = []; + // Build component stats section let componentSection = ''; if (analysis.newComponents && analysis.newComponents.length > 0) { @@ -160,6 +181,52 @@ if (analysis.modifiedComponents && analysis.modifiedComponents.length > 0) { } } +// Import and CSS size of every public component whose implementation changed, +// family folders included, before and after. +let sizeSection = ''; +const componentSizes = analysis.componentSizes || {}; +const baseComponentSizes = analysis.baseComponentSizes || {}; +const sizeKeys = [ + ...new Set([ + ...Object.keys(componentSizes), + ...Object.keys(baseComponentSizes), + ]), +].sort(); +if (sizeKeys.length > 0) { + sizeSection += '### Component Size\n\n'; + sizeSection += + '| Component | Import before | Import after | Delta | CSS before | CSS after | Delta |\n'; + sizeSection += + '|-----------|---------------|--------------|-------|------------|-----------|-------|\n'; + for (const key of sizeKeys) { + const head = componentSizes[key] || {}; + const base = baseComponentSizes[key] || {}; + const importDelta = bytesDelta(head.importGzipBytes, base.importGzipBytes); + const cssDelta = bytesDelta(head.cssGzipBytes, base.cssGzipBytes); + const jsOver = importDelta !== null && importDelta > JS_BUDGET_BYTES; + const cssOver = cssDelta !== null && cssDelta > CSS_BUDGET_BYTES; + if (jsOver || cssOver) + budgetFlags.push({comp: key, importDelta, cssDelta, jsOver, cssOver}); + sizeSection += `| ${inline(key)} | ${sizeText(base.importGzipBytes)} | ${sizeText(head.importGzipBytes)} | ${signedSizeText(importDelta)}${jsOver ? ' ⚠️' : ''} | ${sizeText(base.cssGzipBytes)} | ${sizeText(head.cssGzipBytes)} | ${signedSizeText(cssDelta)}${cssOver ? ' ⚠️' : ''} |\n`; + } + sizeSection += + '\nSizes are gzip. Import counts the JavaScript the import loads eagerly; CSS counts the package stylesheet rules referenced by the modules it can load from its own package, lazily loaded ones included.\n\n'; +} + +// Components whose growth passes the touch-only budget. +let budgetSection = ''; +if (budgetFlags.length > 0) { + budgetSection += '### ⚠️ Touch-only size budget\n\n'; + budgetSection += + 'These components grew past the touch-only budget (2 KB gzip JavaScript or 1 KB gzip CSS per component). If the growth runs only on a coarse pointer, load its JavaScript lazily so the import does not carry it, or justify the cost in the pull request description; CSS stays in the package stylesheet, so justify it.\n\n'; + budgetSection += + '| Component | JavaScript | CSS |\n|-----------|------------|-----|\n'; + for (const flag of budgetFlags) { + budgetSection += `| ${inline(flag.comp)} | ${signedSizeText(flag.importDelta)}${flag.jsOver ? ' ⚠️' : ''} | ${signedSizeText(flag.cssDelta)}${flag.cssOver ? ' ⚠️' : ''} |\n`; + } + budgetSection += '\n'; +} + // Build accessibility section using shared module const a11ySection = buildA11ySection(a11yReport); @@ -203,6 +270,19 @@ if (bundlePackages.length > 0 && Array.isArray(baseBundles)) { bundleSection += `| ${inline(b.package)} | ${inline(b.esmSize) || 'N/A'} | ${inline(base.gzipSize) || 'N/A'} | ${inline(b.gzipSize) || 'N/A'} | ${signedBytes(delta)} |\n`; } bundleSection += `\n`; + const sheets = bundlePackages.filter( + b => typeof b.stylesheetGzipBytes === 'number', + ); + if (sheets.length > 0) { + bundleSection += `| Package stylesheet | Gzip before | Gzip after | Delta |\n`; + bundleSection += `|--------------------|-------------|------------|-------|\n`; + for (const b of sheets) { + const base = baseBundles.find(entry => entry.package === b.package) || {}; + const delta = bytesDelta(b.stylesheetGzipBytes, base.stylesheetGzipBytes); + bundleSection += `| ${inline(b.package)} | ${sizeText(base.stylesheetGzipBytes)} | ${sizeText(b.stylesheetGzipBytes)} | ${signedSizeText(delta)} |\n`; + } + bundleSection += `\n`; + } } else if (bundlePackages.length > 0) { bundleSection += `| Package | Size (ESM) | Size (CJS) | Gzipped |\n`; bundleSection += `|---------|------------|------------|----------|\n`; @@ -270,8 +350,8 @@ const diffModeCaveat = const body = `## PR Analysis Report -${diffModeCaveat}${previewAvailabilitySection}${storybookSection}${sandboxSection}${componentSection || '_No new or modified components detected._\n\n'} -${bundleSection} +${diffModeCaveat}${previewAvailabilitySection}${storybookSection}${sandboxSection}${componentSection || (sizeSection ? '' : '_No new or modified components detected._\n\n')} +${sizeSection}${budgetSection}${bundleSection} ${a11ySection} ${visualSection}--- diff --git a/.github/scripts/generate-pr-comment.test.mjs b/.github/scripts/generate-pr-comment.test.mjs index 0ce8d5a3f7024..c1b02c57ce48e 100644 --- a/.github/scripts/generate-pr-comment.test.mjs +++ b/.github/scripts/generate-pr-comment.test.mjs @@ -225,3 +225,84 @@ describe('generate-pr-comment base sizes', () => { expect(out).toContain('| Package | Size (ESM) | Size (CJS) | Gzipped |'); }); }); + +describe('generate-pr-comment touch-only budget', () => { + function withBase(head, base) { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'astryx-budget-')); + try { + const baseFile = path.join(dir, 'size-base.json'); + fs.writeFileSync( + baseFile, + JSON.stringify({ + baseComponentSizes: {'core/BottomSheet': base}, + baseBundlePackages: [], + }), + ); + return runComment( + { + ...baseAnalysis, + modifiedComponents: [], + componentSizes: {'core/BottomSheet': head}, + bundlePackages: [], + }, + ['--base-stats', baseFile], + ); + } finally { + fs.rmSync(dir, {recursive: true, force: true}); + } + } + + it('flags JavaScript growth over 2 KB and CSS growth over 1 KB, family folders included', () => { + const out = withBase( + { + package: '@astryxdesign/core', + importGzipBytes: 10000, + cssGzipBytes: 3000, + }, + { + package: '@astryxdesign/core', + importGzipBytes: 7000, + cssGzipBytes: 1900, + }, + ); + expect(out).toContain('### ⚠️ Touch-only size budget'); + expect(out).toContain( + '| core/BottomSheet | 6.8KB | 9.8KB | +2.9KB ⚠️ | 1.9KB | 2.9KB | +1.1KB ⚠️ |', + ); + }); + + it('does not flag growth within budget', () => { + const out = withBase( + { + package: '@astryxdesign/core', + importGzipBytes: 9000, + cssGzipBytes: 2900, + }, + { + package: '@astryxdesign/core', + importGzipBytes: 7000, + cssGzipBytes: 1900, + }, + ); + expect(out).not.toContain('Touch-only size budget'); + expect(out).toContain( + '| core/BottomSheet | 6.8KB | 8.8KB | +2.0KB | 1.9KB | 2.8KB | +1000B |', + ); + }); + + it('does not flag a component that is new at head', () => { + const out = withBase( + { + package: '@astryxdesign/core', + importGzipBytes: 90000, + cssGzipBytes: 9000, + }, + { + package: '@astryxdesign/core', + importGzipBytes: null, + cssGzipBytes: null, + }, + ); + expect(out).not.toContain('Touch-only size budget'); + }); +}); diff --git a/.github/scripts/lib/component-size.js b/.github/scripts/lib/component-size.js new file mode 100644 index 0000000000000..622931e8e3a20 --- /dev/null +++ b/.github/scripts/lib/component-size.js @@ -0,0 +1,330 @@ +// Copyright (c) Meta Platforms, Inc. and affiliates. + +/** + * @file component-size.js + * @input A built package's dist directory, a component's dist entry, and the + * package's prebuilt stylesheet + * @output The gzip size of the JavaScript a component's import loads eagerly + * (the minified entry chunk and the chunks it imports statically, package + * imports included) and the gzip size of the stylesheet rules referenced by + * the modules it can load, import() targets included + * @position Shared by analyze-pr.js for the PR size report. Sizes are gzip at + * the default compression level (spec:AST-080 FR8, + * architecture:component-style-authoring INV6). + */ + +const fs = require('node:fs'); +const path = require('node:path'); +const zlib = require('node:zlib'); + +function gzipBytes(text) { + return zlib.gzipSync(Buffer.from(text, 'utf8')).length; +} + +// The package's prebuilt stylesheet: `dist/.css` from its `.css` export +// other than the reset and Tailwind theme files. +function packageStylesheetPath(packageDir) { + try { + const manifest = JSON.parse( + fs.readFileSync(path.join(packageDir, 'package.json'), 'utf8'), + ); + const keys = Object.keys(manifest.exports || {}).filter( + key => + key.endsWith('.css') && + key !== './reset.css' && + key !== './tailwind-theme.css', + ); + for (const key of keys) { + const target = manifest.exports[key]; + const file = typeof target === 'string' ? target : target?.default; + if (!file) continue; + const resolved = path.join(packageDir, file); + if (fs.existsSync(resolved)) return resolved; + } + } catch { + // An unreadable manifest has no stylesheet to measure. + } + return null; +} + +// Bundle what importing `entry` loads eagerly: the entry chunk and the chunks +// it imports statically, including modules reached through package imports +// (other Astryx packages, React, StyleX, third-party libraries), minified for +// production. Also returns the modules of every chunk the import can load, +// import() targets included, for CSS. Returns null when the import cannot be +// bundled. +function bundleImportWithInputs(entry, exportName) { + if (!entry || !fs.existsSync(entry)) return null; + const os = require('node:os'); + const scratch = fs.mkdtempSync(path.join(os.tmpdir(), 'astryx-size-')); + try { + const esbuild = require('esbuild'); + const workingDir = process.cwd(); + const entryFile = path.join(scratch, 'entry.mjs'); + fs.writeFileSync( + entryFile, + exportName + ? `export {${exportName}} from ${JSON.stringify(path.resolve(entry))};` + : `export * from ${JSON.stringify(path.resolve(entry))};`, + ); + // Code splitting keeps modules reached only through import() in their own + // chunks, so the figure counts what the import loads eagerly: the entry + // chunk and every chunk it imports statically. + const outdir = path.join(scratch, 'out'); + const result = esbuild.buildSync({ + absWorkingDir: workingDir, + entryPoints: [entryFile], + outdir, + metafile: true, + bundle: true, + splitting: true, + minify: true, + format: 'esm', + platform: 'browser', + define: {'process.env.NODE_ENV': '"production"'}, + loader: {'.css': 'empty'}, + write: false, + logLevel: 'silent', + }); + const outputs = result.metafile.outputs; + const byPath = new Map( + result.outputFiles.map(file => [ + path.relative(workingDir, file.path), + file.text, + ]), + ); + const entryOutput = Object.keys(outputs).find( + key => outputs[key].entryPoint !== undefined, + ); + // Chunks the entry reaches, following only the given import kinds. + function reachable(kinds) { + const seen = new Set(); + const pending = [entryOutput]; + while (pending.length > 0) { + const key = pending.pop(); + if (key === undefined || seen.has(key)) continue; + seen.add(key); + for (const imported of outputs[key].imports || []) { + if (kinds.includes(imported.kind) && !imported.external) { + pending.push(imported.path); + } + } + } + return seen; + } + let text = ''; + for (const key of reachable(['import-statement'])) { + text += byPath.get(key) ?? ''; + } + // CSS ships in the package stylesheet whether its module loads eagerly or + // through import(), so class names come from every chunk the entry can + // load, dynamic imports included; chunks no kept code imports are ignored. + const inputs = new Set(); + for (const key of reachable(['import-statement', 'dynamic-import'])) { + for (const input of Object.keys(outputs[key].inputs || {})) { + inputs.add(path.resolve(workingDir, input)); + } + } + inputs.delete(entryFile); + return {text, inputs: [...inputs]}; + } catch { + return null; + } finally { + fs.rmSync(scratch, {recursive: true, force: true}); + } +} + +// The modules each entry's bundle reaches, import() targets included, from one +// build over every entry: Map of absolute entry path to absolute input paths. +// Returns null when the entries cannot be bundled together. +function bundleInputsByEntry(entries) { + const existing = entries.filter(entry => fs.existsSync(entry)); + if (existing.length === 0) return new Map(); + const os = require('node:os'); + const scratch = fs.mkdtempSync(path.join(os.tmpdir(), 'astryx-reach-')); + try { + const esbuild = require('esbuild'); + const workingDir = process.cwd(); + const result = esbuild.buildSync({ + absWorkingDir: workingDir, + entryPoints: existing, + outdir: scratch, + metafile: true, + bundle: true, + format: 'esm', + platform: 'browser', + define: {'process.env.NODE_ENV': '"production"'}, + loader: {'.css': 'empty'}, + write: false, + logLevel: 'silent', + }); + const reach = new Map(); + for (const output of Object.values(result.metafile.outputs)) { + if (output.entryPoint === undefined) continue; + reach.set( + fs.realpathSync(path.resolve(workingDir, output.entryPoint)), + Object.keys(output.inputs).map(input => + path.resolve(workingDir, input), + ), + ); + } + // Key by the paths callers passed, which may differ from real paths. + const byEntry = new Map(); + for (const entry of existing) { + byEntry.set(entry, reach.get(fs.realpathSync(entry)) ?? []); + } + return byEntry; + } catch { + return null; + } finally { + fs.rmSync(scratch, {recursive: true, force: true}); + } +} + +function bundleImport(entry) { + return bundleImportWithInputs(entry)?.text ?? null; +} + +// StyleX atomic class names the bundle references. +function classNamesIn(js) { + return new Set(js.match(/\bx[a-z0-9]{3,12}\b/g) || []); +} + +// Parse CSS into nested blocks: {prelude, children} for block rules and +// {prelude} for statements. Comments are dropped; strings are kept intact. +function parseBlocks(css) { + let index = 0; + function skipComment() { + if (css.startsWith('/*', index)) { + const end = css.indexOf('*/', index + 2); + index = end < 0 ? css.length : end + 2; + return true; + } + return false; + } + function readUntil(stops) { + let out = ''; + while (index < css.length && !stops.includes(css[index])) { + if (skipComment()) continue; + const ch = css[index]; + if (ch === '"' || ch === "'") { + const start = index; + index += 1; + while (index < css.length && css[index] !== ch) { + if (css[index] === '\\') index += 1; + index += 1; + } + index += 1; + out += css.slice(start, index); + continue; + } + out += ch; + index += 1; + } + return out; + } + function parseList() { + const nodes = []; + while (index < css.length) { + const prelude = readUntil(['{', ';', '}']).trim(); + const ch = css[index]; + if (ch === '}') { + index += 1; + break; + } + index += 1; + if (ch === ';' || ch === undefined) { + if (prelude) nodes.push({prelude}); + continue; + } + // A style rule's body holds declarations; an at-rule's body may hold rules. + if ( + prelude.startsWith('@') && + !/^@(font-face|page|property)\b/.test(prelude) + ) { + nodes.push({prelude, children: parseList()}); + } else { + const body = readUntil(['}']); + index += 1; + nodes.push({prelude, body}); + } + } + return nodes; + } + return parseList(); +} + +// The stylesheet rules whose selectors name one of `classes`, inside the +// at-rules that wrap them, plus the @keyframes those rules name. +function rulesForClasses(css, classes) { + if (!css || classes.size === 0) return ''; + const classPattern = /\.([A-Za-z_][\w-]*)/g; + const tree = parseBlocks(css); + function keep(nodes, keyframes) { + let out = ''; + for (const node of nodes) { + if (node.children) { + const name = /^@(?:-webkit-)?keyframes\s+([\w-]+)/.exec( + node.prelude, + )?.[1]; + if (name !== undefined) { + if (keyframes.has(name)) { + out += `${node.prelude}{${node.children.map(child => `${child.prelude}{${child.body ?? ''}}`).join('')}}`; + } + continue; + } + const inner = keep(node.children, keyframes); + if (inner) out += `${node.prelude}{${inner}}`; + } else if (node.body !== undefined) { + const names = [...node.prelude.matchAll(classPattern)].map(m => m[1]); + if (names.some(name => classes.has(name))) { + out += `${node.prelude}{${node.body}}`; + } + } + } + return out; + } + const rules = keep(tree, new Set()); + // Keyframes a kept rule names through `animation` or `animation-name`. + const named = new Set(); + for (const match of rules.matchAll(/animation(?:-name)?\s*:\s*([^;}]+)/g)) { + for (const token of match[1].split(/[\s,]+/)) if (token) named.add(token); + } + return named.size === 0 ? rules : keep(tree, named); +} + +/** + * Measure one component import: gzip JavaScript it pulls in and gzip CSS of + * the stylesheet rules that JavaScript references. Null where unmeasurable. + */ +function measureComponentImport(entry, stylesheetText, ownDistDir, exportName) { + const bundle = bundleImportWithInputs(entry, exportName); + if (bundle === null) return {importGzipBytes: null, cssGzipBytes: null}; + // CSS counts only the rules the component's own package modules reference: + // classes from modules reached in other packages belong to their stylesheets. + // Compare real paths: esbuild reports inputs with symlinks resolved. + const ownDir = + fs.realpathSync(path.resolve(ownDistDir || path.dirname(entry))) + path.sep; + const classes = new Set(); + for (const input of bundle.inputs) { + if (!fs.existsSync(input) || !fs.realpathSync(input).startsWith(ownDir)) + continue; + for (const name of classNamesIn(fs.readFileSync(input, 'utf8'))) + classes.add(name); + } + const css = stylesheetText ? rulesForClasses(stylesheetText, classes) : null; + return { + importGzipBytes: gzipBytes(bundle.text), + cssGzipBytes: css === null ? null : gzipBytes(css), + }; +} + +module.exports = { + gzipBytes, + packageStylesheetPath, + bundleImport, + classNamesIn, + rulesForClasses, + measureComponentImport, + bundleInputsByEntry, +}; diff --git a/.github/scripts/lib/component-size.test.mjs b/.github/scripts/lib/component-size.test.mjs new file mode 100644 index 0000000000000..2566ddf3d1f48 --- /dev/null +++ b/.github/scripts/lib/component-size.test.mjs @@ -0,0 +1,202 @@ +// Copyright (c) Meta Platforms, Inc. and affiliates. + +/** + * @file component-size.test.mjs + * Pins how the PR size report measures one component import: the JavaScript + * it loads eagerly and the stylesheet rules referenced by the modules it can + * load. + */ + +import fs from 'node:fs'; +import {createRequire} from 'node:module'; +import os from 'node:os'; +import path from 'node:path'; +import {afterEach, describe, expect, it} from 'vitest'; + +const require = createRequire(import.meta.url); +const { + bundleImport, + classNamesIn, + measureComponentImport, + packageStylesheetPath, + rulesForClasses, +} = require('./component-size.js'); + +const roots = []; +function tree(files) { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'astryx-size-')); + roots.push(root); + for (const [file, content] of Object.entries(files)) { + fs.mkdirSync(path.dirname(path.join(root, file)), {recursive: true}); + fs.writeFileSync(path.join(root, file), content); + } + return root; +} + +afterEach(() => { + for (const root of roots.splice(0)) + fs.rmSync(root, {recursive: true, force: true}); +}); + +describe('component import size', () => { + it('bundles every module the import reaches, package imports included', () => { + const root = tree({ + 'dist/Card/index.js': "export {Card} from './Card.js';\n", + 'dist/Card/Card.js': + "import {sheet} from 'fake-sheet';\nimport {helper} from '../shared/helper.js';\nexport function Card() { return sheet('xcard1 ' + helper()); }\n", + 'dist/shared/helper.js': + "export function helper() { return 'xhelp22'; }\n", + 'dist/Other/index.js': "export const unrelated = 'xother9';\n", + 'node_modules/fake-sheet/package.json': JSON.stringify({ + name: 'fake-sheet', + main: 'index.js', + }), + 'node_modules/fake-sheet/index.js': + "export function sheet(name) { return 'xsheet7 ' + name + ' PACKAGE_CODE_MARKER'; }\n", + }); + const js = bundleImport(path.join(root, 'dist/Card/index.js')); + expect(js).toContain('xhelp22'); + expect(js).toContain('PACKAGE_CODE_MARKER'); + expect(js).not.toContain('xother9'); + expect([...classNamesIn(js)].sort()).toEqual([ + 'xcard1', + 'xhelp22', + 'xsheet7', + ]); + }); + + it('keeps only the rules whose selectors name a referenced class, with their at-rules', () => { + const css = [ + '@layer reset, astryx-base, astryx-theme;', + '@layer astryx-base{', + '.xcard1{color:red}', + '.xother9{color:blue}', + '@media (pointer: coarse){.xcard1:active{opacity:.5}.xother9{opacity:1}}', + '@keyframes xspin1{to{transform:rotate(1turn)}}', + '}', + ].join(''); + const kept = rulesForClasses(css, new Set(['xcard1'])); + expect(kept).toBe( + '@layer astryx-base{.xcard1{color:red}@media (pointer: coarse){.xcard1:active{opacity:.5}}}', + ); + expect(rulesForClasses(css, new Set())).toBe(''); + }); + + it('counts CSS only for classes the component package modules reference', () => { + const root = tree({ + 'packages/lab/dist/Tip/index.js': + "export {Button} from '../../../core/dist/Button.js';\nexport const Tip = () => 'xtip1';\n", + 'packages/core/dist/Button.js': "export const Button = () => 'xbtn9';\n", + 'packages/lab/dist/lab.css': + '.xtip1{color:red}.xbtn9{color:blue;padding:4px;margin:2px}', + }); + const css = fs.readFileSync( + path.join(root, 'packages/lab/dist/lab.css'), + 'utf8', + ); + const own = measureComponentImport( + path.join(root, 'packages/lab/dist/Tip/index.js'), + css, + path.join(root, 'packages/lab/dist'), + ); + const onlyTip = measureComponentImport( + path.join(root, 'packages/lab/dist/Tip/index.js'), + '.xtip1{color:red}', + path.join(root, 'packages/lab/dist'), + ); + expect(own.cssGzipBytes).toBe(onlyTip.cssGzipBytes); + expect( + bundleImport(path.join(root, 'packages/lab/dist/Tip/index.js')), + ).toContain('xbtn9'); + }); + + it('counts only what the import loads eagerly, not import() targets', () => { + const root = tree({ + 'dist/Menu/index.js': + "export function Menu() { return import('./Touch.js').then(m => m.touch()); }\n", + 'dist/Menu/Touch.js': + "export function touch() { return 'LAZY_TOUCH_CODE'; }\n", + }); + const js = bundleImport(path.join(root, 'dist/Menu/index.js')); + expect(js).not.toContain('LAZY_TOUCH_CODE'); + expect(js).toContain('import('); + }); + + it('counts CSS a lazily loaded module references, since it ships in the stylesheet', () => { + const root = tree({ + 'dist/Menu/index.js': + "export function Menu() { return 'xmenu1 ' + import('./Touch.js'); }\n", + 'dist/Menu/Touch.js': "export const touch = 'xtouch1';\n", + }); + const sheet = '.xmenu1{color:red}.xtouch1{padding:44px}.xother1{margin:0}'; + const withLazy = measureComponentImport( + path.join(root, 'dist/Menu/index.js'), + sheet, + path.join(root, 'dist'), + ); + const eagerOnly = measureComponentImport( + path.join(root, 'dist/Menu/index.js'), + '.xmenu1{color:red}.xother1{margin:0}', + path.join(root, 'dist'), + ); + expect(withLazy.cssGzipBytes).toBeGreaterThan(eagerOnly.cssGzipBytes); + }); + + it('ignores CSS behind an import() in an export the import does not use', () => { + const root = tree({ + 'dist/Menu/index.js': + "export function Menu() { return 'xmenu1'; }\nexport function Unused() { return import('./Touch.js'); }\n", + 'dist/Menu/Touch.js': "export const touch = 'xtouch1';\n", + }); + const sheet = '.xmenu1{color:red}.xtouch1{padding:44px}'; + const used = measureComponentImport( + path.join(root, 'dist/Menu/index.js'), + sheet, + path.join(root, 'dist'), + 'Menu', + ); + const menuOnly = measureComponentImport( + path.join(root, 'dist/Menu/index.js'), + '.xmenu1{color:red}', + path.join(root, 'dist'), + 'Menu', + ); + expect(used.cssGzipBytes).toBe(menuOnly.cssGzipBytes); + }); + + it('keeps the @keyframes a kept rule names', () => { + const css = + '@layer astryx-base{.xspin2{animation-name:xrot1}.xother9{animation-name:xrot9}@keyframes xrot1{from{opacity:0}to{opacity:1}}@keyframes xrot9{to{opacity:1}}}'; + expect(rulesForClasses(css, new Set(['xspin2']))).toBe( + '@layer astryx-base{.xspin2{animation-name:xrot1}@keyframes xrot1{from{opacity:0}to{opacity:1}}}', + ); + }); + + it('measures gzip JavaScript and CSS, and null for a missing entry', () => { + const root = tree({ + 'package.json': JSON.stringify({ + exports: { + './reset.css': './dist/reset.css', + './astryx.css': {default: './dist/astryx.css'}, + }, + }), + 'dist/reset.css': '*{margin:0}', + 'dist/astryx.css': '.xcard1{color:red}.xother9{color:blue}', + 'dist/Card/index.js': "export const Card = () => 'xcard1';\n", + }); + const sheet = packageStylesheetPath(root); + expect(sheet).toBe(path.join(root, 'dist/astryx.css')); + const size = measureComponentImport( + path.join(root, 'dist/Card/index.js'), + fs.readFileSync(sheet, 'utf8'), + ); + expect(size.importGzipBytes).toBeGreaterThan(0); + expect(size.cssGzipBytes).toBeGreaterThan(0); + expect( + measureComponentImport(path.join(root, 'dist/Nope/index.js'), ''), + ).toEqual({ + importGzipBytes: null, + cssGzipBytes: null, + }); + }); +});