From c64d3a50e8268f68e130ef8a98e2711bdfbf223e Mon Sep 17 00:00:00 2001 From: conico974 Date: Tue, 18 Aug 2026 10:30:43 +0200 Subject: [PATCH] Refactor CSS compatibility checks (#4504) --- .../workflows/css-browser-compatibility.yaml | 1 - packages/gitbook/package.json | 1 + .../check-css-browser-compatibility.ts | 153 ++++++++++-------- .../src/lib/cssBrowserCompatibility.test.ts | 96 ----------- .../src/lib/cssBrowserCompatibility.ts | 81 ---------- 5 files changed, 91 insertions(+), 241 deletions(-) diff --git a/.github/workflows/css-browser-compatibility.yaml b/.github/workflows/css-browser-compatibility.yaml index 76be1dba0..ade3ee0ee 100644 --- a/.github/workflows/css-browser-compatibility.yaml +++ b/.github/workflows/css-browser-compatibility.yaml @@ -6,7 +6,6 @@ on: permissions: contents: read - issues: write pull-requests: read concurrency: diff --git a/packages/gitbook/package.json b/packages/gitbook/package.json index 07f2c74ff..5ac494f75 100644 --- a/packages/gitbook/package.json +++ b/packages/gitbook/package.json @@ -124,6 +124,7 @@ "build": "bun run generate:assets && next build --webpack", "build:local": "bun run generate:assets && GITBOOK_URL=http://localhost:3000 next build --webpack", "check:css-browser-compatibility": "bun scripts/check-css-browser-compatibility.ts", + "check:css-browser-compatibility:local": "bun run check:css-browser-compatibility --local origin/main", "start": "GITBOOK_URL=http://localhost:3000 next start", "build:cloudflare": "bun run generate:assets && GITBOOK_RUNTIME=cloudflare opennextjs-cloudflare build", "dev:cloudflare": "wrangler dev --port 8771 --env preview", diff --git a/packages/gitbook/scripts/check-css-browser-compatibility.ts b/packages/gitbook/scripts/check-css-browser-compatibility.ts index 3f0e4df59..87d3d48df 100644 --- a/packages/gitbook/scripts/check-css-browser-compatibility.ts +++ b/packages/gitbook/scripts/check-css-browser-compatibility.ts @@ -1,11 +1,10 @@ +import { execFileSync } from 'node:child_process'; import { readFile } from 'node:fs/promises'; +import { join } from 'node:path'; import { type CompatibilityDiagnostic, - type IssueCommentClient, - formatCompatibilityComment, getCompatibilityDiagnostics, - upsertCompatibilityComment, } from '../src/lib/cssBrowserCompatibility'; interface PullRequestEvent { @@ -33,12 +32,6 @@ interface GitBlobResponse { encoding: string; } -interface IssueComment { - body: string; - id: number; - user: { login: string } | null; -} - class GitHubRequestError extends Error { constructor( readonly status: number, @@ -48,7 +41,7 @@ class GitHubRequestError extends Error { } } -class GitHubApi implements IssueCommentClient { +class GitHubApi { constructor( private readonly repository: string, private readonly token: string @@ -114,34 +107,6 @@ class GitHubApi implements IssueCommentClient { async getFileAtRef(path: string, ref: string): Promise { return this.getContent(path, ref); } - - async listIssueComments(issueNumber: number): Promise { - const comments: IssueComment[] = []; - - for (let page = 1; ; page += 1) { - const result = await this.request( - `/repos/${this.repository}/issues/${issueNumber}/comments?per_page=100&page=${page}` - ); - comments.push(...result); - if (result.length < 100) { - return comments; - } - } - } - - async createIssueComment(issueNumber: number, body: string): Promise { - await this.request(`/repos/${this.repository}/issues/${issueNumber}/comments`, { - body: JSON.stringify({ body }), - method: 'POST', - }); - } - - async updateIssueComment(commentId: number, body: string): Promise { - await this.request(`/repos/${this.repository}/issues/comments/${commentId}`, { - body: JSON.stringify({ body }), - method: 'PATCH', - }); - } } async function getBrowserslist(api: GitHubApi, headSha: string): Promise { @@ -175,6 +140,83 @@ async function getBaseContent( } } +function report(diagnostics: CompatibilityDiagnostic[]): boolean { + if (diagnostics.length === 0) { + console.log('CSS browser compatibility check passed.'); + return true; + } + + console.error( + `${diagnostics.length} newly added CSS declaration(s) are not fully supported by the configured Browserslist targets:` + ); + for (const diagnostic of diagnostics) { + // Workflow command so the failure is annotated on the PR diff. + console.error( + `::error file=${diagnostic.file},line=${diagnostic.line},col=${diagnostic.column}::${diagnostic.property} is not supported by ${diagnostic.unsupportedBrowsers}` + ); + } + return false; +} + +/** Same check as CI, but against a local `git diff` instead of the GitHub API. */ +async function runLocal(baseRef: string): Promise { + const git = (...args: string[]) => + execFileSync('git', args, { encoding: 'utf8', maxBuffer: 64 * 1024 * 1024 }); + const root = git('rev-parse', '--show-toplevel').trim(); + const mergeBase = git('merge-base', baseRef, 'HEAD').trim(); + const browsers = ( + JSON.parse(await readFile(join(root, 'packages/gitbook/package.json'), 'utf8')) as { + browserslist?: string[]; + } + ).browserslist; + + if (!browsers?.length) { + throw new Error('packages/gitbook/package.json must define a Browserslist configuration.'); + } + + // Working tree, so uncommitted changes are checked too. + const changes = git( + 'diff', + '--name-status', + '--find-renames', + '--diff-filter=ACMR', + mergeBase, + '--', + '*.css' + ) + .split('\n') + .filter(Boolean) + .map((line) => { + const [status, ...paths] = line.split('\t'); + const previousPath = paths.length > 1 ? paths[0] : undefined; + const path = paths.at(-1) as string; + return { added: status?.startsWith('A'), path, previousPath }; + }); + + const diagnostics: CompatibilityDiagnostic[] = []; + for (const change of changes) { + let base = ''; + if (!change.added) { + try { + base = git('show', `${mergeBase}:${change.previousPath ?? change.path}`); + } catch { + base = ''; + } + } + diagnostics.push( + ...(await getCompatibilityDiagnostics({ + base, + browsers, + file: change.path, + head: await readFile(join(root, change.path), 'utf8'), + })) + ); + } + + console.log(`Checked ${changes.length} changed CSS file(s) against ${baseRef}.`); + return report(diagnostics); +} + async function run(): Promise { const token = process.env.GITHUB_TOKEN; const repository = process.env.GITHUB_REPOSITORY; @@ -212,34 +254,19 @@ async function run(): Promise { ); } - const comment = formatCompatibilityComment({ - diagnostics, - headSha: pullRequest.head.sha, - repository, - }); - await upsertCompatibilityComment({ - body: comment, - createIfMissing: diagnostics.length > 0, - client: api, - issueNumber: pullRequest.number, - }); - - if (diagnostics.length === 0) { - console.log('CSS browser compatibility check passed.'); - return true; - } - - console.error('Unsupported CSS declarations found:'); - for (const diagnostic of diagnostics) { - console.error( - `${diagnostic.file}:${diagnostic.line} ${diagnostic.property} ā€” ${diagnostic.unsupportedBrowsers}` - ); - } - return false; + return report(diagnostics); } +const localFlagIndex = process.argv.indexOf('--local'); + try { - process.exitCode = (await run()) ? 0 : 1; + let success: boolean; + if (localFlagIndex === -1) { + success = await run(); + } else { + success = await runLocal(process.argv[localFlagIndex + 1] ?? 'origin/main'); + } + process.exitCode = success ? 0 : 1; } catch (error) { console.error(error); process.exitCode = 1; diff --git a/packages/gitbook/src/lib/cssBrowserCompatibility.test.ts b/packages/gitbook/src/lib/cssBrowserCompatibility.test.ts index 853679e5a..0459f7ff5 100644 --- a/packages/gitbook/src/lib/cssBrowserCompatibility.test.ts +++ b/packages/gitbook/src/lib/cssBrowserCompatibility.test.ts @@ -1,13 +1,9 @@ import { describe, expect, it } from 'bun:test'; import { - COMMENT_MARKER, - type IssueCommentClient, - formatCompatibilityComment, getAddedDeclarations, getChangedLines, getCompatibilityDiagnostics, - upsertCompatibilityComment, } from './cssBrowserCompatibility'; const baseCSS = `.card { @@ -117,95 +113,3 @@ describe('getCompatibilityDiagnostics', () => { expect(diagnostics[0]?.property).toBe('appearance'); }); }); - -class FakeCommentClient implements IssueCommentClient { - created: string[] = []; - comments: { - body: string; - id: number; - user: { login: string } | null; - }[] = []; - updated: { body: string; id: number }[] = []; - - async createIssueComment(_issueNumber: number, body: string): Promise { - this.created.push(body); - } - - async listIssueComments(): Promise { - return this.comments; - } - - async updateIssueComment(commentId: number, body: string): Promise { - this.updated.push({ body, id: commentId }); - } -} - -describe('PR comments', () => { - const diagnostics = [ - { - column: 5, - feature: 'css-container-queries', - file: 'packages/gitbook/src/example.css', - line: 3, - property: 'container-type', - unsupportedBrowsers: 'Safari 12', - }, - ]; - - it('formats an actionable compatibility report', () => { - const comment = formatCompatibilityComment({ - diagnostics, - headSha: 'abc123', - repository: 'GitbookIO/gitbook', - }); - - expect(comment).toContain(COMMENT_MARKER); - expect(comment).toContain('container-type'); - expect(comment).toContain('Safari 12'); - expect(comment).toContain('example.css:3'); - }); - - it('creates a failure comment, updates it on rerun, and marks it clean', async () => { - const client = new FakeCommentClient(); - const failureComment = formatCompatibilityComment({ - diagnostics, - headSha: 'abc123', - repository: 'GitbookIO/gitbook', - }); - - await upsertCompatibilityComment({ body: failureComment, client, issueNumber: 42 }); - expect(client.created).toEqual([failureComment]); - - client.comments = [{ body: failureComment, id: 9, user: { login: 'github-actions[bot]' } }]; - const passingComment = formatCompatibilityComment({ - diagnostics: [], - headSha: 'def456', - repository: 'GitbookIO/gitbook', - }); - await upsertCompatibilityComment({ - body: passingComment, - createIfMissing: false, - client, - issueNumber: 42, - }); - - expect(client.updated).toEqual([{ body: passingComment, id: 9 }]); - }); - - it('does not add a passing comment when no compatibility comment exists', async () => { - const client = new FakeCommentClient(); - await upsertCompatibilityComment({ - body: formatCompatibilityComment({ - diagnostics: [], - headSha: 'abc123', - repository: 'GitbookIO/gitbook', - }), - createIfMissing: false, - client, - issueNumber: 42, - }); - - expect(client.created).toEqual([]); - expect(client.updated).toEqual([]); - }); -}); diff --git a/packages/gitbook/src/lib/cssBrowserCompatibility.ts b/packages/gitbook/src/lib/cssBrowserCompatibility.ts index 13973f91c..0c73ef796 100644 --- a/packages/gitbook/src/lib/cssBrowserCompatibility.ts +++ b/packages/gitbook/src/lib/cssBrowserCompatibility.ts @@ -2,9 +2,7 @@ import { diffLines } from 'diff'; import postcss, { type Declaration } from 'postcss'; import stylelint from 'stylelint'; -export const COMMENT_MARKER = ''; const COMPATIBILITY_RULE = 'plugin/no-unsupported-browser-features'; -const MAX_COMMENT_DIAGNOSTICS = 50; export interface ChangedLines { added: Set; @@ -23,18 +21,6 @@ export interface CompatibilityDiagnostic extends AddedDeclaration { unsupportedBrowsers: string; } -interface Comment { - body: string; - id: number; - user: { login: string } | null; -} - -export interface IssueCommentClient { - createIssueComment(issueNumber: number, body: string): Promise; - listIssueComments(issueNumber: number): Promise; - updateIssueComment(commentId: number, body: string): Promise; -} - function lineCount(value: string): number { return value === '' ? 0 : value.split('\n').length - (value.endsWith('\n') ? 1 : 0); } @@ -205,70 +191,3 @@ export async function getCompatibilityDiagnostics({ ).values() ); } - -function escapeTableCell(value: string): string { - return value.replaceAll('\\', '\\\\').replaceAll('|', '\\|').replaceAll('\n', ' '); -} - -function blobUrl(repository: string, headSha: string, file: string, line: number): string { - const encodedFile = file.split('/').map(encodeURIComponent).join('/'); - return `https://github.com/${repository}/blob/${headSha}/${encodedFile}#L${line}`; -} - -export function formatCompatibilityComment({ - diagnostics, - headSha, - repository, -}: { - diagnostics: CompatibilityDiagnostic[]; - headSha: string; - repository: string; -}): string { - if (diagnostics.length === 0) { - return `${COMMENT_MARKER}\n## CSS browser compatibility\n\nāœ… No newly added CSS declarations have browser-compatibility issues.`; - } - - const visibleDiagnostics = diagnostics.slice(0, MAX_COMMENT_DIAGNOSTICS); - const rows = visibleDiagnostics.map((diagnostic) => { - const location = `[${escapeTableCell(diagnostic.file)}:${diagnostic.line}](${blobUrl(repository, headSha, diagnostic.file, diagnostic.line)})`; - return `| ${location} | \`${escapeTableCell(diagnostic.property)}\` | ${escapeTableCell(diagnostic.unsupportedBrowsers)} |`; - }); - const remaining = diagnostics.length - visibleDiagnostics.length; - const heading = diagnostics.length === 1 ? 'declaration is' : 'declarations are'; - - return [ - COMMENT_MARKER, - '## CSS browser compatibility', - '', - `āŒ ${diagnostics.length} newly added CSS ${heading} not fully supported by the configured Browserslist targets.`, - '', - '| Location | Property | Unsupported browsers |', - '| --- | --- | --- |', - ...rows, - ...(remaining > 0 ? ['', `_${remaining} additional finding(s) omitted._`] : []), - ].join('\n'); -} - -export async function upsertCompatibilityComment({ - body, - createIfMissing = true, - client, - issueNumber, -}: { - body: string; - createIfMissing?: boolean; - client: IssueCommentClient; - issueNumber: number; -}): Promise { - const comments = await client.listIssueComments(issueNumber); - const existingComment = comments.find( - (comment) => - comment.user?.login === 'github-actions[bot]' && comment.body.includes(COMMENT_MARKER) - ); - - if (existingComment) { - await client.updateIssueComment(existingComment.id, body); - } else if (createIfMissing) { - await client.createIssueComment(issueNumber, body); - } -}