-
Notifications
You must be signed in to change notification settings - Fork 39
chore: Switched to Eslint from Next Lint (Soon to be Deprecated) #138
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
@Thunder-Blaze is attempting to deploy a commit to the mrimmortal09's projects Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughReordered ESLint config blocks in eslint.config.mjs to begin and end with ignores around extends. Updated useContributors error handling to use unknown with safe message extraction. Changed lint script from next lint to eslint . and added @eslint/eslintrc to dependencies and devDependencies. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes ✨ Finishing Touches
🧪 Generate unit tests
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
hooks/useContributors.ts (1)
48-56: Initial fetch result is never sorted.The sort effect only runs on
sortBychanges. After the first fetch,contributorsstays unsorted.Apply this diff to sort immediately after fetching (and keep the effect for subsequent
sortBychanges):- const data = await response.json() - setContributors(data) + const data = (await response.json()) as TypeContributors[] + const sorted = + sortBy === 'contributions' + ? [...data].sort((a, b) => b.contributions - a.contributions) + : [...data].sort((a, b) => a.login.localeCompare(b.login)) + setContributors(sorted)package.json (1)
57-64: Add missing ESLint peer dependencies
eslint-config-next’s TS setup and React Hooks rules require these devDependencies:
- @typescript-eslint/parser
- @typescript-eslint/eslint-plugin
- eslint-plugin-react-hooks
Install them in package.json and re-run your lint check to confirm resolution.
🧹 Nitpick comments (4)
hooks/useContributors.ts (2)
22-46: Abort in-flight fetch to avoid setting state on unmounted component.Prevents memory leaks and React warnings during rapid navigation.
Example change:
useEffect(() => { const ac = new AbortController(); const fetchContributors = async () => { setIsLoading(true); try { const response = await fetch( `https://api.github.com/repos/${RepoDetails.owner}/${RepoDetails.repo}/contributors`, { signal: ac.signal, headers: { Accept: 'application/vnd.github+json' } } ); if (!response.ok) { throw new Error(`Failed to fetch contributors: ${response.status} ${response.statusText}`); } const data = (await response.json()) as TypeContributors[]; // initial sort here (see previous comment)... } catch (err) { if (ac.signal.aborted) return; setError(err instanceof Error ? err.message : String(err)); } finally { if (!ac.signal.aborted) setIsLoading(false); } }; fetchContributors(); return () => ac.abort(); }, [/* repo details if they can change */]);
29-31: Propagate HTTP context in errors.Including
status/statusTexthelps users and logs.Apply this diff:
- if (!response.ok) { - throw new Error('Failed to fetch contributors') - } + if (!response.ok) { + throw new Error(`Failed to fetch contributors: ${response.status} ${response.statusText}`) + }package.json (1)
9-9: Tighten lint script for CI.Fail on warnings to keep the bar high; add an autofix script.
- "lint": "eslint .", + "lint": "eslint . --max-warnings=0", + "lint:fix": "eslint . --fix",eslint.config.mjs (1)
12-22: Duplicate ignores; simplify config array.Keep a single ignores block; no need to repeat it at the end.
-const eslintConfig = [{ - ignores: ["node_modules/**", ".next/**", "out/**", "build/**", "next-env.d.ts"] -}, ...compat.extends("next/core-web-vitals", "next/typescript"), { - ignores: [ - "node_modules/**", - ".next/**", - "out/**", - "build/**", - "next-env.d.ts", - ], -}]; +const eslintConfig = [ + { ignores: ["node_modules/**", ".next/**", "out/**", "build/**", "next-env.d.ts"] }, + ...compat.extends("next/core-web-vitals", "next/typescript"), +];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
eslint.config.mjs(1 hunks)hooks/useContributors.ts(1 hunks)package.json(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
hooks/useContributors.ts (1)
app/contributors/page.tsx (1)
useContributors(5-80)
🔇 Additional comments (2)
hooks/useContributors.ts (1)
34-39: Good move tounknown+ safe message extraction.More robust and aligns with TS best practices.
eslint.config.mjs (1)
12-22: Incorrect suggestion: Next v15 lacks a native flat export
eslint-config-next v15 does not ship a built-in “eslint-config-next/flat” export; you must continue using FlatCompat (or a community-maintained flat port) for ESLint v9 flat-config support.Likely an incorrect or invalid review comment.
0d218de to
a1d8e6b
Compare
Description
Live Demo (if any)
Note for Maintainer
Checkout
Summary by CodeRabbit
Bug Fixes
Chores