Repository navigation
feat: support vitest 5 - #742
benquarmby wants to merge 1 commit into
Conversation
BREAKING CHANGE: Support for vitest versions lower than 5 has been removed. Consumers must upgrade to a supported version.
benquarmby
left a comment
There was a problem hiding this comment.
@gnapse, @kentcdodds : Sorry to be the bringer of breaking changes 🙈
Vitest 5 puts us in a difficult spot in terms of TypeScript declarations.
In either case, I hope these comments help.
| any, | ||
| T | ||
| > {} | ||
| interface Assertion<R, T> extends TestingLibraryMatchers<T, R> {} |
There was a problem hiding this comment.
- The recommended approach is to augment only
Matchers<R, T>. However, it doesn't support function overloads liketoHaveClass. We have to continue to augment bothAssertionandAsymmetricMatchersContaining:Lines 281 to 282 in 3782c78
- There was an existing defect here where
T(the type of the expected argument) was being passed as the return type ofTestingLibraryMatchers. It obviously didn't do a lot of harm, but this PR corrects the order.
| any, | ||
| any | ||
| > {} | ||
| extends TestingLibraryMatchers<any, any> {} |
There was a problem hiding this comment.
This is just prettier being prettier.
| "peerDependencies": { | ||
| "@testing-library/dom": ">=10 <11", | ||
| "vitest": ">= 0.32" | ||
| "vitest": ">=5 <6" |
There was a problem hiding this comment.
Vitest 5 is a demonstration of why open ended peer deps don't work. This tightens the range to what we know we're compatible with today. Reasoning in #731 (comment) #correct-over-convenient
|
Sorry, I'm no longer using this so I'm no longer maintaining. |
…ot yet #105 raised vitest to 5.0.0 with its frontend check red, and nothing on main noticed because the backend job ahead of it was already failing on another bump. Vitest 5 made `Assertion<T>` into `Assertion<R, T>`; jest-dom 7.0.1 augments the one-parameter shape, the merge silently fails under `skipLibCheck`, and a clean `tsc --build` reports 208 TS2339 errors across every test file while the suite runs green - the runtime half still works. `frontend/types/jest-dom-vitest.d.ts` augments vitest's documented `Matchers<R, T>` extension point with jest-dom's matchers, in the parameter order jest-dom's own pending fix uses (testing-library/jest-dom#742). The four project tsconfigs include it. Delete both when jest-dom ships #742. Verified from a lockfile install: `tsc --build --force` 208 errors without the file, 0 with it; `lint`, the whole vitest run (33 files, 317 passed) and `build` each exit=0. HANDOFF records the trap: `npm run typecheck` passed on this tree because stale `*.tsbuildinfo` skipped the check. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Raise starlette to 1.7.0, because anyio 4.15 moved the portal and the warning gate refused the old import `main` has failed its backend job on every push since #106 raised anyio to 4.15.1: `anyio.abc.BlockingPortal` is deprecated there, starlette 1.6.0 still imports it in `testclient.py`, and `filterwarnings = ["error"]` turns that import into a collection error in `tests/api/test_events_ws.py` before a single test runs. Both open Dependabot pull requests fail on the same line. starlette 1.7.0 imports the portal from `anyio.from_thread`. The comment in `constraints.txt` records why an ignore was refused: it would pass the gate while leaving the farm on a version the next anyio actually breaks. Verified: `tests/api/test_events_ws.py` 14 passed; the full suite 1 619 passed, 7 skipped, exit=0 in 1687.95s; all six gates exit=0. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * Say on the three worker intervals that a change waits for a restart (#32) `scheduler_tick_seconds`, `sla_sweep_seconds` and `telemetry_poll_seconds` are handed to their loops once in `workers/runner.py`; their neighbours on the settings screen resolve at the read edge and take effect on save. The screen gave no sign of the difference, so an owner saw the audit row and concluded the change had taken. The hint now states it, in both catalogues, and the new `SettingsPage` test pins that each of the three carries it in its own section. Reverting the catalogue change fails that test (run). The hot reload itself stays deferred, as the issue says. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * Stop publishing a floor account and a password that nothing seeds Closes #12. `docs/DEVELOPMENT.md` listed `floor@printorian.example` / `shop-floor-pass-1` and a fixed owner password; nothing in the repository creates the first or sets the second. Verified against a freshly created database: `alembic upgrade head` (0001 -> 0026), `provision_with` with the password `owner-pass-12345`, then `POST /auth/sign-in` through Starlette's TestClient - `boss@` 200, `floor@` 401 `error.identity.invalid_credentials`, and `SELECT email, role FROM users` returns the one owner row. The table now says the owner's password is whatever was typed at the prompt and the second account is made in the console with the role the gate being checked needs. The role decision the issue flagged as needs-person is not made: the document no longer asserts one. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * Score a supplier from the orders that reached the shelf, and say what cannot be scored (#34) The last clause of #34's "done when": the supplier scorecard computes from delivered purchase orders rather than being typed in. `supplier_scores` groups orders by supplier with three FILTER counts on one join - reached STORED, of those the ones that carried an expected_at, of those the ones on the shelf by that date. `on_time_share` is on_time / dated, and null when nothing was dated: an undated delivery is neither punctual nor late, and counting it either way moves the score for every date a buyer did not type. Three of the kit's six columns are deliberately absent, each for a reason the schema docstring records: a defect share needs a rejected quantity no receipt records and would read 0% for everyone; turnover is money the board never carries; a composite rating is a policy nobody has written. `test_purchasing_api.py` crossed the 400-line gate with the API test for this and is split at its seam: `_purchasing_support.py` (helpers, no fixtures - the `_catalog_support.py` rule) and `test_purchasing_scorecard_api.py`. Verified: nine unit tests and two API tests; four mutations each fail at least one - RECEIVING counted as delivered (1), deliveries as the denominator (2), 0 for an undated share (3), every dated delivery on time (1). The screen test draws the share with its counts and an em dash where nothing was dated. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * Record the session: why main was red, what was verified, what is left Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * Type jest-dom's matchers on vitest 5's expect, because jest-dom has not yet #105 raised vitest to 5.0.0 with its frontend check red, and nothing on main noticed because the backend job ahead of it was already failing on another bump. Vitest 5 made `Assertion<T>` into `Assertion<R, T>`; jest-dom 7.0.1 augments the one-parameter shape, the merge silently fails under `skipLibCheck`, and a clean `tsc --build` reports 208 TS2339 errors across every test file while the suite runs green - the runtime half still works. `frontend/types/jest-dom-vitest.d.ts` augments vitest's documented `Matchers<R, T>` extension point with jest-dom's matchers, in the parameter order jest-dom's own pending fix uses (testing-library/jest-dom#742). The four project tsconfigs include it. Delete both when jest-dom ships #742. Verified from a lockfile install: `tsc --build --force` 208 errors without the file, 0 with it; `lint`, the whole vitest run (33 files, 317 passed) and `build` each exit=0. HANDOFF records the trap: `npm run typecheck` passed on this tree because stale `*.tsbuildinfo` skipped the check. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
<!-- Template from https://github.com/kubevirt/kubevirt/blob/main/.github/PULL_REQUEST_TEMPLATE.md?--> <!-- Thanks for sending a pull request! Here are some tips for you: 1. Consider creating this PR as draft: https://github.com/CherryHQ/cherry-studio/blob/main/CONTRIBUTING.md --> > ### Branch strategy > > - Active development targets `main`. ### What this PR does Before this PR: The test toolchain is on Vitest 4 (`^4.1.10`). After this PR: - `vitest` and `@vitest/*` are on `^5.0.1` in the root and all 7 workspace packages. - The root pins `@opentelemetry/api` to `1.9.1`, so `packages/ui` resolves the same `vitest` copy as the root. - Both benchmarks use the new `bench` test-context fixture, and the `bench:*` scripts target the new `<name> (bench)` projects. - Two tests that broke under Vitest 5 are fixed (`sentryLog.test.ts`, `AppShellTabBar.test.tsx`). - `CLAUDE.md` now says Vitest 5. Fixes #21024 ### Why we need it and why it was done in this way Vitest 5.0 shipped on 2026-09-03. Every suite and `pnpm typecheck` were run on the upgraded tree, and each breakage was fixed directly: - **`clearMocks` now defaults to `true`.** We adopt the new default. The only test that relied on mock-call history across tests was `sentryLog.test.ts`, which looked up a handler that `LoggerService` registers at import time. It now captures the handler once at module scope. - **Non-writable global stubs.** `AppShellTabBar.test.tsx` replaced `requestAnimationFrame` / `cancelAnimationFrame` with non-writable properties. Under v5, `vi.useRealTimers()` throws on them, and the leftover fake timers broke 3 later tests. The descriptors are now writable. - **Benchmark API rewrite.** `bench` is no longer exported from `vitest`. Benchmarks are now `test('…', async ({ bench }) => bench.compare(...))`. The tinybench `setup` / `teardown` options become per-bench `beforeAll` / `afterAll`, and `iterations` / `warmupIterations` move to the `bench.compare` run options. Benchmarks also run in a separate `<name> (bench)` project, so the old `--project renderer` filter fails with `No projects matched the filter`. - **jest-dom matcher types lost in `packages/ui`** (543 `tsc` errors across 42 files). Vitest declares `@opentelemetry/api` as an optional peer. The lockfile had resolved it to 1.9.0 for several workspace packages (a stale resolution already present with v4), which installs a separate `vitest` copy for `packages/ui`. `@testing-library/jest-dom/vitest` augments only the root copy. Pinning the root dependency to `1.9.1` re-resolves the peer, and its lockfile diff is limited to the `vitest` / `@vitest/mocker` peer variants. The following tradeoffs were made: - `@opentelemetry/api` is pinned exactly, so future bumps are manual. The root already pins `vite`, `jsdom` and `typescript` exactly. - jest-dom's `Assertion<T>` augmentation still conflicts with Vitest 5's `Assertion<R, T>` (testing-library/jest-dom#738). With our TypeScript 7.0.2 and `skipLibCheck: true`, that is only a suppressed TS2428 inside `node_modules`, and the matchers still type-check. We can upgrade jest-dom once testing-library/jest-dom#742 is released. The following alternatives were considered: - `pnpm update -r @opentelemetry/api` or `pnpm update -r vitest`: both leave the stale peer resolution in place. - `pnpm dedupe`: rewrites many unrelated packages. - A pnpm `overrides` entry for `@opentelemetry/api`: also unifies the copies, but any `overrides` edit re-resolves the lockfile and moves root `react` to 19.2.7 while `react-dom` stays at 19.2.3. `react-dom` then throws `Incompatible React versions` on load. - A local jest-dom type shim, or replacing jest-dom: unnecessary once the `vitest` copies are unified. - Pinning `clearMocks: false`: rejected. About 700 test files already call `vi.clearAllMocks()` by hand. Links to places where the discussion took place: #21024 ### Breaking changes None. Only dev dependencies, tests, benchmarks and a docs line change. `@opentelemetry/api` already resolved to 1.9.1 at runtime. ### Special notes for your reviewer - The commits are split: two test fixes, both also valid on Vitest 4, followed by the upgrade itself. - Verified locally on Node 24.21.0: every Vitest project, the standalone package suites (`dsh-bridge`, `remote-protocol`, `remote-transport`, `ai-sdk-provider`), `pnpm lint`, `pnpm test:lint`, `pnpm bench:renderer`, and `pnpm test:coverage` (the report covers 5088 source files, with no test files, type declarations, `dist` output or `node_modules` among them). - One `probes.e2e.test.ts` case fails locally whenever the DNS resolver answers `.invalid` names (for example a fake-IP proxy). It is unrelated to this change. - Inline projects in `vitest.config.ts` now inherit the root config by default, so the explicit `extends: true` flags are redundant. They were left in place to keep this diff focused. ### Checklist This checklist is not enforcing, but it's a reminder of items that could be relevant to every PR. Approvers are expected to review this list. - [x] Branch: This PR targets `main` - [x] PR: The PR description is expressive enough and will help future contributors - [x] Code: [Write code that humans can understand](https://en.wikiquote.org/wiki/Martin_Fowler#code-for-humans) and [Keep it simple](https://en.wikipedia.org/wiki/KISS_principle) - [ ] Refactor: You have [left the code cleaner than you found it (Boy Scout Rule)](https://learning.oreilly.com/library/view/97-things-every/9780596809515/ch08.html) - [x] Upgrade: Impact of this change on upgrade flows was considered and addressed if required - [ ] Documentation: A [user-guide update](https://docs.cherry-ai.com) was considered and is present (link) or not required. Check this only when the PR introduces or changes a user-facing feature or behavior. - [x] Self-review: I have reviewed my own code (e.g., via [`/gh-pr-review`](/.claude/skills/gh-pr-review/SKILL.md), `gh pr diff`, or GitHub UI) before requesting review from others ### Release note <!-- Write your release note: 1. Enter your extended release note in the below block. If the PR requires additional action from users switching to the new release, include the string "action required". 2. If no release note is required, just write "NONE". 3. Only include user-facing changes (new features, bug fixes visible to users, UI changes, behavior changes). For CI, maintenance, internal refactoring, build tooling, or other non-user-facing work, write "NONE". --> ```release-note NONE ``` --------- Signed-off-by: eurfelux <eurfelux@gmail.com> Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
In my experience upgrading type augmentation for @accesslint/vitest, the new types still work correctly for consumers on Vitest v4 or below as long as they have |
cleitonpg
left a comment
There was a problem hiding this comment.
Consumer-side verification from a downstream org currently carrying a manual type bridge for this exact issue — measured against vitest 5.0.1 + jest-dom 7.0.1 + TypeScript 5.6.3, strict, moduleResolution: bundler:
1. The shipped 7.0.1 types reproduce the reported failure exactly as described in #738:
node_modules/@testing-library/jest-dom/types/vitest.d.ts(5,13): error TS2428:
All declarations of 'Assertion' must have identical type parameters.
(same TS2428 against vitest/dist/chunks/config.d.*.d.ts's Assertion<R, T>).
2. This PR's types/vitest.d.ts compiles clean against vitest 5.0.1: zero errors, and the augmentation is substantive — matcher availability (toBeInTheDocument, toHaveTextContent with the RegExp overload, toHaveAccessibleName), asymmetric matchers (expect.toBeInTheDocument()), and the sync return-typing (const r: void = expect(el).toBeInTheDocument()) all resolve.
3. Negative control — no silent any: expect(el).toHaveTextContent(123) correctly fails with TS2345: Argument of type 'number' is not assignable to parameter of type 'string | RegExp', so argument checking stays live.
The TestingLibraryMatchers<T, R> generic-order flip plus the arity fix is exactly what's needed, and dropping support for vitest <5 in the same PR is the right call given the declarations are mutually exclusive (as you noted).
One residual risk worth stating for the record: the augmentation re-declares Assertion<R, T> without re-stating vitest 5's constraint (R extends void | Promise<void> = void). TypeScript 5.6 accepts the merge (constraints aren't re-checked at merge site), which keeps this resilient across vitest minor bumps — but it does lean on the compiler's current merge leniency. If TS ever tightens identical-constraint enforcement for merged declarations, this shape regresses. Copying the constraint verbatim would trade that for brittleness against future vitest definition changes. I checked the merge behavior explicitly on 5.6; treating it as a documented assumption of the PR would be reasonable.
We're blocked on this from deleting our own workaround, so thank you for pushing it — and to the maintainers: anything a downstream consumer can do to help this land (review bandwidth, additional matrix entries, docs for the 8.0.0 release notes)?
Vitest 5 changed Assertion<T> to Assertion<R, T> and dropped the jest.Matchers linkage the plain @testing-library/jest-dom import relied on under Vitest 4, so the web typecheck failed with ~1800 missing-matcher errors. Patch jest-dom types/vitest.d.ts with upstream testing-library/jest-dom#742 (pnpm.patchedDependencies) and switch setupTests to the /vitest entry so the arity-2 augmentation merges. Move the removed deps.inline option to server.deps.inline for Vitest 5. Bump vitest, @vitest/coverage-v8 and @vitest/ui together to 5.0.3 (coverage-v8/ui pin vitest exactly). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HxWLuSSgYgidvY59ecNYmS
What:
This PR adds support for Vitest 5 and removes support for all versions below that. It resolves #738 .
Important
This is a breaking change for vitest <5 consumers. The commit message contains this footer:
BREAKING CHANGE: Support for vitest versions lower than 5 has been removed. Consumers must upgrade to a supported version.Why:
It is not possible for vitest + typescript + testing-library consumers to safely uptake vitest 5 due to declaration mismatches. Full breakdown in #738 . Sadly the new declarations are mutually exclusive with the old.
How:
TL;DR:
AssertionaugmentationI will add some inline comments to explain.
Checklist:
Comments:
I considered adding a
vitest-v4barrel, but testing it got out of control. I also discovered that vitest 4 works from the jest augmentations in the root barrel if an escape hatch is needed. See #738 (comment).