chore(repo): lint every package, not just the ones that declare a script - #646
Merged
Conversation
`pnpm -r check:lint` fans out only to packages that DEFINE a `check:lint`
script. Exactly one did — core — so ESLint had never run against the other 35
`packages/*`, including the repo's own `no-restricted-syntax` ratchet. A banned
`...(x !== undefined ? { k: x } : {})` spread in a companion passed every local
gate (#451); the identical pattern in core failed, purely because core happened
to own the script.
`check:lint` is now `node scripts/check-lint.mjs`, which enumerates
`packages/*` on DISK and lints each one's `src/` and `test/`. Coverage no longer
depends on a package opting in, so a new package is gated the moment it exists.
Verified by probe: re-introducing the #451 spread in `@stitchapi/redis` fails
the gate, and passes again when reverted.
Rules move from `packages/core/eslint.config.ts` to a root `eslint.config.ts`,
unchanged. Core's `eslint-suppressions.json` regenerates byte-for-byte
identical under the new setup, which is the check that the move preserved its
semantics exactly.
One eslint process per package, not one whole-workspace pass. The rules are
type-aware, so a single pass has to hold all 36 TypeScript programs in one heap:
that dies with SIGABRT at ~4.7 GB RSS under Node's default heap — with a
tsconfig glob AND with `projectService` — which would have made the gate
unrunnable in CI. Sharded, it peaks at 1.3 GB and finishes in ~10s (faster than
the single pass, since the shards run four-up). Per-package cwd also anchors
`parserOptions.project` and the `eslint-suppressions.json` lookup, which is why
core's baseline keeps working untouched.
The pass surfaced 554 violations. 211 are fixed here:
- The `no-restricted-syntax` spread in `@stitchapi/vercel-ai` — the #451 class.
A pre-existing comment already explained that `compact` is wrong here
(`parameters`/`inputSchema` are required `unknown` keys it would optionalize),
so it takes the inline disable core's config prescribes for that case and
already uses at engine.ts:1380.
- `require-yield` in rtk-query's never-ending stream fake; `no-prototype-builtins`
in sandbox-sim (→ `Object.hasOwn`); a dead `no-eval` directive in openapi; an
unused import and an unused `TextEncoder` in sandbox-sim.
- ~200 mechanical fixes from a `--fix` pass restricted to fixers that cannot
change behaviour (`array-type`, `consistent-type-definitions`,
`consistent-type-imports`, `no-unnecessary-type-arguments`/`-assertion`/
`-conversion`, `prefer-optional-chain`/`-includes`/`-regexp-exec`), plus the
same optional-chain fix hand-applied to seven copies of one test helper.
`--fix-type` excludes `directive`, because a plain `--fix` deletes
`eslint-disable` comments it considers unused and ate ten of them — core's
included — on the first attempt.
The `--fix` also rewrote elysia's `StitchContext` from a `type` to an
`interface`, breaking the constraint the comment directly above it warns about
(an interface gets no implicit index signature, so it fails Elysia's
`SingletonBase`). Reverted, with the disable that comment always implied.
`pnpm check:types` catches it — elysia is green on main and was red with the
rewrite.
The remaining 343 are baselined per package with ESLint's bulk-suppressions
ratchet — the mechanism core already used, and which the history shows being
burned down rather than grown. They are dominated by work that needs real
per-package decisions, not lint edits: 120 `no-unsafe-member-access` from the
fingerprint packages walking untyped third-party schema internals, and 41
`only-throw-error` whose fix would change what those packages throw, i.e. public
behaviour. New violations fail; these do not.
Two rule options rather than suppressions, both because the alternative was
editing code that is already correct:
- `no-unused-vars` gets `^_` ignore patterns. Every package compiles under
`noUnusedLocals`/`noUnusedParameters`, which TypeScript itself exempts
`_`-prefixed bindings from — so 54 of the 55 hits were the compiler's own
convention, and the two gates disagreed about the same files.
- `vitest/expect-expect` learns `assertConformance`, the shared store-contract
runner, so those suites stop reading as assertion-free.
Also here because the gate needs them: `sandbox-sim/tsconfig.test.json` gets
`"exclude": []` — `exclude` is inherited through `extends`, so the base config's
`**/*.test.ts` carried over and the project resolved to ZERO test files, making
it dead config that nothing ran. It now covers its 9 tests and typechecks clean.
Plain-JS packages (`completions-plugin`, .mjs with no tsconfig) drop to the
syntactic rule set instead of being excluded, since the type-aware rules throw
rather than skip when there is no program.
CI and the lefthook hooks already call `pnpm check:lint`, so nothing in
verify.yml or lefthook.yml changes.
Closes #457
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #457
The hole
pnpm -r check:lintfans out only to packages that define acheck:lintscript. Exactly one did —packages/core— so ESLint had never run against the other 35packages/*, the repo'sno-restricted-syntaxratchet included. That is why the banned...(x !== undefined ? { k: x } : {})spread in #451 passed every local gate while the identical pattern in core failed: core just happened to own the script.(The issue names
@stitchapi/downloadas the package that regressed; there is no such package onmaintoday, so this PR covers the 35 that do exist.)Approach: a single root pass (the issue's option b), sharded
check:lintis nownode scripts/check-lint.mjs, which enumeratespackages/*on disk and lints each one'ssrc/andtest/. Coverage no longer depends on a package opting in, so a new package is gated the moment it exists — the failure mode the issue asked to close. Per-package scripts (option a) would have left the same hole onepackage.jsonedit away.Rules move from
packages/core/eslint.config.tsto a rooteslint.config.ts, unchanged. The check that the move preserved core's semantics exactly: core'seslint-suppressions.jsonregenerates byte-for-byte identical.Proof the ratchet works — re-introducing the #451 spread in
@stitchapi/redis(a package that has never been linted):…and clean again when reverted.
Why one process per package, not one pass
The rules are type-aware, so a single whole-workspace pass must hold all 36 TypeScript programs in one heap. It dies with SIGABRT at ~4.7 GB RSS under Node's default heap — with a tsconfig glob and with
projectService— which would have made the gate unrunnable in CI. Sharded four-up it peaks at 1.3 GB and finishes in ~10s, faster than the single pass. Per-packagecwdalso anchorsparserOptions.projectand theeslint-suppressions.jsonlookup, which is why core's baseline keeps working untouched.CI and lefthook already call
pnpm check:lint, soverify.ymlandlefthook.ymlare unchanged.What the pass surfaced: 554 violations
Fixed — 211
no-restricted-syntax@stitchapi/vercel-ai. A pre-existing comment there already explainedcompactis wrong (parameters/inputSchemaare requiredunknownkeys it would optionalize), so it takes the inline disable core's own config prescribes for that case and already uses atengine.ts:1380.require-yieldyield undefined as neverafter the never-settling await.no-prototype-builtinsObject.hasOwn.no-evaldisable in openapi.no-unused-varsTextEncoderin sandbox-sim.--fixrestricted to fixers that cannot change behaviour:array-type,consistent-type-definitions,consistent-type-imports,no-unnecessary-type-arguments/-assertion/-conversion,prefer-optional-chain/-includes/-regexp-exec. Plus the same optional-chain fix hand-applied to 7 copies of one test helper the fixer wouldn't touch.Two things worth flagging from the
--fix:--fix-typeexcludesdirective. A plain--fixdeleteseslint-disablecomments it thinks are unused and ate ten of them — core'sno-restricted-syntaxjustification included — on the first attempt. Caught and redone.--fixrewrote elysia'sStitchContextfrom atypeto aninterface, breaking the exact constraint the comment directly above it warns about (an interface gets no implicit index signature → fails Elysia'sSingletonBase). Reverted, with the disable that comment always implied.pnpm check:typescatches this: elysia is green onmainand was red with the rewrite.Baselined — 343
Per package, via ESLint's bulk-suppressions ratchet — the mechanism core already used, and which
git logshows being burned down rather than grown. New violations fail; these do not. No blanketeslint-disablecomments were added.They are dominated by work needing real per-package decisions, not lint edits:
no-unsafe-member-access—fingerprint-zod/-effectwalking untyped third-party schema internals. Fixing means typing those internals.only-throw-error— the fingerprint packages throw non-Errorvalues. Changing that changes public behaviour, so per the brief I left it.no-confusing-void-expression, 32no-empty-function, 26no-unnecessary-condition(defensive guards for untyped callers), and a long tail.Regenerate/burn down with
node scripts/check-lint.mjs --suppress-all/--prune-suppressions(verified idempotent).Config decisions (the complete list)
Two rule options — not suppressions — because the alternative was editing code that is already correct:
no-unused-varsgets^_ignore patterns. Every package compiles undernoUnusedLocals/noUnusedParameters, which TypeScript itself exempts_-prefixed bindings from. 54 of the 55 hits were the compiler's own convention; without this the two gates contradict each other on the same file. Same reasoning as thedot-notationtuning core already documents.vitest/expect-expectlearnsassertConformance, the shared store-contract runner, so those suites stop reading as assertion-free.Three inline disables, each with a
--justification: the vercel-ai spread, the elysiatype, and one deno-kv type-level test whose assertion is a@ts-expect-error(nothing toexpect()).One plain-JS accommodation:
.js/.mjs/.cjsdrop to the syntactic rule set, because type-aware rules throw rather than skip with no program. This keepscompletions-plugin(pure.mjs, no tsconfig) in the gate instead of excluded.Drive-by the gate required
packages/sandbox-sim/tsconfig.test.jsongets"exclude": [].excludeis inherited throughextends, so the base config's**/*.test.tscarried over and this project resolved to zero test files — dead config that nothing ran, despite its own comment claiming it type-checks the suite. It now covers its 9 tests andtsc -p tsconfig.test.jsonpasses. Needed here because those tests are colocated insrc/and had no lint project.Deferred
eval-harness'srunner/score/tasks, core'sbin/scripts/test-d. The runner coverssrc+test, per the issue.react-hooks/exhaustive-depsis referenced by a disable comment in@stitchapi/reactbut the plugin isn't installed; baselined rather than adding a dependency in this PR.scripts/check-companion-exports.mjshas the same opt-in bug — a hardcoded 8-package list. Untouched here.Verification
Every command run from the repo root, all green:
pnpm check:lint✓ eslint clean across 36 packages(was: core only)pnpm check:typespnpm testpnpm check:formatpnpm format)pnpm check:contractpnpm check:unknown-keyspnpm check:types-dpnpm check:exportspnpm check:exports:companionspnpm check:changelog/check:release/check:docs-linksBaseline confirmed by stashing:
check:formatandcheck:typeswere green onorigin/mainbefore this branch, so the reformatting and the elysia catch are attributable to this change, not pre-existing drift.🤖 Generated with Claude Code