lint never ran on PRs, and turbo skipped 7 packages — three error ratchets were inert #2923

Activity

1. os-zhuang commented on Jul 28, 2026

   

   os-zhuang

   on Jul 28, 2026

   ContributorAuthor

   More actions

   Correcting my own recommendation above: wiring lint.yml is worth doing now, and I had this backwards.

   I said it would be a no-op gate. It is not. eslint.config.js already carries three error-level rules whose stated purpose is to fail CI — and because lint.yml only runs on workflow_dispatch, all three have never gated anything.

   // ADR-0054 Phase 5 ratchet — ban synthetic-event triggers (C1). Error so a
   // new violation fails CI; the existing surfaces were converted to direct
   // idempotent commands first, so this lints clean today.
   'object-ui/no-synthetic-event-trigger': 'error',
   
   // objectui#2879 ratchet — a hook called inside try/catch desyncs hook order
   // when the catch swallows a throw. ... Error so a tenth copy fails CI; all
   // known sites were converted first, so this lints clean today.
   'object-ui/no-try-catch-around-hook': 'error',
   
   // ... Error so a new inline mirror fails CI; the covered fields were
   // converted first, so this lints clean today.
   'object-ui/no-inline-spec-config': 'error',   // scoped to packages/types/src/objectql.ts

   Three separate deliberate efforts — ADR-0054 Phase 5, #2879, and the objectql.ts type-discipline ratchet — each written on the assumption that CI enforces it. Each author even pre-cleaned the violations so the rule would "lint clean today." The enforcement they were built against does not exist. That is the same shape as #2911, one layer up: the rule is authored, the runtime never runs it.

   And the cost of switching it on is zero: pnpm lint reports 0 errors across all 38 packages today, precisely because those sites were pre-cleaned. Wiring the workflow activates three intended ratchets immediately with no cleanup.

   So the corrected recommendation is wire lint.yml on PRs now — not for the reason I originally gave (it still would not have caught #2911; see below), but because three ratchets are currently inert.

   What wiring it does and does not buy

   Does: activates the three object-ui/* ratchets, so a new synthetic-event trigger, a new hook-in-try/catch, or a new inline spec mirror fails a PR — as their authors intended.

   Does not: catch #2911. no-unused-vars is 'warn', and warnings never fail without --max-warnings. That part of my original analysis stands.

   Actual composition of the 7,724 warnings

   Now measured properly rather than sampled:

   rule	count	share
   @typescript-eslint/no-explicit-any	6,262	81%
   react-refresh/only-export-components	486	6%
   @typescript-eslint/no-unused-vars	322	4%
   react-hooks/set-state-in-effect	252	3%
   react-hooks/exhaustive-deps	177	2%
   react-hooks/refs	111	1%
   react-hooks/use-memo	33
   react-hooks/preserve-manual-memoization	32
   react-hooks/immutability	12
   react-hooks/incompatible-library	2

   One rule is 81% of the noise. That reframes option 2 above: a --max-warnings baseline of 7,724 is really "6,262 anys plus 1,462 of everything else," and no-explicit-any is a style/discipline call on a codebase whose adapters legitimately use any. Ratcheting the aggregate mostly ratchets that one rule.

   Note also that five of the react-hooks/* rules are explicitly downgraded with the comment "Downgrade new React Compiler rules to warnings (codebase predates these rules)" — a deliberate choice, not neglect. Those ~440 should stay warnings.

   All 322 no-unused-vars are the 'X' is defined but never used / is assigned a value but never used form — i.e. exactly the #2911 class.

   Revised plan

   1. Wire lint.yml to pull_request / push as-is. Free, no cleanup, activates three dead ratchets. Do this first and separately — it is a pure win and easy to review.
   2. Then, optionally, promote @typescript-eslint/no-unused-vars to error after clearing its 322 violations. argsIgnorePattern: '^_' is already configured, so unused params just need _ prefixes; unused imports/locals are mostly deletions. This is the piece that would have caught plugin-map 类型检查在 main 上是红的（maplibre-gl@6 移除默认导出）；根因是 CI 根本不跑 type-check，22 个包的类型错误对 CI 完全不可见 #2911.
   3. Leave no-explicit-any and the React Compiler rules at warn. 6,262 + ~440, both deliberate. Do not put them behind --max-warnings without a separate decision.

   Do not add --max-warnings=7724 as a blanket cap — it reads as a health signal while being dominated by one stylistic rule.
2. os-zhuang

   changed the title [-]lint never runs on PRs — but wiring it as-is would be a no-op gate (7,724 warnings, 0 errors)[/-] [+]lint never ran on PRs, and turbo skipped 7 packages — three `error` ratchets were inert[/+] on Jul 28, 2026
3. os-zhuang

   added a commit that references this issue on Jul 28, 2026

   feat(lint): run ESLint on PRs, and cover every package (#2923) (#2928)

   ...

   cf20681
4. os-zhuang

   closed this as completedin #2928on Jul 28, 2026
5. os-zhuang

   added a commit that references this issue on Jul 28, 2026

   fix(console,runner): render the approvals inbox against one ticking c…

   ...

   b076050