feat: add ESLint configuration for frontend (#676)#732
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a frontend analytics tracking layer and updates lint/type-check tooling across the repo (ESLint for frontend; Ruff/Mypy/Mutmut for Python packages).
Changes:
- Introduce a new frontend analytics service plus React hooks for event tracking.
- Extend the frontend ESLint flat config with React-related plugins/rules and add related dev dependencies (plus Stryker).
- Adjust Python lint/type-check configuration in
backendandai-engine(Ruff rule sets, Mypy strictness, Mutmut config).
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/services/analytics.ts | New analytics service API for posting tracking events and generating anon IDs. |
| frontend/src/hooks/useAnalytics.ts | New hooks wrapping analytics service (page tracking, conversion/upload/export helpers). |
| frontend/package.json | Adds React ESLint plugins and Stryker mutation-testing deps. |
| frontend/eslint.config.js | Enables React/React Compiler linting integration in flat ESLint config. |
| backend/pyproject.toml | Updates Mypy strictness/options, extends Ruff rules/ignores, adds Mutmut config. |
| ai-engine/pyproject.toml | Extends Ruff rules/ignores, adds Mutmut config, and loosens Mypy strictness/options. |
| export const trackPageView = ( | ||
| page: string, | ||
| options: Omit<AnalyticsOptions, 'properties'> = {} | ||
| ): void => { | ||
| trackEvent(AnalyticsEventType.PAGE_VIEW, AnalyticsEventCategory.NAVIGATION, { | ||
| ...options, | ||
| properties: { | ||
| ...options.properties, | ||
| page, | ||
| }, | ||
| }); | ||
| }; |
There was a problem hiding this comment.
options is typed as Omit<AnalyticsOptions, 'properties'>, but trackPageView reads options.properties (line 148). This is a type error and will fail TypeScript compilation. Fix by either (a) accepting AnalyticsOptions here, or (b) changing the signature to accept a separate properties?: Record<string, unknown> parameter (or a type that includes properties).
| const getUserId = (): string | undefined => { | ||
| // For now, we'll use a generated anonymous ID | ||
| // In the future, this could be tied to actual user accounts | ||
| let userId = localStorage.getItem('analytics_user_id'); | ||
| if (!userId) { | ||
| userId = `anon_${Date.now()}_${Math.random().toString(36).substr(2, 9)}`; | ||
| localStorage.setItem('analytics_user_id', userId); | ||
| } | ||
| return userId; | ||
| }; |
There was a problem hiding this comment.
getUserIdValue() is declared to return string, but it returns getUserId() which is typed as string | undefined. This will fail TypeScript compilation. Since getUserId() always returns a string (it creates one when missing), update getUserId's return type to string (and keep getUserIdValue as string), or otherwise make the return types consistent.
| * Get or create an anonymous user ID. | ||
| */ | ||
| export const getUserIdValue = (): string => { | ||
| return getUserId(); |
There was a problem hiding this comment.
getUserIdValue() is declared to return string, but it returns getUserId() which is typed as string | undefined. This will fail TypeScript compilation. Since getUserId() always returns a string (it creates one when missing), update getUserId's return type to string (and keep getUserIdValue as string), or otherwise make the return types consistent.
| return getUserId(); | |
| return getUserId() ?? ''; |
| // React 19 strict rules | ||
| // React Compiler (formerly React Forget) rules | ||
| 'react-compiler/react-compiler': 'warn', | ||
| // Warn about deprecated React APIs | ||
| 'react-hooks/set-state-in-effect': 'off', // Allow setState in effects for data fetching patterns | ||
| // New React 19 rules would go here when eslint-plugin-react releases them |
There was a problem hiding this comment.
eslint-plugin-react-hooks (added as react-hooks) does not define a set-state-in-effect rule in the commonly used rule set (and with the declared dependency eslint-plugin-react-hooks: ^7.0.1). ESLint will error on unknown rules. Remove this rule entry or replace it with a valid rule name from the installed plugin.
| }, | ||
| settings: { | ||
| react: { | ||
| version: '19.2', |
There was a problem hiding this comment.
Hardcoding the React version in ESLint settings can drift from the actual installed React version and cause incorrect lint behavior. Prefer version: 'detect' (or omit the setting) so the plugin can infer the correct version from dependencies.
| version: '19.2', | |
| version: 'detect', |
| show_error_codes = true | ||
| namespace_packages = true | ||
| explicit_package_bases = true | ||
| exclude = ["tests/", "scripts/", ".venv/", "docs/", "examples/", "__pycache__/", "__main__.py"] |
There was a problem hiding this comment.
Mypy’s exclude setting is typically a single regex pattern; providing an array may not be parsed as intended depending on the mypy version/config loader. Consider converting this to a single regex (e.g., using (...)|(...)), or verify that the chosen mypy version supports TOML arrays here.
| exclude = ["tests/", "scripts/", ".venv/", "docs/", "examples/", "__pycache__/", "__main__.py"] | |
| exclude = '(^tests/|^scripts/|^\.venv/|^docs/|^examples/|^__pycache__/|__main__\.py$)' |
- Add ruff configuration to pyproject.toml for backend/ai-engine linting - Enable rules for common errors (E, F), imports (I), style (Q), naming (N), docstrings (D), and pyupgrade (UP) - Add pydocstyle (D) and pyupgrade (UP) ignore rules for legacy code - Exclude mutants/ directory from backend checks
- Add .eslintrc.json or eslint.config.js (using flat config) - Enable TypeScript ESLint parser - Configure strict rules for React 19 - Add React global for JSX support - Add React Compiler (formerly React Forget) rules - Add eslint-plugin-react and eslint-plugin-react-compiler - Update package.json with new dependencies - Add analytics service files
- Fixed unused 'event' parameter in useAnalytics.ts (changed to _event) - Added eslint-plugin-react and react-compiler to eslint.config.js - Added no-redeclare disable rule for analytics.ts - Restored per-file-ignores for performance.py, rate_limiter.py, task_worker.py - Applied Prettier formatting to ConversionFlowManager.tsx and analytics.ts Co-authored-by: openhands <openhands@all-hands.dev>
74bd5a6 to
12a103e
Compare
Closes #676