From f6cfc5a1ae4a924443f19ab97f377bbcf7b99802 Mon Sep 17 00:00:00 2001 From: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Date: Tue, 28 Jul 2026 22:36:15 +0800 Subject: [PATCH] feat(lint): run ESLint on PRs, and cover every package (#2923) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `lint.yml` was `workflow_dispatch`-only, so ESLint had never gated a PR. That mattered more than it looked: eslint.config.js sets three `object-ui/*` rules to `error` *specifically* so a new violation fails CI — ADR-0054 Phase 5's no-synthetic-event-trigger, #2879's no-try-catch-around-hook, and the objectql.ts no-inline-spec-config ratchet. Each author pre-cleaned the existing sites so the rule would "lint clean today". All three were inert, because nothing ran them. Same two-layer hole as #2911's type-check gate, and the second layer bites here too: `turbo run lint` silently skips packages with no `lint` script, and 7 of 45 had none — including `apps/console`, the largest surface in the repo. - lint.yml now runs on pull_request + push, keeping workflow_dispatch. - scripts/check-lint-coverage.mjs: every package must lint or be a declared gap, and the list only shrinks. Runs before install (reads package.json only). - lint scripts added to the 5 packages that were already clean: site, vscode-extension, and the three examples. Coverage 38 -> 43 of 45. - console (14 errors) and runner (3) declared in DEBT, tracked in #2927. These are pre-existing and were merely invisible; this does not turn them red. Two config-level fixes, both generated-or-misparsed rather than code debt — they are why `apps/site` looked like it had 7 errors and console 15: - ignore `**/.source` — fumadocs-mdx codegen for apps/site, already gitignored. Linting generated output only reports on the generator's choices. - ignore `**/tailwind.config.js` — authored in TypeScript despite the `.js` extension, so the base JS parser failed on `import type`. - drop a stale `eslint-disable-next-line @next/next/no-img-element` in apps/site: the Next plugin is not in the flat config, so the directive itself errored as an unknown rule. Replaced with a comment stating why a plain is correct there. `--max-warnings` deliberately not set: the 7,724 warnings repo-wide are 81% no-explicit-any, plus React Compiler rules the config downgrades on purpose. This gate is about errors. Proven before being trusted — planting a hook inside try/catch in a linted package now yields `pnpm lint` exit 1 and `Failed: @object-ui/i18n#lint`, which was impossible before. Coverage guard verified red in all three modes (new scriptless package, DEBT entry outliving its gap, script removed from a covered package). Refs #2911, #2923, #2927 Co-Authored-By: Claude --- .github/workflows/lint.yml | 30 +++++ apps/site/app/(home)/page.tsx | 3 +- apps/site/package.json | 1 + eslint.config.js | 16 ++- examples/byo-backend-console/package.json | 1 + examples/console-starter/package.json | 1 + examples/hello-world/package.json | 3 + package.json | 1 + packages/vscode-extension/package.json | 1 + scripts/check-lint-coverage.mjs | 129 ++++++++++++++++++++++ 10 files changed, 184 insertions(+), 2 deletions(-) create mode 100644 scripts/check-lint-coverage.mjs diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index f6d5836a3..2e6cfff39 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -1,6 +1,30 @@ name: Lint +# Until #2923 this workflow was `workflow_dispatch`-only, so ESLint had never +# gated a PR. That mattered more than it looked: `eslint.config.js` sets three +# `object-ui/*` rules to `error` *specifically* so a new violation fails CI +# (ADR-0054 Phase 5, #2879, and the objectql.ts ratchet), and each author +# pre-cleaned the existing sites so the rule would lint clean. All three +# ratchets were inert because nothing ran them. +# +# `--max-warnings` is deliberately not set: the 7,724 warnings repo-wide are 81% +# `no-explicit-any`, plus React Compiler rules the config downgrades on purpose. +# This gate is about errors. on: + push: + branches: [main, develop] + paths-ignore: + - '**/*.md' + - 'content/**' + - 'docs/**' + - '.changeset/**' + pull_request: + branches: [main, develop] + paths-ignore: + - '**/*.md' + - 'content/**' + - 'docs/**' + - '.changeset/**' workflow_dispatch: concurrency: @@ -33,6 +57,12 @@ jobs: node-version: '20.x' cache: 'pnpm' + # Every package must run ESLint or be declared a known gap. turbo skips + # scriptless packages silently, so without this a package reads as clean + # because nothing linted it. Runs before install: only reads package.json. + - name: Verify lint coverage + run: node scripts/check-lint-coverage.mjs + - name: Turbo Cache uses: actions/cache@v6 with: diff --git a/apps/site/app/(home)/page.tsx b/apps/site/app/(home)/page.tsx index 34ce10b65..882d47d87 100644 --- a/apps/site/app/(home)/page.tsx +++ b/apps/site/app/(home)/page.tsx @@ -88,7 +88,8 @@ export default function HomePage() { rel="noopener noreferrer" className="inline-block transition hover:opacity-80" > - {/* eslint-disable-next-line @next/next/no-img-element */} + {/* Plain , not next/image: these are third-party badge + endpoints that render their own SVG and need no optimisation. */} {b.alt} [p.name, p])); + +const errors = []; + +// 1. Undeclared gap — a package that never runs ESLint. +for (const pkg of packages) { + if (pkg.hasScript) continue; + if (DEBT[pkg.name]) continue; + errors.push( + `${pkg.name} (${pkg.dir}) has no "lint" script, so \`pnpm lint\` skips it entirely.\n` + + ` Add "lint": "eslint ." to its package.json. If it still has ESLint errors,\n` + + ` add it to DEBT in scripts/check-lint-coverage.mjs with an error count.` + ); +} + +// 2. Ratchet — a declared gap that has been closed must leave the list. +for (const name of Object.keys(DEBT)) { + const pkg = byName.get(name); + if (!pkg) { + errors.push(`${name} is listed in DEBT but is not a workspace package any more — delete the entry.`); + } else if (pkg.hasScript) { + errors.push( + `${name} now has a "lint" script — delete its DEBT entry so the gap cannot reopen` + + `${DEBT[name].issue ? ` (and close #${DEBT[name].issue} if it is done)` : ""}.` + ); + } +} + +// ── Report ─────────────────────────────────────────────────────────────────── +const linted = packages.filter((p) => p.hasScript).length; +const debtCount = Object.keys(DEBT).length; +const debtErrors = Object.values(DEBT).reduce((sum, d) => sum + d.errors, 0); + +if (errors.length === 0) { + console.log( + `✅ lint coverage: ${linted}/${packages.length} packages linted, ` + + `${debtCount} with outstanding errors (${debtErrors} total).` + ); + process.exit(0); +} + +console.error("❌ lint coverage regressed:\n"); +for (const message of errors) { + console.error(` • ${message}`); +} +console.error( + "\nA package with no `lint` script is not clean — turbo skips it and CI sees nothing.\n" + + "See https://github.com/objectstack-ai/objectui/issues/2923 for why this guard exists." +); +process.exit(1);