Refactor CSS compatibility checks (#4504)

2026-09-12 05:48:57 +00:00 · 2026-08-18 10:30:43 +02:00
parent fd070ce9ca
commit c64d3a50e8
5 changed files with 91 additions and 241 deletions
@@ -6,7 +6,6 @@ on:

 permissions:
    contents: read
-    issues: write
    pull-requests: read

 concurrency:
@@ -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",
@@ -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<string> {
        return this.getContent(path, ref);
    }
-
-    async listIssueComments(issueNumber: number): Promise<IssueComment[]> {
-        const comments: IssueComment[] = [];
-
-        for (let page = 1; ; page += 1) {
-            const result = await this.request<IssueComment[]>(
-                `/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<void> {
-        await this.request(`/repos/${this.repository}/issues/${issueNumber}/comments`, {
-            body: JSON.stringify({ body }),
-            method: 'POST',
-        });
-    }
-
-    async updateIssueComment(commentId: number, body: string): Promise<void> {
-        await this.request(`/repos/${this.repository}/issues/comments/${commentId}`, {
-            body: JSON.stringify({ body }),
-            method: 'PATCH',
-        });
-    }
 }

 async function getBrowserslist(api: GitHubApi, headSha: string): Promise<string[]> {
@@ -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<boolean> {
+    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<boolean> {
    const token = process.env.GITHUB_TOKEN;
    const repository = process.env.GITHUB_REPOSITORY;
@@ -212,34 +254,19 @@ async function run(): Promise<boolean> {
        );
    }

-    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;
@@ -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<void> {
-        this.created.push(body);
-    }
-
-    async listIssueComments(): Promise<typeof this.comments> {
-        return this.comments;
-    }
-
-    async updateIssueComment(commentId: number, body: string): Promise<void> {
-        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([]);
-    });
-});
@@ -2,9 +2,7 @@ import { diffLines } from 'diff';
 import postcss, { type Declaration } from 'postcss';
 import stylelint from 'stylelint';

-export const COMMENT_MARKER = '<!-- gitbook-css-browser-compatibility -->';
 const COMPATIBILITY_RULE = 'plugin/no-unsupported-browser-features';
-const MAX_COMMENT_DIAGNOSTICS = 50;

 export interface ChangedLines {
    added: Set<number>;
@@ -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<void>;
-    listIssueComments(issueNumber: number): Promise<Comment[]>;
-    updateIssueComment(commentId: number, body: string): Promise<void>;
-}
-
 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<void> {
-    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);
-    }
-}