chore(tooling): editorconfig, CODEOWNERS, jsdom component tests, knip#636
Conversation
…knip Adds four developer-productivity tools that do not overlap with tooling main already ships. (git-hooks, a bundle budget, axe a11y scanning, and Sentry all landed on main independently while this work was in progress, so those are dropped in favour of main's versions.) - .editorconfig — editor consistency aligned to .prettierrc / .gitattributes. - .github/CODEOWNERS — review routing on the high-risk paths flagged in AGENTS.md. - jsdom + @testing-library/react — interactive component tier as a SEPARATE vitest project (tests/**/*.dom.test.tsx); the existing node suite is untouched. - knip — advisory dead-code / unused-dependency detection (npm run check:knip; not wired into any gate). Verified against current main: 235 test files / 2209 tests pass (0 failures), typecheck / lint / format:check clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
📝 WalkthroughWalkthroughThe PR adds jsdom-based Vitest projects, React Testing Library setup, DOM tests for ChangesTesting and Repository Tooling
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/setup/jsdom.setup.ts`:
- Around line 29-35: Update the scrollIntoView setup in beforeEach so its mock
is recreated or reset for every test, rather than only initialized when absent.
Ensure vi.restoreAllMocks() or an equivalent per-test cleanup clears call counts
and arguments while preserving the jsdom fallback behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 2bf5ba3a-8634-4eb9-867d-e2ef594db76f
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
.editorconfig.github/CODEOWNERSknip.jsonpackage.jsontests/accessible-table.dom.test.tsxtests/setup/jsdom.setup.tstests/use-event-callback.dom.test.tsxvitest.config.mts
| beforeEach(() => { | ||
| installMatchMediaStub(false); | ||
| // jsdom does not implement scrollIntoView; components call it on focus/expand. | ||
| if (!Element.prototype.scrollIntoView) { | ||
| Element.prototype.scrollIntoView = vi.fn(); | ||
| } | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
scrollIntoView mock never resets between tests.
The if (!Element.prototype.scrollIntoView) guard means the mock is created once per test file and never cleared afterward; vi.restoreAllMocks() in afterEach doesn't touch it since it wasn't created via vi.spyOn. Any future assertion on scrollIntoView call count/args will see accumulated state from earlier tests in the same file.
As per coding guidelines, "reliability concerns that degrade user experience or resilience" and reliability findings should be prioritized in this glob.
🔧 Proposed fix
beforeEach(() => {
installMatchMediaStub(false);
// jsdom does not implement scrollIntoView; components call it on focus/expand.
- if (!Element.prototype.scrollIntoView) {
- Element.prototype.scrollIntoView = vi.fn();
- }
+ Element.prototype.scrollIntoView = vi.fn();
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| beforeEach(() => { | |
| installMatchMediaStub(false); | |
| // jsdom does not implement scrollIntoView; components call it on focus/expand. | |
| if (!Element.prototype.scrollIntoView) { | |
| Element.prototype.scrollIntoView = vi.fn(); | |
| } | |
| }); | |
| beforeEach(() => { | |
| installMatchMediaStub(false); | |
| // jsdom does not implement scrollIntoView; components call it on focus/expand. | |
| Element.prototype.scrollIntoView = vi.fn(); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/setup/jsdom.setup.ts` around lines 29 - 35, Update the scrollIntoView
setup in beforeEach so its mock is recreated or reset for every test, rather
than only initialized when absent. Ensure vi.restoreAllMocks() or an equivalent
per-test cleanup clears call counts and arguments while preserving the jsdom
fallback behavior.
Source: Coding guidelines
1 participant
Summary
Adds four developer-productivity tools that do not overlap with tooling
mainalready ships. While this work was in progress,
mainindependently landed agit-hooks system (
install-git-hooks.mjs), a bundle budget (check-bundle-budget.mjs),axe a11y scanning, and server-side Sentry (
@sentry/node+error-capture.ts) — sothose four are intentionally dropped in favour of main's versions, leaving only
the non-conflicting additions here:
.editorconfig— editor consistency aligned to.prettierrcand.gitattributes(lf, utf-8, 2-space, final newline; markdown + generateddatabase.types.tsexempt from trailing-whitespace trimming)..github/CODEOWNERS— review routing on the high-risk paths flagged in AGENTS.md (supabase, env, RAG/retrieval/search, source governance, workflows). Solo-owner today; sections are ready for collaborators.tests/**/*.dom.test.tsx, jsdom env,tests/setup/jsdom.setup.ts). The existing node project (tests/**/*.test.ts) is byte-for-byte unchanged. Seeds two example specs (anAccessibleTableexpand/dialog interaction and auseEventCallbackrenderHooktest).npm run check:knip,knip.json). Not wired into any gate; false positives (tailwindcss, server-only, taskkill, Deno edge fns) suppressed.Verification
Run against current
main:test— 235 test files / 2209 tests pass, 0 failures (node project unchanged + new jsdom project 4/4)typecheck,lint,format:check— clean (exit 0)check:knip— runs (advisory)No source-logic, build, bundle, retrieval, answer-generation, privacy, or clinical behavior changed — these are config + test-tooling additions only.
Clinical Governance Preflight
N/A — does not touch ingestion, answer generation, search/ranking, source rendering, document access, privacy, production env, or clinical output.
Notes
npm run test, so it's covered by the existing requiredstatic-prgate with no CI changes.knipis deliberately advisory: a fresh run on a mature repo surfaces a large triage list; gating on it would be a regression. Baseline (~164 unused exports / 66 types) is future cleanup.🤖 Generated with Claude Code
Summary by CodeRabbit
Tests
Chores