Skip to content

chore: enable additional eslint-react rules and fix violations#1028

Merged
Aureliolo merged 2 commits intomainfrom
chore/enable-eslint-react-rules
Apr 3, 2026
Merged

chore: enable additional eslint-react rules and fix violations#1028
Aureliolo merged 2 commits intomainfrom
chore/enable-eslint-react-rules

Conversation

@Aureliolo
Copy link
Copy Markdown
Owner

@Aureliolo Aureliolo commented Apr 3, 2026

Summary

Enable 7 eslint-react rules not included in recommended-typescript and fix all violations. Also fix a pre-existing flaky test.

New ESLint rules enabled

Rule Severity Purpose
jsx-no-leaked-dollar error Prevent dollar signs leaking into rendered JSX output
jsx-no-useless-fragment warn Remove unnecessary <></> fragment wrappers
dom-no-missing-button-type warn Require type attribute on <button> elements
dom-no-unsafe-target-blank error Require rel="noopener" with target="_blank" (security)
no-duplicate-key error Catch duplicate keys in JSX lists
no-unstable-context-value warn Catch unstable context values causing unnecessary re-renders
no-unstable-default-props warn Catch unstable default props causing unnecessary re-renders

Note: jsx-no-leaked-semicolon (also new in 4.2.3) is already included in recommended-typescript automatically.

Violations fixed

  • 10 dom-no-missing-button-type violations: added type="button" to buttons in Sidebar, StatusBar, guards, stories, and tests

Flaky test fix

  • client.test.ts > clears auth localStorage keys on 401 was failing because VITE_DEV_AUTH_BYPASS=true in web/.env caused the 401 interceptor to skip localStorage clearing in tests. Fixed by mocking @/utils/dev to disable the bypass.

Test plan

  • npm --prefix web run lint -- zero warnings
  • npm --prefix web run type-check -- clean
  • npm --prefix web run test -- 2341/2341 passing (including the previously flaky test)

Context

Follow-up to dependency PRs #1020-#1027 (all merged). The eslint-react upgrade from 4.2.1 to 4.2.3 added new rules worth enabling.

@Aureliolo
chore: enable additional eslint-react rules and fix violations
5b2f269 
Enable 7 eslint-react rules not in recommended-typescript:
- jsx-no-leaked-dollar (error): prevent dollar signs leaking into JSX
- jsx-no-useless-fragment (warn): remove unnecessary <></> wrappers
- dom-no-missing-button-type (warn): require type on <button> elements
- dom-no-unsafe-target-blank (error): require rel=noopener (security)
- no-duplicate-key (error): catch duplicate keys in JSX lists
- no-unstable-context-value (warn): catch perf-degrading context values
- no-unstable-default-props (warn): catch perf-degrading default props

Fix all 10 dom-no-missing-button-type violations by adding type=button.

Fix pre-existing flaky test in client.test.ts: mock IS_DEV_AUTH_BYPASS
to false so 401 interceptor actually exercises localStorage clearing
(VITE_DEV_AUTH_BYPASS=true in web/.env was causing the test to skip).
Copilot AI review requested due to automatic review settings April 3, 2026 07:40
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 3, 2026 •
edited
Loading

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: f7e6ed98-7e74-4c08-8de6-8c7fcf64e8c1

📥 Commits

Reviewing files that changed from the base of the PR and between 5b2f269 and cfabb89.

📒 Files selected for processing (1)
  • web/src/__tests__/api/client-bypass.test.ts
📜 Recent review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
  • GitHub Check: Build Sandbox
  • GitHub Check: Build Backend
  • GitHub Check: Build Web
  • GitHub Check: Dashboard Test
  • GitHub Check: Dependency Review
  • GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (2)
web/src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (web/CLAUDE.md)

web/src/**/*.{ts,tsx}: Use Tailwind semantic classes (text-foreground, bg-card, text-accent, text-success, bg-danger, etc.) or CSS variables (var(--so-*)) for colors in React and TypeScript files
NEVER hardcode hex color values or rgba() with hardcoded values in .tsx/.ts files -- use design token variables instead
Do NOT recreate status dots inline -- use <StatusBadge> component instead
Do NOT build card-with-header layouts from scratch -- use <SectionCard> component instead
Do NOT create metric displays with text-metric font-bold inline -- use <MetricCard> component instead
Do NOT render initials circles manually -- use <Avatar> component instead
Do NOT create complex (>8 line) JSX inside .map() -- extract to a shared component instead
Do NOT use rgba() with hardcoded values -- use design token variables instead
Do NOT hardcode Framer Motion transition durations -- use presets from @/lib/motion instead

React dashboard design system: always reuse existing components from web/src/components/ui/ before creating new ones. Never hardcode hex colors, font-family, pixel spacing, or Framer Motion transitions -- use design tokens and @/lib/motion presets.

Files:

  • web/src/__tests__/api/client-bypass.test.ts
web/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

TypeScript 6.0+ required for web dashboard; property-based testing in React uses fast-check (fc.assert + fc.property)

Files:

  • web/src/__tests__/api/client-bypass.test.ts
🧠 Learnings (2)
📓 Common learnings 
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to web/src/**/*.{ts,tsx} : Use React 19, TypeScript 6.0+, and design system tokens from shadcn/ui + Tailwind CSS 4 + Radix UI in web dashboard

Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-02T12:19:36.876Z
Learning: Applies to web/.storybook/preview.tsx : In Storybook 10 stories, use `parameters.a11y.test: 'error' | 'todo' | 'off'` for a11y testing (replaces old `.element` and `.manual`). Set globally in `preview.tsx` to enforce WCAG compliance on all stories

Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-02T12:19:36.876Z
Learning: Applies to web/**/*.stories.tsx : Import from `storybook/test` (not `storybook/test`), `storybook/actions` (not `storybook/addon-actions`), and use `defineMain`/`definePreview` type-safe config in Storybook 10

Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-03T07:34:51.012Z
Learning: Applies to web/**/*.{ts,tsx} : TypeScript 6.0+ required for web dashboard; property-based testing in React uses fast-check (`fc.assert` + `fc.property`)

Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-02T12:19:36.876Z
Learning: Applies to web/src/components/ui/**/*.stories.tsx : Create a `.stories.tsx` file alongside every new shared component in `web/src/components/ui/` with all states (default, hover, loading, error, empty)

Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-03T07:34:51.012Z
Learning: Applies to web/src/**/*.{ts,tsx} : React dashboard design system: always reuse existing components from `web/src/components/ui/` before creating new ones. Never hardcode hex colors, font-family, pixel spacing, or Framer Motion transitions -- use design tokens and `@/lib/motion` presets.

Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-02T12:19:36.876Z
Learning: Applies to web/src/components/**/*.{ts,tsx} : ALWAYS reuse existing components from `web/src/components/ui/` before creating new ones (StatusBadge, MetricCard, Sparkline, SectionCard, AgentCard, DeptHealthBar, ProgressGauge, StatPill, Avatar, Button, Toast, Skeleton variants, EmptyState, ErrorBoundary, ConfirmDialog, CommandPalette, InlineEdit, AnimatedPresence, StaggerGroup/Item, Drawer, InputField, SelectField, SliderField, ToggleField, TaskStatusIndicator, PriorityBadge, ProviderHealthBadge, TokenUsageBar, CodeMirrorEditor, SegmentedControl, ThemeToggle, LiveRegion, MobileUnsupportedOverlay, LazyCodeMirrorEditor, TagInput, MetadataGrid, ProjectStatusBadge, ContentTypeBadge)

Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-02T12:19:36.876Z
Learning: Applies to web/src/**/*.{ts,tsx} : Do NOT recreate status dots inline -- use `<StatusBadge>` component instead

Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-02T12:19:36.876Z
Learning: Applies to web/src/components/ui/*.{ts,tsx} : Use design tokens exclusively in new components -- no hardcoded colors, fonts, or spacing
📚 Learning: 2026-03-20T08:28:32.845Z 
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T08:28:32.845Z
Learning: Applies to web/src/__tests__/**/*.{ts,js} : Dashboard testing: Vitest unit tests organized by feature under `web/src/__tests__/`. Use fast-check for property-based testing (`fc.assert` + `fc.property`).

Applied to files:

  • web/src/__tests__/api/client-bypass.test.ts
🔇 Additional comments (1)
web/src/__tests__/api/client-bypass.test.ts (1)

4-34: Bypass-path interceptor test looks correct.

Mock ordering, 401 rejection invocation, and assertions for all auth localStorage keys are aligned with the intended bypass-on behavior.

Walkthrough

ESLint configuration was extended with @eslint-react/* rules (including jsx-no-leaked-dollar, jsx-no-useless-fragment, dom-no-missing-button-type, dom-no-unsafe-target-blank, no-duplicate-key, no-unstable-context-value, and no-unstable-default-props). Multiple components and stories were updated to add explicit type="button" attributes on buttons. Tests were adjusted: one test file now hoists a mock for the dev auth bypass flag, and a new test file was added to exercise apiClient 401 interceptor behavior with the bypass enabled.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and accurately summarizes the main changes: enabling additional eslint-react rules and fixing the resulting violations.
Description check ✅ Passed The description is directly related to the changeset, providing detailed context about the enabled rules, violations fixed, and the flaky test resolution.
Docstring Coverage ✅ Passed Docstring coverage is 40.00% which is sufficient. The required threshold is 40.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 3, 2026 •
edited
Loading

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Snapshot Warnings

⚠️: No snapshots were found for the head SHA cfabb89.
Ensure that dependencies are being submitted on PR branches. Re-running this action after a short time may resolve the issue. See the documentation for more information and troubleshooting advice.

Scanned Files

None

Copilot AI reviewed Apr 3, 2026
View reviewed changes
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR strengthens the web frontend’s linting by enabling additional @eslint-react rules (beyond recommended-typescript) and updates existing code/tests/stories to comply, plus stabilizes a previously flaky API client test impacted by the dev auth bypass flag.

Changes:

  • Enabled 7 additional @eslint-react rules in web/eslint.config.js.
  • Fixed dom-no-missing-button-type violations by adding type="button" to non-submit buttons across components, stories, and tests.
  • Fixed a flaky 401-interceptor test by mocking @/utils/dev to ensure the dev auth bypass is disabled during the test.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file
File Description
web/eslint.config.js Enables additional @eslint-react rules to tighten React/DOM linting.
web/src/tests/api/client.test.ts Mocks dev auth bypass off so 401 interceptor behavior is deterministic in tests.
web/src/router/guards.tsx Adds type="button" to retry button to prevent accidental form submission.
web/src/components/ui/live-region.stories.tsx Adds type="button" to Storybook button to satisfy DOM lint rule.
web/src/components/ui/animated-presence.stories.tsx Adds type="button" to Storybook list buttons to satisfy DOM lint rule.
web/src/components/layout/StatusBar.tsx Adds type="button" to hamburger button for correct default button semantics.
web/src/components/layout/Sidebar.tsx Adds type="button" to footer buttons to satisfy DOM lint rule.
web/src/tests/components/ui/section-card.test.tsx Adds type="button" to test button to satisfy DOM lint rule.
web/src/tests/components/ui/drawer.test.tsx Adds type="button" to opener button to satisfy DOM lint rule.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

coderabbitai[bot]
coderabbitai bot previously approved these changes Apr 3, 2026
View reviewed changes
gemini-code-assist[bot]
gemini-code-assist bot reviewed Apr 3, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces several new ESLint rules from @eslint-react to improve code quality and security, such as preventing leaked dollar signs in JSX and requiring explicit types for button elements. Consequently, numerous button elements across the codebase, including those in tests, layouts, and stories, have been updated to include type="button". Additionally, the API client tests now include a mock to disable development authentication bypass, ensuring proper testing of the 401 interceptor. I have no feedback to provide.

@Aureliolo
test: add bypass-mode test for 401 interceptor
cfabb89 
Verify that when IS_DEV_AUTH_BYPASS is true, the 401 response
interceptor does NOT clear localStorage. Separate file because
vi.mock is file-scoped.

Pre-reviewed by 5 local agents + 3 external reviewers, 1 finding addressed.
@Aureliolo Aureliolo merged commit 80423be into main Apr 3, 2026
29 checks passed
@Aureliolo Aureliolo deleted the chore/enable-eslint-react-rules branch April 3, 2026 08:28
@Aureliolo Aureliolo mentioned this pull request Apr 3, 2026
Merged
Aureliolo added a commit that referenced this pull request Apr 3, 2026
@Aureliolo @github-actions
chore(main): release 0.5.8 (#1002)
7639021 
🤖 I have created a release *beep* *boop*
---


##
[0.5.8](v0.5.7...v0.5.8)
(2026-04-03)


### Features

* auto-select embedding model + fine-tuning pipeline wiring
([#999](#999))
([a4cbc4e](a4cbc4e)),
closes [#965](#965)
[#966](#966)
* ceremony scheduling batch 3 -- milestone strategy, template defaults,
department overrides
([#1019](#1019))
([321d245](321d245))
* five-pillar evaluation framework for HR performance tracking
([#1017](#1017))
([5e66cbd](5e66cbd)),
closes [#699](#699)
* populate comparison page with 53 competitor entries
([#1000](#1000))
([5cb232d](5cb232d)),
closes [#993](#993)
* throughput-adaptive and external-trigger ceremony scheduling
strategies ([#1003](#1003))
([bb5c9a4](bb5c9a4)),
closes [#973](#973)
[#974](#974)


### Bug Fixes

* eliminate backup service I/O from API test lifecycle
([#1015](#1015))
([08d9183](08d9183))
* update run_affected_tests.py to use -n 8
([#1014](#1014))
([3ee9fa7](3ee9fa7))


### Performance

* reduce pytest parallelism from -n auto to -n 8
([#1013](#1013))
([43e0707](43e0707))


### CI/CD

* bump docker/login-action from 4.0.0 to 4.1.0 in the all group
([#1027](#1027))
([e7e28ec](e7e28ec))
* bump wrangler from 4.79.0 to 4.80.0 in /.github in the all group
([#1023](#1023))
([1322a0d](1322a0d))


### Maintenance

* bump github.com/mattn/go-runewidth from 0.0.21 to 0.0.22 in /cli in
the all group
([#1024](#1024))
([b311694](b311694))
* bump https://github.com/astral-sh/ruff-pre-commit from v0.15.8 to
0.15.9 in the all group
([#1022](#1022))
([1650087](1650087))
* bump node from `71be405` to `387eebd` in /docker/sandbox in the all
group ([#1021](#1021))
([40bd2f6](40bd2f6))
* bump node from `cf38e1f` to `ad82eca` in /docker/web in the all group
([#1020](#1020))
([f05ab9f](f05ab9f))
* bump the all group in /web with 3 updates
([#1025](#1025))
([21d40d3](21d40d3))
* bump the all group with 2 updates
([#1026](#1026))
([36778de](36778de))
* enable additional eslint-react rules and fix violations
([#1028](#1028))
([80423be](80423be))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@coderabbitai coderabbitai[bot] coderabbitai[bot] approved these changes

+1 more reviewer

@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Aureliolo