diff --git a/.github/workflows/frontend-typecheck.yml b/.github/workflows/frontend-typecheck.yml new file mode 100644 index 000000000000..566e44c93913 --- /dev/null +++ b/.github/workflows/frontend-typecheck.yml @@ -0,0 +1,117 @@ +name: Frontend Type Check + +# Reports how a pull request changes the number of TypeScript errors. +# Main keeps its own count as an artifact; pull requests compare against it. +# Not a required check: it calls attention to new errors without blocking. + +on: + push: + branches: + - main + paths: + - frontend/** + - .github/workflows/frontend-typecheck.yml + pull_request: + types: [opened, synchronize, reopened, ready_for_review] + paths: + - frontend/** + - .github/workflows/frontend-typecheck.yml + +permissions: + contents: read + +# A newer push to a pull request replaces its older report. Main's counts are +# never cancelled, since pull requests compare against them. +concurrency: + group: frontend-typecheck-${{ github.event.pull_request.number || github.sha }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} + +jobs: + typescript-errors: + name: TypeScript errors + runs-on: ubuntu-latest + permissions: + contents: read + actions: read # download main's count + pull-requests: write # post the report + + defaults: + run: + working-directory: frontend + + env: + BASE_SHA: ${{ github.event.pull_request.base.sha }} + + steps: + - uses: actions/checkout@v5 + + - name: Setup Node.js + uses: actions/setup-node@v4 + with: + node-version-file: frontend/.nvmrc + cache: npm + cache-dependency-path: frontend/package-lock.json + + - name: Install dependencies + run: npm ci + + - name: Count errors + run: node scripts/typecheck-report/index.mjs count "$GITHUB_SHA" typecheck-counts.json + + - name: Keep main's count + if: github.event_name == 'push' + uses: actions/upload-artifact@v4 + with: + name: typecheck-counts + path: frontend/typecheck-counts.json + retention-days: 90 + + # Main only runs this when frontend files change, so its latest count + # usually belongs to an older commit than the base. The next step checks + # that the frontend is the same in both. + - name: Download main's latest count + if: github.event_name == 'pull_request' + continue-on-error: true + uses: dawidd6/action-download-artifact@v6 + with: + workflow: frontend-typecheck.yml + branch: main + name: typecheck-counts + path: frontend/base + if_no_artifact_found: warn + + - name: Check the count matches the base + if: github.event_name == 'pull_request' + id: base + run: | + if [ ! -f base/typecheck-counts.json ]; then + echo "state=missing" >> "$GITHUB_OUTPUT" + exit 0 + fi + count_sha=$(node -p "require('./base/typecheck-counts.json').sha") + state=current + if [ "$count_sha" != "$BASE_SHA" ]; then + git fetch --no-tags --depth=1 origin "$count_sha" "$BASE_SHA" + git diff --quiet "$count_sha" "$BASE_SHA" -- . || state=stale + fi + echo "state=$state" >> "$GITHUB_OUTPUT" + + - name: Compare with main + if: github.event_name == 'pull_request' + id: compare + run: | + action=$(node scripts/typecheck-report/index.mjs compare base/typecheck-counts.json typecheck-counts.json "${{ steps.base.outputs.state }}" report.md) + echo "action=$action" >> "$GITHUB_OUTPUT" + cat report.md >> "$GITHUB_STEP_SUMMARY" + + # Comments when the total moves or any file gains errors. Otherwise it + # only refreshes an existing comment, so quiet pull requests get none. + # Fork pull requests can't comment; the job summary still has the report. + - name: Report on the pull request + if: github.event_name == 'pull_request' + continue-on-error: true + uses: marocchino/sticky-pull-request-comment@v2 + with: + header: typescript-errors + path: frontend/report.md + only_update: ${{ steps.compare.outputs.action == 'update' }} diff --git a/frontend/scripts/typecheck-report/compare.test.ts b/frontend/scripts/typecheck-report/compare.test.ts new file mode 100644 index 000000000000..164a3c9a13c5 --- /dev/null +++ b/frontend/scripts/typecheck-report/compare.test.ts @@ -0,0 +1,84 @@ +import { compareCounts, ErrorCounts } from './compare' + +const counts = (files: Record): ErrorCounts => ({ + files, + sha: 'abc', +}) + +describe('compareCounts', () => { + it('reports more errors in red and comments', () => { + const report = compareCounts( + counts({ 'a.ts': 2 }), + counts({ 'a.ts': 3, 'b.ts': 1 }), + 'current', + ) + expect(report.action).toBe('comment') + expect(report.markdown).toBe( + [ + '## TypeScript errors', + 'šŸ”“ 2 → 4 (+2)', + '', + '| File | Change |', + '| --- | --- |', + '| `a.ts` | +1 |', + '| `b.ts` | +1 |', + '', + ].join('\n'), + ) + }) + + it('reports fewer errors in green and comments', () => { + const report = compareCounts( + counts({ 'a.ts': 3 }), + counts({ 'a.ts': 1 }), + 'current', + ) + expect(report.action).toBe('comment') + expect(report.markdown).toContain('🟢 3 → 1 (-2)') + }) + + it('comments when a file goes up even if the total does not', () => { + const report = compareCounts( + counts({ 'a.ts': 2, 'b.ts': 2 }), + counts({ 'a.ts': 4 }), + 'current', + ) + expect(report.action).toBe('comment') + expect(report.markdown).toContain('āšŖļø 4 → 4 (0)') + }) + + it('only updates an existing comment when nothing goes up', () => { + const report = compareCounts( + counts({ 'a.ts': 2 }), + counts({ 'a.ts': 2 }), + 'current', + ) + expect(report.action).toBe('update') + expect(report.markdown).toBe('## TypeScript errors\nāšŖļø 2 → 2 (0)\n') + }) + + it('does not compare with a stale count', () => { + const report = compareCounts( + counts({ 'a.ts': 9 }), + counts({ 'a.ts': 1 }), + 'stale', + ) + expect(report.action).toBe('update') + expect(report.markdown).toContain("Main's count is out of date") + }) + + it('says when main has no count yet', () => { + const report = compareCounts(null, counts({ 'a.ts': 1 }), 'missing') + expect(report.action).toBe('update') + expect(report.markdown).toContain('No count from main') + }) + + it('lists at most 30 files', () => { + const head = Object.fromEntries( + Array.from({ length: 32 }, (_, i) => [`f${i}.ts`, 1]), + ) + const report = compareCounts(counts({}), counts(head), 'current') + expect(report.markdown.match(/^\| `/gm)).toHaveLength(30) + expect(report.markdown).toContain('And 2 more files.') + }) +}) diff --git a/frontend/scripts/typecheck-report/compare.ts b/frontend/scripts/typecheck-report/compare.ts new file mode 100644 index 000000000000..2376bd5770cd --- /dev/null +++ b/frontend/scripts/typecheck-report/compare.ts @@ -0,0 +1,92 @@ +export type ErrorCounts = { + sha: string + files: Record +} + +export type BaseState = 'current' | 'stale' | 'missing' + +// `comment` posts or updates the PR comment; `update` only refreshes one +// that already exists, so quiet PRs get no comment. +export type ReportAction = 'comment' | 'update' + +export type Report = { + action: ReportAction + markdown: string +} + +// Keeps the comment well under GitHub's 65536 character limit. +const MAX_ROWS = 30 + +export const total = (files: Record) => + Object.values(files).reduce((sum, n) => sum + n, 0) + +const signed = (n: number) => (n > 0 ? `+${n}` : `${n}`) + +const icon = (delta: number) => { + if (delta > 0) return 'šŸ”“' + if (delta < 0) return '🟢' + return 'āšŖļø' +} + +export const compareCounts = ( + base: ErrorCounts | null, + head: ErrorCounts, + baseState: BaseState, +): Report => { + const heading = '## TypeScript errors' + if (!base || baseState === 'missing') { + return { + action: 'update', + markdown: `${heading}\n${total( + head.files, + )} errors. No count from main to compare with yet.\n`, + } + } + if (baseState === 'stale') { + return { + action: 'update', + markdown: `${heading}\n${total( + head.files, + )} errors. Main's count is out of date, so there is nothing to compare with.\n`, + } + } + + const delta = total(head.files) - total(base.files) + const changed = [ + ...new Set([...Object.keys(base.files), ...Object.keys(head.files)]), + ] + .map((file) => ({ + delta: (head.files[file] || 0) - (base.files[file] || 0), + file, + })) + .filter((row) => row.delta !== 0) + .sort((a, b) => b.delta - a.delta || a.file.localeCompare(b.file)) + + const lines = [ + heading, + `${icon(delta)} ${total(base.files)} → ${total(head.files)} (${signed( + delta, + )})`, + ] + if (changed.length) { + lines.push( + '', + '| File | Change |', + '| --- | --- |', + ...changed + .slice(0, MAX_ROWS) + .map((row) => `| \`${row.file}\` | ${signed(row.delta)} |`), + ) + if (changed.length > MAX_ROWS) { + lines.push('', `And ${changed.length - MAX_ROWS} more files.`) + } + } + + // A file going up is worth a comment even when fixes elsewhere hide it in + // the total. + const anyFileUp = changed.some((row) => row.delta > 0) + return { + action: delta !== 0 || anyFileUp ? 'comment' : 'update', + markdown: `${lines.join('\n')}\n`, + } +} diff --git a/frontend/scripts/typecheck-report/index.mjs b/frontend/scripts/typecheck-report/index.mjs new file mode 100644 index 000000000000..3500d787cb9b --- /dev/null +++ b/frontend/scripts/typecheck-report/index.mjs @@ -0,0 +1,57 @@ +#!/usr/bin/env node +// Counts TypeScript errors per file and compares two counts for the PR report. +// +// node scripts/typecheck-report/index.mjs count +// node scripts/typecheck-report/index.mjs compare +// +// `compare` prints `comment` or `update` for the workflow to act on. +// Never fails on type errors: the report calls attention, it doesn't block. + +import { existsSync, readFileSync, writeFileSync } from 'node:fs' +import { spawnSync } from 'node:child_process' +import { compareCounts, total } from './compare.ts' + +const WITH_FILE = /^(.+?)\(\d+,\d+\): error TS\d+:/ +const WITHOUT_FILE = /^error TS\d+:/ + +const count = (sha, out) => { + const tsc = spawnSync( + 'npx', + ['tsc', '--noEmit', '--pretty', 'false', '--incremental', 'false'], + { encoding: 'utf8', maxBuffer: 64 * 1024 * 1024 }, + ) + // tsc exits 0 when clean and 1 or 2 when it reports errors; anything else + // means it didn't run. + if (tsc.error || ![0, 1, 2].includes(tsc.status)) { + process.stderr.write(`${tsc.stdout}${tsc.stderr}`) + process.exit(1) + } + const files = {} + for (const line of tsc.stdout.split('\n')) { + const file = line.match(WITH_FILE)?.[1] ?? (WITHOUT_FILE.test(line) && '(no file)') + if (file) files[file] = (files[file] || 0) + 1 + } + writeFileSync(out, `${JSON.stringify({ files, sha }, null, 2)}\n`) + process.stdout.write( + `${total(files)} errors in ${Object.keys(files).length} files\n`, + ) +} + +const read = (path) => + existsSync(path) ? JSON.parse(readFileSync(path, 'utf8')) : null + +const compare = (basePath, headPath, baseState, out) => { + const report = compareCounts(read(basePath), read(headPath), baseState) + writeFileSync(out, report.markdown) + process.stdout.write(`${report.action}\n`) +} + +const [mode, ...args] = process.argv.slice(2) +if (mode === 'count') count(...args) +else if (mode === 'compare') compare(...args) +else { + process.stderr.write( + 'Usage: index.mjs count | compare \n', + ) + process.exit(1) +} diff --git a/frontend/scripts/typecheck-report/package.json b/frontend/scripts/typecheck-report/package.json new file mode 100644 index 000000000000..3dbc1ca591c0 --- /dev/null +++ b/frontend/scripts/typecheck-report/package.json @@ -0,0 +1,3 @@ +{ + "type": "module" +}