Lint: stop working around rules that don't fit tests, e2e and the API #398

New issue

Closed

opened 2026-10-14 12:34:17 +00:00 by root · 3 comments

root commented

2026-10-14 12:34:17 +00:00

(Migrated from gitlab.core-project.space)

Problem

An audit of the lint config (on the accounts spec's branch, after the owner's new-cap, max-params and max-lines changes landed in !523) found rules the code keeps working around: suppressions, the same override copied per package, and test describe blocks split only to fit a size limit. They cost more than they catch in tests, in apps/e2e and in the API.

Changes, one lint merge request, in order

One shared test override in the root .oxlintrc.json: add **/*.handlers.ts (the front's MSW fakes) to its files, and turn off eslint/max-lines-per-function, eslint/no-await-in-loop, eslint/init-declarations and promise/avoid-new. Drop the copies of these in the API, front and e2e overrides.
- Delete the suppressions in room-footer.test.tsx, room-offers.test.tsx and room.handlers.ts.
- Optional: merge the describes split only for size (auth.signUp in four, the "own move" trio in room.test.tsx), and fold browserOn back into connect in rpc.fixtures.ts.
eslint/max-lines-per-function in source: keep 80, with skipBlankLines: true and skipComments: true.
apps/e2e runs in Node: a root override with env: { node: true } and import/no-nodejs-modules: off. Check that oxlint accepts env inside an override. Delete the three no-undef suppressions and the playwright.config.ts override.
unicorn/no-null off for apps/api: Drizzle rows and the wire's .nullable() are null-shaped, and strictNullChecks already catches misuse; the front has it off already. Delete the file-by-file allowlist. Keep it on in the engine.
Narrow unicorn/require-array-join-separator in apps/api from all of room/ to the one file defining the Room's join (a false positive on a method name). Optionally allow reflect-metadata in import/no-unassigned-import and drop app.ts's suppression.

Stays as it is

typescript/consistent-type-definitions: its 3 suppressions are declaration merging, which needs an interface.
no-magic-numbers: one suppression, a cubic-bezier value.
promise/prefer-await-to-then in games.service.ts: it flags a real fire-and-forget.
front/no-vi-mock in router.harness.tsx, the scoped offs in use-room-changes.ts, move-mutation.ts's no-set-query-data, the shadcn folder, jsx-a11y/anchor-is-valid, id-length, prefer-destructuring.

Acceptance criteria

Steps 1–5 land in one merge request, each removing the suppressions and override copies it makes unnecessary
No new suppression is added
The gate is green

Out of scope

The front's no-restricted-imports blocks, repeated nearly word for word for each of its 6 verticals (about 1,000 lines of config): worth its own merge request.
new-cap, max-params and max-lines: already changed in !523.

## Problem An audit of the lint config (on the accounts spec's branch, after the owner's `new-cap`, `max-params` and `max-lines` changes landed in !523) found rules the code keeps working around: suppressions, the same override copied per package, and test `describe` blocks split only to fit a size limit. They cost more than they catch in tests, in `apps/e2e` and in the API. ## Changes, one lint merge request, in order 1. **One shared test override** in the root `.oxlintrc.json`: add `**/*.handlers.ts` (the front's MSW fakes) to its files, and turn off `eslint/max-lines-per-function`, `eslint/no-await-in-loop`, `eslint/init-declarations` and `promise/avoid-new`. Drop the copies of these in the API, front and e2e overrides. - Delete the suppressions in `room-footer.test.tsx`, `room-offers.test.tsx` and `room.handlers.ts`. - Optional: merge the describes split only for size (`auth.signUp` in four, the "own move" trio in `room.test.tsx`), and fold `browserOn` back into `connect` in `rpc.fixtures.ts`. 2. **`eslint/max-lines-per-function` in source:** keep 80, with `skipBlankLines: true` and `skipComments: true`. 3. **`apps/e2e` runs in Node:** a root override with `env: { node: true }` and `import/no-nodejs-modules: off`. Check that oxlint accepts `env` inside an override. Delete the three `no-undef` suppressions and the `playwright.config.ts` override. 4. **`unicorn/no-null` off for `apps/api`:** Drizzle rows and the wire's `.nullable()` are null-shaped, and `strictNullChecks` already catches misuse; the front has it off already. Delete the file-by-file allowlist. Keep it on in the engine. 5. **Narrow `unicorn/require-array-join-separator`** in `apps/api` from all of `room/` to the one file defining the Room's `join` (a false positive on a method name). Optionally allow `reflect-metadata` in `import/no-unassigned-import` and drop `app.ts`'s suppression. ## Stays as it is - `typescript/consistent-type-definitions`: its 3 suppressions are declaration merging, which needs an `interface`. - `no-magic-numbers`: one suppression, a cubic-bezier value. - `promise/prefer-await-to-then` in `games.service.ts`: it flags a real fire-and-forget. - `front/no-vi-mock` in `router.harness.tsx`, the scoped offs in `use-room-changes.ts`, `move-mutation.ts`'s `no-set-query-data`, the shadcn folder, `jsx-a11y/anchor-is-valid`, `id-length`, `prefer-destructuring`. ## Acceptance criteria - [ ] Steps 1–5 land in one merge request, each removing the suppressions and override copies it makes unnecessary - [ ] No new suppression is added - [ ] The gate is green ## Out of scope - The front's `no-restricted-imports` blocks, repeated nearly word for word for each of its 6 verticals (about 1,000 lines of config): worth its own merge request. - `new-cap`, `max-params` and `max-lines`: already changed in !523.

root commented

2026-10-14 12:43:33 +00:00

(Migrated from gitlab.core-project.space)

mentioned in merge request !523