Make the ResizeObserver test mock constructible under any vitest - #18
Merged
Merged
Conversation
Recharts calls `new ResizeObserver(...)`, and the mock was built with `vi.fn().mockImplementation(...)`. Whether a vitest mock function can be used as a constructor is an implementation detail, and it changed in vitest 5: every Recharts-backed test fails there with "... is not a constructor", 10 of the 144 in this suite. A plain class does the same job without depending on that detail. Verified both ways: 144/144 on the pinned vitest 3.2.6, and 144/144 on vitest 5.0.1, which is what Dependabot #15 proposes. That PR cannot go green without this. Also stop the generated coverage/ directory leaking into lint and git. It was in neither web/.gitignore nor the eslint ignorePatterns, so running `vitest run --coverage` produced instrumented copies of the source that eslint then reported errors on, and left them untracked in the working tree. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
No unresolved review issues remain.
Review effort: Lite
Findings: None
What changed in this PR
Updates the ResizeObserver test mock for Vitest compatibility and excludes generated coverage artifacts.
Changes:
- Replaces the function mock with a constructible class.
- Ignores coverage output in Git and ESLint.
| File | Description |
|---|---|
web/src/test/setup.js |
Provides a constructible ResizeObserver mock. |
web/.gitignore |
Ignores generated coverage output. |
web/.eslintrc.cjs |
Excludes coverage output from linting. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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.
Why
Dependabot #15 bumps vitest from 3.2.6 to 5.0.1, two major versions, to close two moderate advisories (
vitestand@vitest/mocker, path traversal / arbitrary file read). On that version 10 of the 144 tests fail:Recharts calls
new ResizeObserver(...), and our mock was built withvi.fn().mockImplementation(...). Whether a vitest mock function can be used as a constructor is an implementation detail, and it changed in v5. Every Recharts-backed test (the Dashboard page and integration specs) fails.A plain class does the same job without depending on that detail.
Verified
Lint clean and
vitest run --coveragepasses on both. This change is version-independent, so it is safe to land ahead of the bump.Also
web/coverage/was in neitherweb/.gitignorenor the eslintignorePatterns, so runningvitest run --coveragewrote instrumented copies of the source that eslint then reported errors on, and left them untracked. Both now ignore it.🤖 Generated with Claude Code