Skip to content

Commit cf20681

Browse files
os-zhuangclaude
andauthored
feat(lint): run ESLint on PRs, and cover every package (#2923) (#2928)
`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 <img> 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: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
1 parent 8943178 commit cf20681

10 files changed

Lines changed: 184 additions & 2 deletions

File tree

.github/workflows/lint.yml

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,30 @@
11
name: Lint
22

3+
# Until #2923 this workflow was `workflow_dispatch`-only, so ESLint had never
4+
# gated a PR. That mattered more than it looked: `eslint.config.js` sets three
5+
# `object-ui/*` rules to `error` *specifically* so a new violation fails CI
6+
# (ADR-0054 Phase 5, #2879, and the objectql.ts ratchet), and each author
7+
# pre-cleaned the existing sites so the rule would lint clean. All three
8+
# ratchets were inert because nothing ran them.
9+
#
10+
# `--max-warnings` is deliberately not set: the 7,724 warnings repo-wide are 81%
11+
# `no-explicit-any`, plus React Compiler rules the config downgrades on purpose.
12+
# This gate is about errors.
313
on:
14+
push:
15+
branches: [main, develop]
16+
paths-ignore:
17+
- '**/*.md'
18+
- 'content/**'
19+
- 'docs/**'
20+
- '.changeset/**'
21+
pull_request:
22+
branches: [main, develop]
23+
paths-ignore:
24+
- '**/*.md'
25+
- 'content/**'
26+
- 'docs/**'
27+
- '.changeset/**'
428
workflow_dispatch:
529

630
concurrency:
@@ -33,6 +57,12 @@ jobs:
3357
node-version: '20.x'
3458
cache: 'pnpm'
3559

60+
# Every package must run ESLint or be declared a known gap. turbo skips
61+
# scriptless packages silently, so without this a package reads as clean
62+
# because nothing linted it. Runs before install: only reads package.json.
63+
- name: Verify lint coverage
64+
run: node scripts/check-lint-coverage.mjs
65+
3666
- name: Turbo Cache
3767
uses: actions/cache@v6
3868
with:

apps/site/app/(home)/page.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,8 @@ export default function HomePage() {
8888
rel="noopener noreferrer"
8989
className="inline-block transition hover:opacity-80"
9090
>
91-
{/* eslint-disable-next-line @next/next/no-img-element */}
91+
{/* Plain <img>, not next/image: these are third-party badge
92+
endpoints that render their own SVG and need no optimisation. */}
9293
<img
9394
src={b.src}
9495
alt={b.alt}

apps/site/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
"scripts": {
66
"prebuild": "pnpm --filter @object-ui/example-schema-catalog build",
77
"build": "next build",
8+
"lint": "eslint .",
89
"dev": "next dev",
910
"start": "next start",
1011
"types:check": "fumadocs-mdx && next typegen && tsc --noEmit",

eslint.config.js

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,21 @@ import reactRefresh from 'eslint-plugin-react-refresh'
1313
import tseslint from 'typescript-eslint'
1414
import objectUi from './eslint-rules/index.js'
1515

16-
export default tseslint.config({ ignores: ['**/dist', '**/.next', '**/node_modules', '**/public'] }, {
16+
export default tseslint.config({
17+
ignores: [
18+
'**/dist',
19+
'**/.next',
20+
'**/node_modules',
21+
'**/public',
22+
// fumadocs-mdx codegen for apps/site (gitignored — see apps/site/.gitignore).
23+
// Linting generated output only reports on the generator's choices.
24+
'**/.source',
25+
// Tailwind configs are authored in TypeScript despite the `.js` extension,
26+
// and the TS parser below is scoped to `.ts`/`.tsx`, so the base JS parser
27+
// would fail on `import type`. Build config, not app code.
28+
'**/tailwind.config.js',
29+
],
30+
}, {
1731
extends: [js.configs.recommended, ...tseslint.configs.recommended],
1832
files: ['**/*.{ts,tsx}'],
1933
languageOptions: {

examples/byo-backend-console/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
"scripts": {
88
"dev": "vite",
99
"build": "tsc && vite build",
10+
"lint": "eslint .",
1011
"type-check": "tsc --noEmit",
1112
"preview": "vite preview"
1213
},

examples/console-starter/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
"scripts": {
88
"dev": "vite",
99
"build": "tsc && vite build",
10+
"lint": "eslint .",
1011
"type-check": "tsc --noEmit",
1112
"preview": "vite preview"
1213
},

examples/hello-world/package.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,5 +9,8 @@
99
"@object-ui/react": "workspace:*",
1010
"react": "19.2.8",
1111
"react-dom": "19.2.8"
12+
},
13+
"scripts": {
14+
"lint": "eslint ."
1215
}
1316
}

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
"test:ui": "vitest --ui",
3131
"test:coverage": "vitest run --coverage",
3232
"lint": "turbo run lint",
33+
"lint:coverage": "node scripts/check-lint-coverage.mjs",
3334
"type-check": "turbo run type-check",
3435
"type-check:coverage": "node scripts/check-type-check-coverage.mjs",
3536
"cli": "node packages/cli/dist/cli.js",

packages/vscode-extension/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,7 @@
192192
"scripts": {
193193
"vscode:prepublish": "pnpm run build",
194194
"build": "tsup",
195+
"lint": "eslint .",
195196
"dev": "tsup --watch",
196197
"test": "vitest run --passWithNoTests",
197198
"test:watch": "vitest",

scripts/check-lint-coverage.mjs

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
#!/usr/bin/env node
2+
/**
3+
* Validates that every workspace package is actually linted by CI.
4+
*
5+
* The sibling of scripts/check-type-check-coverage.mjs, for the same reason:
6+
* `pnpm lint` runs `turbo run lint`, and turbo silently skips any package with
7+
* no `lint` script — so a package without one is not "clean", it is unlinted.
8+
* That is how `apps/console`, the largest surface in the repo, went unlinted
9+
* while carrying 14 ESLint errors that nobody had ever seen (#2923).
10+
*
11+
* The stakes are concrete: `eslint.config.js` sets three `object-ui/*` rules to
12+
* `error` specifically so a new violation fails CI (ADR-0054 Phase 5, #2879,
13+
* and the objectql.ts type-discipline ratchet). Those ratchets are worthless in
14+
* a package that never runs ESLint.
15+
*
16+
* Run: node scripts/check-lint-coverage.mjs
17+
* Exit: 0 = OK, 1 = coverage regressed or the list is stale
18+
*/
19+
20+
import { readFileSync, readdirSync, statSync } from "fs";
21+
import { resolve, dirname, join } from "path";
22+
import { fileURLToPath } from "url";
23+
24+
const root = resolve(dirname(fileURLToPath(import.meta.url)), "..");
25+
26+
// ── Known gaps ───────────────────────────────────────────────────────────────
27+
// Packages with outstanding ESLint *errors*, so they cannot carry a `lint`
28+
// script without turning CI red. Warnings do not matter here — ESLint only
29+
// fails on errors unless `--max-warnings` is set, and it deliberately is not
30+
// (7,724 warnings repo-wide are dominated by `no-explicit-any`; see #2923).
31+
//
32+
// Every entry is real debt: fix the errors, add `"lint": "eslint ."`, then
33+
// delete the entry.
34+
const DEBT = {
35+
"@object-ui/console": {
36+
errors: 14,
37+
issue: 2927,
38+
note: "8x react-hooks/purity (Date.now during render) + 4x react-hooks/static-components (components created during render reset state) + no-useless-assignment + prefer-const",
39+
},
40+
"@object-ui/runner": {
41+
errors: 3,
42+
issue: 2927,
43+
note: "3x react-hooks/static-components in LayoutRenderer",
44+
},
45+
};
46+
47+
// ── Collect workspace packages ───────────────────────────────────────────────
48+
const GROUPS = ["packages", "apps", "examples"];
49+
50+
function collect() {
51+
const out = [];
52+
for (const group of GROUPS) {
53+
let entries;
54+
try {
55+
entries = readdirSync(resolve(root, group));
56+
} catch {
57+
continue;
58+
}
59+
for (const entry of entries) {
60+
const dir = join(group, entry);
61+
const manifest = resolve(root, dir, "package.json");
62+
try {
63+
statSync(manifest);
64+
} catch {
65+
continue;
66+
}
67+
const pkg = JSON.parse(readFileSync(manifest, "utf8"));
68+
if (!pkg.name) continue;
69+
out.push({
70+
name: pkg.name,
71+
dir,
72+
hasScript: Boolean(pkg.scripts?.lint),
73+
});
74+
}
75+
}
76+
return out;
77+
}
78+
79+
const packages = collect();
80+
const byName = new Map(packages.map((p) => [p.name, p]));
81+
82+
const errors = [];
83+
84+
// 1. Undeclared gap — a package that never runs ESLint.
85+
for (const pkg of packages) {
86+
if (pkg.hasScript) continue;
87+
if (DEBT[pkg.name]) continue;
88+
errors.push(
89+
`${pkg.name} (${pkg.dir}) has no "lint" script, so \`pnpm lint\` skips it entirely.\n` +
90+
` Add "lint": "eslint ." to its package.json. If it still has ESLint errors,\n` +
91+
` add it to DEBT in scripts/check-lint-coverage.mjs with an error count.`
92+
);
93+
}
94+
95+
// 2. Ratchet — a declared gap that has been closed must leave the list.
96+
for (const name of Object.keys(DEBT)) {
97+
const pkg = byName.get(name);
98+
if (!pkg) {
99+
errors.push(`${name} is listed in DEBT but is not a workspace package any more — delete the entry.`);
100+
} else if (pkg.hasScript) {
101+
errors.push(
102+
`${name} now has a "lint" script — delete its DEBT entry so the gap cannot reopen` +
103+
`${DEBT[name].issue ? ` (and close #${DEBT[name].issue} if it is done)` : ""}.`
104+
);
105+
}
106+
}
107+
108+
// ── Report ───────────────────────────────────────────────────────────────────
109+
const linted = packages.filter((p) => p.hasScript).length;
110+
const debtCount = Object.keys(DEBT).length;
111+
const debtErrors = Object.values(DEBT).reduce((sum, d) => sum + d.errors, 0);
112+
113+
if (errors.length === 0) {
114+
console.log(
115+
`✅ lint coverage: ${linted}/${packages.length} packages linted, ` +
116+
`${debtCount} with outstanding errors (${debtErrors} total).`
117+
);
118+
process.exit(0);
119+
}
120+
121+
console.error("❌ lint coverage regressed:\n");
122+
for (const message of errors) {
123+
console.error(` • ${message}`);
124+
}
125+
console.error(
126+
"\nA package with no `lint` script is not clean — turbo skips it and CI sees nothing.\n" +
127+
"See https://github.com/objectstack-ai/objectui/issues/2923 for why this guard exists."
128+
);
129+
process.exit(1);

0 commit comments

Comments
 (0)