Skip to content

chore: adopt stricter lint rules and ban z.coerce - #1837

Merged
joshunrau merged 9 commits into
DouglasNeuroInformatics:mainfrom
joshunrau:linting
Oct 29, 2026
Merged

chore: adopt stricter lint rules and ban z.coerce#1837
joshunrau merged 9 commits into
DouglasNeuroInformatics:mainfrom
joshunrau:linting

Conversation

@joshunrau

@joshunrau joshunrau commented Oct 29, 2026

Copy link
Copy Markdown
Collaborator

Adopts the lint rules from the Clinivance platform's oxlint config that fit this repo, and fixes
everything they flag.

  • Custom rules, in .eslint/rules/ and registered as odc/*: route-exports,
    unit-test-subject, require-disable-reason, no-coverage-ignore, multiline-arrow-body,
    class-name-composition, handler-names.
  • Built-in rules:
    • core rules such as curly, eqeqeq, no-nested-ternary and preserve-caught-error;
    • typescript-eslint's no-confusing-void-expression and return-await;
    • React and jsx-a11y additions;
    • import/enforce-node-protocol-usage;
    • three Vitest test-style rules;
    • Playwright's recommended set for testing/src, with every rule at error.
  • z.coerce is banned, including through the z3/z4 aliases.
  • New dev dependencies: @vitest/eslint-plugin and eslint-plugin-playwright, through the
    catalog.

Every eslint-disable now states its reason after --. perfectionist/sort-objects is off for the
playground's example instruments and for instrument-stubs, as it already was for
instrument-library.

API change

Dates now parse through $DateLike in packages/schemas/src/core, which accepts a Date or an
ISO 8601 date-time with a timezone. The API now answers 400 to a numeric timestamp, an empty string,
a date without a time, or a date-time without a timezone. It used to coerce all of these, so a
null on a required date became 1970.

Every client in this repo already sends ISO date-times. odc-cli --min-date was the exception, and
now sends UTC.

Commits

  1. d8f1ecd57: the rules and config.
  2. ad240c3ad, c4b8fe2dc: the test-layout rules, and the unit tests they flagged.
  3. 0efbf44ab: eslint --fix across the repo. It is mechanical, so it can be skimmed.
  4. 1520a9cf1: date and number parsing.
  5. 83de59345: hand fixes for everything else.
  6. 1a34ac7fe, 5cd61934b: follow-ups in the CLI and the docs.

Verification

  • pnpm lint: green.
  • pnpm test:coverage: 418 files and 4,887 tests, at 100% on every metric. New tests cover:
    • $DateLike, in packages/schemas/src/core/__tests__/core.test.ts;
    • the audit query, in packages/schemas/src/audit/__tests__/audit.test.ts;
    • number-keyed form options and their JSON Schema, in instrument.form.test.ts;
    • the API's minDate pipe, in instrument-records.controller.test.ts;
    • the gateway and web config parsing.
  • pnpm test:e2e: 278 of 327 passed locally. The other 49 all failed on the API's login
    throttle (429), which parallel workers trip on a local machine (Local end-to-end runs trip the API's login throttle #1835). Re-run with two workers,
    48 passed. subject-detail.spec.ts "should plot a selected measure" failed once, then passed in
    10 reruns.

Problems found along the way, filed separately: #1833, #1834, #1835, #1836.

Co-Authored-By: Claude Opus 5.5 [email protected]

joshunrau and others added 9 commits October 28, 2026 18:13
@joshunrau @claude
chore: adopt stricter lint rules and ban z.coerce
d8f1ecd 
Port seven custom rules into .eslint/rules (registered under odc/): route-exports,
unit-test-subject, require-disable-reason, no-coverage-ignore, multiline-arrow-body,
class-name-composition and handler-names.

Enable the core, typescript-eslint, react and jsx-a11y rules that report little or nothing today,
import/enforce-node-protocol-usage, three vitest test-style rules, and the Playwright recommended
set for testing/src with every rule at error. Ban z.coerce, which accepts inputs like '' and null.

Ignore the generated apps/web/src/route-tree.ts, and update the docs that showed eslint-disable
comments without a reason.

The source is not fixed yet, so pnpm lint fails until the follow-up commits land.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@joshunrau @claude
chore: accept the subject's file name when a unit test imports the mo…
ad240c3 
…dule itself

A test of an entrypoint with no exports, or of what loading a module does, imports the module
itself (a side-effect import, `import * as`, or `await import()`) rather than names, so it had no
name to title its describe after. odc/unit-test-subject now accepts the subject's file name there.

It also requires whatever follows `Name.` in a title to be member names, so a title like
`plugin.onLoad ?raw` no longer passes as a member of `plugin`.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@joshunrau @claude
test: title unit tests after their subject and fix the vitest lint er…
c4b8fe2 
…rors

Every top-level describe is now titled after what it tests, every hook sits inside a describe,
every toThrow names the error the code actually throws, and test() is it().

- Component tests import the component file rather than its folder's index.ts.
- Route tests import Route and read its options, instead of capturing them through a mocked
  createFileRoute.
- Store slice tests build their store from the slice creator they cover.
- The WORD_LIST checks move to word-list.test.ts, and the $CreateRemoteAssignmentsData checks
  move from the gateway's router test to packages/schemas.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@joshunrau @claude
refactor: apply the autofixes of the new lint rules
0efbf44 
Run eslint --fix, as pnpm lint does, then prettier. Multi-line arrow bodies become blocks,
Node built-ins are imported through the node: protocol, and the curly, prefer-template,
object-shorthand, prefer-arrow-callback and JSX autofixes apply.

prefer-arrow-callback now allows named functions. Its autofix had turned the named function
passed to forwardRef into an arrow, losing each component's display name, and had made the
vi.fn constructor mocks in two tests into arrows, which cannot be called with new. Those mocks
are now named functions. The gateway's assignment handler now returns nothing from every
branch, since the void-expression autofix had left it returning a response from only some.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@joshunrau @claude
fix(schemas): parse dates as ISO 8601 date-times instead of coercing …
1520a9c 
…them

z.coerce turned inputs like '' and null into valid values, so a null on a required date became
1970. Dates now go through $DateLike in packages/schemas/src/core, which accepts a Date or an
ISO 8601 date-time (what JSON.stringify makes of a Date) and rejects everything else.

Numbers are parsed strictly too: the audit query and the web app's network latency use libjs's
$NumberLike, the gateway's ports accept only digit strings, and number-keyed form options accept
only numeric keys. The ban now also covers the z3 and z4 aliases.

The API now refuses some inputs it used to accept: a date given as a numeric timestamp, an empty
string, a date without a time, or a date-time without a timezone. Every client in this repository
already sends full ISO date-times.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@joshunrau @claude
refactor: fix the remaining errors of the new lint rules
83de593 
Fix by hand what the new rules flag and eslint --fix cannot: block-bodied promise executors,
void calls as statements, nested ternaries as lookups or early returns, handle-prefixed handler
names, button types, strict equality, cn() for class names, and a parseInt radix.

Every eslint-disable comment states its reason, or is gone where the code could satisfy the rule
instead. perfectionist/sort-objects is off for the playground's example and template instruments
and for instrument-stubs, as it already is for instrument-library, since instrument definitions
are written in reading order.

In the Playwright suite, getPageModel counts as an assertion for expect-expect, the users page
object's selectOption is renamed selectPermissionOption so the plugin stops mistaking it for
Playwright's own, and the playground slice tests bind setState instead of passing it unbound.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@joshunrau @claude
fix: send odc-cli's --min-date with a timezone
1a34ac7 
`odc-cli instrument-records find --min-date 2024-01-31` sent `2024-01-31T00:00:00`, a date-time
without a timezone, which the API now rejects with a 400 since minDate is parsed as $DateLike. A
date given without a timezone is now read as UTC, as `subjects find --min-date` already does.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@joshunrau @claude
docs(web): match the subjectsQueryOptions example to the code
5cd6193 
The autofix of odc/multiline-arrow-body gave useSubjectsQuery.ts a block body, so the canonical
example in apps/web/AGENTS.md now shows the same.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@joshunrau
Merge remote-tracking branch 'upstream/main' into linting
b4de482 
# Conflicts:
#	apps/web/src/components/BulkRemoteAssignmentWizard/__tests__/BulkRemoteAssignmentWizard.test.tsx
#	apps/web/src/components/MailSettings/MailSettings.tsx
#	apps/web/src/utils/__tests__/bulk-assignments.test.ts
#	apps/web/src/utils/bulk-assignments.ts
#	eslint.config.js
@joshunrau
joshunrau merged commit 68844be into DouglasNeuroInformatics:main Oct 29, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@joshunrau