Skip to content

fix(web): resolve jest-dom matcher types under pnpm via packageExtensions - #3494

Merged
fanzeyi merged 1 commit into
mainfrom
fix/web-jest-dom-vitest-types
Jul 29, 2026
Merged

fix(web): resolve jest-dom matcher types under pnpm via packageExtensions#3494
fanzeyi merged 1 commit into
mainfrom
fix/web-jest-dom-vitest-types

Conversation

@fanzeyi

@fanzeyi fanzeyi commented Jul 29, 2026

Copy link
Copy Markdown
Member

Related issue

N/A

Summary

  • @testing-library/jest-dom never declares vitest as a (peer) dependency.
    Under pnpm's store layout, jest-dom's declare module "vitest" matcher-type
    augmentation can't resolve vitest, so it silently fails to merge and tsc
    loses every DOM matcher (toBeInTheDocument, toHaveClass, …) — even though
    they register fine at runtime. See vitest 4.1.6+ silently breaks @testing-library/jest-dom type augmentation (toBeInTheDocument missing on Assertion) vitest-dev/vitest#10411.
  • Declare the missing peer via pnpm packageExtensions so pnpm links vitest
    into jest-dom's scope and the augmentation resolves. This is a root-cause fix
    at the dependency layer — no hand-written type shim needed.
  • Note: type-check still has unrelated pre-existing errors and is not yet
    gated in CI; this fix only removes the jest-dom matcher category.

Test Plan

  • pnpm install --frozen-lockfile --filter web — lockfile stays consistent.
  • pnpm --filter web run type-check — jest-dom matcher errors drop from 1589
    to 0 (remaining errors are unrelated and pre-existing).
  • pnpm --filter web run test (e.g. src/shell/WorkspacePanel.test.tsx) —
    15/15 pass under Node 22; runtime is unaffected.

Demo

N/A

Type of change

  • Bug fix
  • Feature
  • UI / frontend change
  • Refactor / chore
  • Docs
  • Test / CI
  • Breaking change

Test coverage

  • Unit tests added / updated
  • Integration tests added / updated
  • E2E tests added / updated
  • Manual verification completed
  • Existing tests cover this change
  • Not applicable

Coverage notes

Verified by comparing pnpm --filter web run type-check jest-dom error counts
(1589 → 0) and running the existing vitest suite (unaffected). The change is
dependency-resolution config only, with no new runtime code to test.

…ions

## Related issue

N/A

## Summary

- `@testing-library/jest-dom` never declares `vitest` as a (peer) dependency.
  Under pnpm's store layout, jest-dom's `declare module "vitest"` matcher-type
  augmentation can't resolve `vitest`, so it silently fails to merge and `tsc`
  loses every DOM matcher (`toBeInTheDocument`, `toHaveClass`, …) — even though
  they register fine at runtime. See vitest-dev/vitest#10411.
- Declare the missing peer via pnpm `packageExtensions` so pnpm links `vitest`
  into jest-dom's scope and the augmentation resolves. This is a root-cause fix
  at the dependency layer — no hand-written type shim needed.
- Note: `type-check` still has unrelated pre-existing errors and is not yet
  gated in CI; this fix only removes the jest-dom matcher category.

## Test Plan

- `pnpm install --frozen-lockfile --filter web` — lockfile stays consistent.
- `pnpm --filter web run type-check` — jest-dom matcher errors drop from 1589
  to 0 (remaining errors are unrelated and pre-existing).
- `pnpm --filter web run test` (e.g. `src/shell/WorkspacePanel.test.tsx`) —
  15/15 pass under Node 22; runtime is unaffected.

## Demo

N/A

## Type of change

- [ ] Bug fix
- [ ] Feature
- [ ] UI / frontend change
- [x] Refactor / chore
- [ ] Docs
- [ ] Test / CI
- [ ] Breaking change

## Test coverage

- [ ] Unit tests added / updated
- [ ] Integration tests added / updated
- [ ] E2E tests added / updated
- [x] Manual verification completed
- [x] Existing tests cover this change
- [ ] Not applicable

## Coverage notes

Verified by comparing `pnpm --filter web run type-check` jest-dom error counts
(1589 → 0) and running the existing vitest suite (unaffected). The change is
dependency-resolution config only, with no new runtime code to test.

Signed-off-by: Zeyi (Rice) Fan <zeyi.f@databricks.com>
@github-actions github-actions Bot added the size/S Pull request size: S label Jul 29, 2026
@fanzeyi fanzeyi changed the title fix/web jest dom vitest types fix(web): resolve jest-dom matcher types under pnpm via packageExtensions Jul 29, 2026
@omnigent-ci

omnigent-ci Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Polly AI Review

Review: fix(web): resolve jest-dom matcher types under pnpm via packageExtensions

1. Blocking issues

None. The change is dependency-resolution config only. The lockfile is internally consistent with the pnpm-workspace.yaml edit:

  • The packageExtensionsChecksum was regenerated (expected when packageExtensions changes).
  • The @testing-library/jest-dom@6.9.1 package gains peerDependencies: vitest: '*', matching the declared extension.
  • The snapshot resolves to 6.9.1(vitest@4.1.10) with vitest linked into its dependency set.

vitest@4.1.10 satisfies the existing ^4.1.5 constraint in web/package.json, so no version conflict is introduced. No new packages, no version changes to existing pins, no downgrades.

2. Security vulnerabilities

None. No packages added or upgraded — vitest is already a direct devDependency of web and is merely being linked into jest-dom's peer scope. No new extras, no constraint violations, no security-sensitive downgrades. This is config-only and does not weaken any boundary.

3. Non-blocking notes

  • Runtime coupling of a peer that's really a dev-time-only concern. Declaring vitest: '*' as a peer of jest-dom via packageExtensions is fine for this repo (jest-dom is only ever used in the test/type-check context), but the * range is intentionally permissive — a future major vitest bump won't be gated by this peer. That's acceptable since the real version constraint lives in web/package.json, but worth remembering the extension itself imposes no bound.
  • Cleanup marker is good. The comment already says "Drop once jest-dom ships the peer upstream," which is the right deprecation-style breadcrumb. Consider linking the upstream issue (vitest 4.1.6+ silently breaks @testing-library/jest-dom type augmentation (toBeInTheDocument missing on Assertion) vitest-dev/vitest#10411) in the comment so a future reader can check status without hunting the PR description.
  • type-check not gated in CI (per the PR description) means this fix can silently regress. Out of scope for this PR, but the matcher-type category dropping to 0 is only durable if type-check eventually becomes a required gate.

4. Summary

A well-scoped, root-cause fix at the dependency layer rather than a hand-written type shim. The packageExtensions declaration and the regenerated lockfile entries are mutually consistent, introduce no new packages or constraint violations, and carry no security risk. The accompanying comment explains the why and includes a removal condition. Approve; the only follow-ups (link the upstream issue, eventually gate type-check in CI) are non-blocking.


Automated review by Polly · workflow run

@fanzeyi
fanzeyi merged commit c38e174 into main Jul 29, 2026
67 of 69 checks passed
@fanzeyi
fanzeyi deleted the fix/web-jest-dom-vitest-types branch July 29, 2026 08:24
@github-actions github-actions Bot added the no-doc-update Merged PR does not need a docs update label Jul 29, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🏷️ Doc impact: no-doc-update

This only adjusts pnpm lockfile/workspace config to re-add a vitest peer dependency for jest-dom's type augmentation — a build/tooling tweak with no user-facing surface or integration change.

Auto-classified on merge. Set the label manually before merging to override. · run

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-doc-update Merged PR does not need a docs update size/S Pull request size: S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant