Consolidate dual ESLint config to flat config - #170
Conversation
- Merge .eslintrc.cjs rules into eslint.config.js (ESLint 9+ flat config) - Add react-hooks plugin and rules - Add @typescript-eslint/no-explicit-any rule - Set no-unused-vars to error with argsIgnorePattern: ^_ - Delete legacy .eslintrc.cjs file - Fix react-hooks/set-state-in-effect violations by using lazy initializers - Update CONTRIBUTING.md with ESLint 9+ requirement
![infinitcode-ai[bot]](https://avatars.githubusercontent.com/in/1117690?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary
The pull request migrates ESLint configuration from .eslintrc.cjs to eslint.config.js, consolidates globals, adds react-hooks plugin, and moves useEffect initialization logic into useState lazy initializers in CookieConsent and ThemeContext. The changes are generally positive, improving code organization and performance, but the removal of useEffect in ThemeContext introduces a potential issue where applyTheme is called during render, which may cause side effects in React's strict mode.
Review Verdict: ❌ Improvements Suggested
The PR is largely correct, but the side effect in ThemeContext's useState initializer violates React's rules and could cause issues in strict mode. This should be addressed before merging.
Changes
| File Path | Changes Detected |
|---|---|
.eslintrc.cjs |
• Deleted the old ESLint configuration file as part of migration to flat config. |
eslint.config.js |
• Added import for eslint-plugin-react-hooks and its recommended rules.• Extracted browser globals into a shared browserGlobals object to reduce duplication.• Removed the no-unused-vars rule from the JavaScript config (now handled by TypeScript config).• Added @typescript-eslint/no-explicit-any rule set to warn.• Updated @typescript-eslint/no-unused-vars rule to error with argsIgnorePattern. |
src/components/CookieConsent.tsx |
• Removed useEffect and moved localStorage check into useState lazy initializer for performance improvement. |
src/components/UploadDemo.tsx |
• Moved simulateUpload function definition above the useEffect that calls it, improving code readability and logical order. |
src/lib/ThemeContext.tsx |
• Removed useEffect and moved getInitialTheme and applyTheme calls into useState lazy initializer. |
Issues
🟠 Major Severity
1. Side effect in useState initializer in ThemeContext - src/lib/ThemeContext.tsx (lines 74-77)
Risk: Calling applyTheme inside the useState initializer is a side effect during render, which is not allowed in React. In strict mode, the initializer may be called twice, causing the theme to be applied twice and potentially leading to inconsistent UI state.
Fix: Move the applyTheme call into a useEffect or use a layout effect to apply the theme after the initial render.
- const [theme, setTheme] = useState<Theme>(() => {
- const initial = getInitialTheme();
- applyTheme(initial);
- return initial;
- });
+ const [theme, setTheme] = useState<Theme>(getInitialTheme);
+
+ useEffect(() => {
+ applyTheme(theme);
+ }, [theme]);Code Style & Consistency
All identifiers follow project casing conventions.
Hot Take
"This PR is like a chef who rearranges the kitchen but forgets to turn off the stove - moving
applyThemeintouseStateinitializer is a recipe for double-cooked bugs in strict mode. At least the ESLint config got a nice haircut, even if it's still wearing the same old globals."
Example Commands:
@infinitcodeai review Trigger an instant AI PR review.
@infinitcodeai {prompt} Ask anything about your codebase.
Note: For additional settings, navigate to Infinitcode AI.
About
Automated review powered by Infinitcode AI
Report generated at 4/23/2026, 2:16:36 PM
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| import reactRefresh from 'eslint-plugin-react-refresh'; | ||
| import globals from 'globals'; | ||
|
|
||
| const browserGlobals = { |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 0 |
| Duplication | 0 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR consolidates the repository onto ESLint’s flat-config setup by folding the old .eslintrc.cjs rules into eslint.config.js, then updates a few React components and docs to align with the stricter linting setup.
Changes:
- Merges legacy ESLint rules into the flat config and adds
react-hooks/ stricter TypeScript lint rules. - Refactors
ThemeContextandCookieConsentinitialization to avoid state-setting inside effects, and rearrangesUploadDemo. - Removes the legacy
.eslintrc.cjsfile and documents the flat-config migration inCONTRIBUTING.md.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/lib/ThemeContext.tsx |
Refactors theme initialization/application logic for hook-rule compliance. |
src/components/UploadDemo.tsx |
Moves upload simulation helper within the component. |
src/components/CookieConsent.tsx |
Switches consent visibility initialization to a lazy state initializer. |
eslint.config.js |
Becomes the single source of ESLint configuration and adds new rules/plugins. |
CONTRIBUTING.md |
Updates contributor prerequisites/documentation for flat config. |
.eslintrc.cjs |
Removes the legacy ESLint config file. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -1,4 +1,4 @@ | |||
| import { createContext, useEffect, useState, type ReactNode } from 'react'; | |||
| import { createContext, useState, type ReactNode } from 'react'; | |||
| const simulateUpload = () => { | ||
| setIsUploading(true); | ||
| setTimeout(() => { | ||
| setIsUploading(false); | ||
| setIsComplete(true); | ||
| }, 2000); | ||
| }; |
| - Node.js 18+ | ||
| - npm 9+ | ||
| - Git | ||
| - ESLint 9+ (for local development, ESLint uses flat config) |
| // Apply theme to DOM after initial render (side effect belongs in useEffect) | ||
| useEffect(() => { | ||
| const initialTheme = getInitialTheme(); | ||
| setTheme(initialTheme); | ||
| applyTheme(initialTheme); | ||
| }, []); | ||
| applyTheme(theme); | ||
| }, [theme]); |
Resolved conflicts: - eslint.config.js: consolidated rules with error level for no-explicit-any - CookieConsent.tsx: use getInitialConsentState helper - UploadDemo.tsx: useCallback for simulateUpload with proper deps - ThemeContext.tsx: useEffect for applyTheme (fixes side effect in initializer)
643c776 to
d8c90ec
Compare
|
Summary
.eslintrc.cjsintoeslint.config.js(ESLint 9+ flat config format)@typescript-eslint/no-explicit-any: warnruleno-unused-varsto error withargsIgnorePattern: ^_.eslintrc.cjsfile (silently ignored by ESLint 9+)react-hooks/set-state-in-effectviolations by using lazy initializersAcceptance Criteria
Testing Steps
npm run lint- should pass with 0 warningsnpm run build- should build successfully.eslintrc*files exist at root