Handle GitHub API deprecations + Contributors/Profile fix #205
Conversation
…butors + profile working
|
Warning Rate limit exceeded@ASR1015 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 19 minutes and 30 seconds before requesting another review. ⌛ 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. 📒 Files selected for processing (1)
WalkthroughReplace BrowserRouter with React Router v6.4+ data-router and RootLayout; add ghFetch/ghJson and Octokit factory; refactor ContributorProfile, Contributors, and useGitHubData to use token-authenticated GitHub REST/GraphQL calls; update constants, deps, and export AppRouter. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Router as RouterProvider
participant Root as RootLayout
participant Page as RouteComponent
Note over Router,Page: data-router resolves route and renders nested element
User->>Router: Navigate to path
Router->>Root: Render RootLayout (ThemeWrapper, Navbar, Footer)
Root->>Page: Render nested route via Outlet (Suspense)
Page-->>User: Display content / loading / error
sequenceDiagram
autonumber
participant Page as Contributors / ContributorProfile
participant GHHelper as ghFetch / octokit.graphql
participant API as GitHub API
Note right of GHHelper #DDEBF7: Requests include Accept and X-GitHub-Api-Version\nAuthorization when token present
Page->>GHHelper: Request (REST path or GraphQL, token)
GHHelper->>API: HTTP/GraphQL request
API-->>GHHelper: Response (ok / error)
GHHelper-->>Page: Return JSON or throw error
alt ok
Page->>Page: Map nodes→items, update state
else error
Page->>Page: Surface error (toast/state)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Deploy Preview for github-spy ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Hi, I have fixed the GitHub API deprecation warnings by adding the required application/vnd.github+json header and updating contributor/profile fetch logic with PAT support. I have tested locally (contributors list + profile pages working fine). Please review and let me know if any changes are needed. |
|
The PR is ready for review. Please let me know if any changes are needed. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (13)
src/lib/githubFetch.ts (2)
9-17: Normalize path handling (supports absolute URLs and avoids double slashes).Prevents subtle bugs when callers pass paths without a leading slash or a full URL.
- return fetch(`${base}${path}`, { ...init, headers }); + const url = path.startsWith("http") + ? path + : `${base}${path.startsWith("/") ? "" : "/"}${path}`; + return fetch(url, { ...init, headers });
11-13: Pick one Authorization scheme and document it.Use a single scheme to avoid confusion. “Bearer” works for fine‑grained and classic PATs.
Confirm your tokens are fine‑grained/classic PATs; if any integration expects the old “token ” scheme, we can switch.
- if (token) { - headers.set("Authorization", `Bearer ${token}`); // or `token ${token}` - } + if (token) { + headers.set("Authorization", `Bearer ${token}`); + }src/Routes/Router.tsx (2)
11-20: Add a 404 route.Prevents blank screens for unknown paths.
{ path: "/contributor/:username", element: <ContributorProfile /> }, + { path: "*", element: <div className="p-6">Not Found</div> },
1-1: Consider lazy-loading route components to cut initial bundle size.Wrap provider in Suspense and convert imports to lazy.
-import { createBrowserRouter, RouterProvider } from "react-router-dom"; +import { createBrowserRouter, RouterProvider } from "react-router-dom"; +import { lazy, Suspense } from "react";-import Tracker from "../pages/Tracker/Tracker.tsx"; -import About from "../pages/About/About"; -import Contact from "../pages/Contact/Contact"; -import Contributors from "../pages/Contributors/Contributors"; -import Signup from "../pages/Signup/Signup.tsx"; -import Login from "../pages/Login/Login.tsx"; -import ContributorProfile from "../pages/ContributorProfile/ContributorProfile.tsx"; -import Home from "../pages/Home/Home.tsx"; +const Tracker = lazy(() => import("../pages/Tracker/Tracker.tsx")); +const About = lazy(() => import("../pages/About/About")); +const Contact = lazy(() => import("../pages/Contact/Contact")); +const Contributors = lazy(() => import("../pages/Contributors/Contributors")); +const Signup = lazy(() => import("../pages/Signup/Signup.tsx")); +const Login = lazy(() => import("../pages/Login/Login.tsx")); +const ContributorProfile = lazy(() => import("../pages/ContributorProfile/ContributorProfile.tsx")); +const Home = lazy(() => import("../pages/Home/Home.tsx"));-export default function AppRouter() { - return <RouterProvider router={router} />; -} +export default function AppRouter() { + return ( + <Suspense fallback={<div className="p-4">Loading...</div>}> + <RouterProvider router={router} /> + </Suspense> + ); +}src/pages/ContributorProfile/ContributorProfile.tsx (1)
106-117: Guard repository_url before splitting.Avoids runtime errors if a result lacks repository_url.
- {prs.map((pr) => { - const repoName = pr.repository_url.split("/").slice(-2).join("/"); + {prs.map((pr) => { + const repoName = pr.repository_url + ? pr.repository_url.split("/").slice(-2).join("/") + : "unknown/unknown";src/App.tsx (2)
8-16: Unify import style (drop .tsx extensions).Keeps imports consistent and avoids lint noise. Safe with typical TS/bundler configs.
-import Tracker from "./pages/Tracker/Tracker.tsx"; +import Tracker from "./pages/Tracker/Tracker"; @@ -import Signup from "./pages/Signup/Signup.tsx"; +import Signup from "./pages/Signup/Signup"; -import Login from "./pages/Login/Login.tsx"; +import Login from "./pages/Login/Login"; -import ContributorProfile from "./pages/ContributorProfile/ContributorProfile.tsx"; +import ContributorProfile from "./pages/ContributorProfile/ContributorProfile"; -import Home from "./pages/Home/Home.tsx"; +import Home from "./pages/Home/Home";
.
17-25: Avoid global vertical centering in
items-center(andjustify-center) will vertically center every page, which is awkward for scrollable content (e.g., lists). Prefer top-start layout.- <main className="flex-grow bg-gray-50 dark:bg-gray-800 flex justify-center items-center"> + <main className="flex-grow bg-gray-50 dark:bg-gray-800 flex justify-start items-stretch"> <Outlet /> </main>Optionally add scroll restoration:
+ {/* Keeps scroll position consistent on navigation */} + <ScrollRestoration />And import it:
-import { createBrowserRouter, RouterProvider, Outlet } from "react-router-dom"; +import { createBrowserRouter, RouterProvider, Outlet, ScrollRestoration } from "react-router-dom";src/pages/Contributors/Contributors.tsx (6)
4-7: Strongly type contributors to avoidany[].Add a minimal type:
type Contributor = { id: number; login: string; html_url: string; avatar_url: string; contributions: number; };Then:
-const [contributors, setContributors] = useState<any[]>([]); +const [contributors, setContributors] = useState<Contributor[]>([]);
10-17: getToken is unused; either use it or remove it.Use it to dedupe token retrieval.
- const token = - localStorage.getItem("gh_pat") || - localStorage.getItem("github_pat") || - localStorage.getItem("token") || - ""; + const token = await getToken();(Apply within loadContributors; see diff below.)
56-61: Duplicate token retrieval.Replace the inline localStorage reads with the getToken helper.
- const token = - localStorage.getItem("gh_pat") || - localStorage.getItem("github_pat") || - localStorage.getItem("token") || - ""; + const token = await getToken();
62-71: Fetching contributors: endpoint usage is fine; consider including anonymous contributors if desired.Optional: append
&anon=1if you want to include anonymous commits in the list.- `/repos/${owner}/${repo}/contributors?per_page=50`, + `/repos/${owner}/${repo}/contributors?per_page=50&anon=1`,(Only if this matches your UX.)
80-87: Error surfacing is good; add a hint when hitting 403 (rate limit).On 403, append “(rate limited or token missing/insufficient scope)”.
No code change required; informational.
101-108: Link to the in-app profile view as well.Make it easy to jump to /contributor/:username without leaving the app.
- <a href={c.html_url} target="_blank" rel="noreferrer" className="text-blue-700 hover:underline"> - {c.login} - </a> + <a href={`/contributor/${c.login}`} className="text-blue-700 hover:underline"> + {c.login} + </a> + <a href={c.html_url} target="_blank" rel="noreferrer" className="text-sm text-blue-700 hover:underline opacity-80"> + (GitHub) + </a>If you prefer SPA navigation, switch the first anchor to Link and import it:
import { Link } from "react-router-dom";- <a href={`/contributor/${c.login}`} className="text-blue-700 hover:underline"> + <Link to={`/contributor/${c.login}`} className="text-blue-700 hover:underline"> {c.login} - </a> + </Link>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
src/App.tsx(2 hunks)src/Routes/Router.tsx(2 hunks)src/lib/githubFetch.ts(1 hunks)src/pages/ContributorProfile/ContributorProfile.tsx(4 hunks)src/pages/Contributors/Contributors.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
src/App.tsx (5)
src/pages/Contributors/Contributors.tsx (1)
Contributors(4-116)src/pages/ContributorProfile/ContributorProfile.tsx (1)
ContributorProfile(20-127)src/Routes/Router.tsx (1)
AppRouter(22-24)src/pages/Home/Home.tsx (1)
Home(5-14)src/pages/Tracker/Tracker.tsx (3)
theme(48-349)_(238-241)_(94-96)
src/Routes/Router.tsx (1)
src/App.tsx (1)
AppRouter(60-67)
src/lib/githubFetch.ts (2)
src/hooks/useGitHubData.ts (3)
getOctokit(3-71)octokit(12-26)username(29-58)src/hooks/useGitHubAuth.ts (3)
username(4-25)username(9-12)octokit(14-14)
src/pages/ContributorProfile/ContributorProfile.tsx (2)
src/pages/Tracker/Tracker.tsx (1)
GitHubItem(38-46)src/hooks/useGitHubData.ts (1)
username(29-58)
🔇 Additional comments (4)
src/App.tsx (3)
1-1: Good switch to data router.Using createBrowserRouter/RouterProvider is the right move for v6.4+.
35-36: Toaster config looks good.Reasonable durations and dark-mode styling.
60-67: Replaceas anywithas constfor thefutureprop- future={{ v7_startTransition: true } as any} + future={{ v7_startTransition: true } as const}Version ^6.28.0 supports the v7 flags; main.tsx imports and renders the default export (AppRouter) as
<App/>.src/pages/Contributors/Contributors.tsx (1)
1-1: ghFetch import LGTM.Assuming ghFetch sets X-GitHub-Api-Version: 2022-11-28 and Authorization when token is present.
Could you confirm githubFetch adds the version header and Accept: application/vnd.github+json?
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/App.tsx (1)
58-60: Heads-up: ContributorProfile route depends on deprecated /search/issues.Endpoint removal date is September 4, 2025—migrate ASAP (GraphQL or new REST).
Want a follow-up PR checklist for this migration?
🧹 Nitpick comments (6)
src/App.tsx (6)
1-2: Add ScrollRestoration for better UX on navigation.Restores scroll position between route changes.
Apply:
-import { createBrowserRouter, RouterProvider, Outlet } from "react-router-dom"; +import { createBrowserRouter, RouterProvider, Outlet, ScrollRestoration } from "react-router-dom";
9-16: Normalize lazy import paths.Mixing extensions can hurt cache predictability; keep consistent.
-const Home = lazy(() => import("./pages/Home/Home.tsx")); -const Tracker = lazy(() => import("./pages/Tracker/Tracker.tsx")); +const Home = lazy(() => import("./pages/Home/Home")); +const Tracker = lazy(() => import("./pages/Tracker/Tracker")); const About = lazy(() => import("./pages/About/About")); const Contact = lazy(() => import("./pages/Contact/Contact")); const Contributors = lazy(() => import("./pages/Contributors/Contributors")); -const Signup = lazy(() => import("./pages/Signup/Signup.tsx")); -const Login = lazy(() => import("./pages/Login/Login.tsx")); -const ContributorProfile = lazy(() => import("./pages/ContributorProfile/ContributorProfile.tsx")); +const Signup = lazy(() => import("./pages/Signup/Signup")); +const Login = lazy(() => import("./pages/Login/Login")); +const ContributorProfile = lazy(() => import("./pages/ContributorProfile/ContributorProfile"));
18-44: Add ScrollRestoration and adjust layout alignment.Centering all pages can be awkward for long lists; start at top-left.
<Navbar /> - <main className="flex-grow bg-gray-50 dark:bg-gray-800 flex justify-center items-center"> + <ScrollRestoration /> + <main className="flex-grow bg-gray-50 dark:bg-gray-800 flex justify-center items-start">
25-27: Use an accessible Suspense fallback.Improves screen reader experience.
- <Suspense fallback={<div className="p-6">Loading…</div>}> + <Suspense fallback={<div role="status" aria-live="polite" className="p-6">Loading…</div>}>
64-70: Consolidate router definitions into one AppRouter. You have twoexport default function AppRouter()definitions (in src/App.tsx and src/Routes/Router.tsx); remove the redundant one or re-export from a single module to maintain a single source of truth.
46-63: Add 404 fallback route and configure router basenameNeither your Vite config nor package.json defines a custom base path (no base in vite.config.ts, no homepage/BASE_URL in package.json), so if you deploy to GitHub Pages you’ll need to pass
basenametocreateBrowserRouter. You should also add a catch-all route to prevent blank screens on unknown URLs:const router = createBrowserRouter([ { path: "/", element: <RootLayout />, errorElement: <div className="p-6">Something went wrong loading this page.</div>, children: [ { index: true, element: <Home /> }, { path: "track", element: <Tracker /> }, { path: "signup", element: <Signup /> }, { path: "login", element: <Login /> }, { path: "about", element: <About /> }, { path: "contact", element: <Contact /> }, { path: "contributors", element: <Contributors /> }, { path: "contributor/:username", element: <ContributorProfile /> }, + { path: "*", element: <div className="p-6">Page not found.</div> }, ], }, -]); +], { basename: import.meta.env.BASE_URL || "/" });(optional) verify or set a
basein vite.config.ts or a homepage field in package.json if you’re deploying under a subpath.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/App.tsx(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/App.tsx (1)
src/Routes/Router.tsx (2)
AppRouter(22-24)Routes(11-24)
🔇 Additional comments (1)
src/App.tsx (1)
30-39: Toaster config looks good.Sane defaults and dark-mode aware.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/lib/githubFetch.ts (4)
10-15: Don’t override caller Authorization; accept preformatted schemes and preserve custom Accept.Avoid clobbering an Authorization already provided via init.headers, and allow callers to pass a preformatted "Bearer ..." or "token ..." string. Also set Accept only if absent so callers can opt into previews.
Apply this diff:
- const headers = new Headers(init.headers || {}); - if (token) { - headers.set("Authorization", `Bearer ${token}`); // or `token ${token}` - } - headers.set("Accept", "application/vnd.github+json"); + const headers = new Headers(init.headers || {}); + // Preserve caller-provided Authorization; allow preformatted schemes. + if (!headers.has("Authorization") && token) { + const preformatted = token.startsWith("Bearer ") || token.startsWith("token "); + headers.set("Authorization", preformatted ? token : `Bearer ${token}`); + } + if (!headers.has("Accept")) { + headers.set("Accept", "application/vnd.github+json"); + } headers.set("X-GitHub-Api-Version", GITHUB_API_VERSION);
9-10: Build the request URL with URL(base, path) for robust joining and absolute-URL support.Handles missing/extra slashes and lets callers pass absolute URLs without double-prefixing.
Apply this diff:
- return fetch(`${base}${path}`, { ...init, headers }); + const url = new URL(path, base).toString(); + return fetch(url, { ...init, headers });Also applies to: 17-17
4-8: Annotate ghFetch return type.Minor clarity/readability win in TS.
Apply this diff:
-export async function ghFetch( +export async function ghFetch( path: string, token: string, init: RequestInit = {} -) { +): Promise<Response> {
20-33: Throw a structured error with parsed GitHub payload (message, errors, docs URL) and headers.Improves debuggability (rate-limit headers, validation errors) and avoids logging entire raw bodies.
Apply this diff:
export async function ghJson<T>( path: string, token: string, init: RequestInit = {} ): Promise<T> { const res = await ghFetch(path, token, init); if (!res.ok) { - const text = await res.text().catch(() => ""); - throw new Error( - `GitHub API ${res.status} ${res.statusText}${text ? `: ${text}` : ""}` - ); + const ct = res.headers.get("content-type") || ""; + let body: unknown; + if (ct.includes("application/json")) { + body = await res.json().catch(() => undefined); + } else { + body = (await res.text().catch(() => ""))?.slice(0, 2000); + } + throw new GitHubApiError(res.status, res.statusText, body, res.headers); } return res.json() as Promise<T>; }Add this helper class (outside the ranges shown, e.g., right below the version constant):
export class GitHubApiError extends Error { constructor( public status: number, public statusText: string, public body: unknown, public headers: Headers ) { const summary = typeof body === "object" && body && "message" in (body as any) ? String((body as any).message) : typeof body === "string" ? body : ""; super(`GitHub API ${status} ${statusText}${summary ? `: ${summary}` : ""}`); this.name = "GitHubApiError"; } }Notes:
- UI can inspect x-ratelimit-remaining/reset to render friendlier messages.
- If you ever call ghJson on endpoints returning 204, consider a variant that returns void for those routes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/lib/githubFetch.ts(1 hunks)
🔇 Additional comments (1)
src/lib/githubFetch.ts (1)
2-2: Versioned/Accept headers are correct for GitHub REST.Good call setting X-GitHub-Api-Version and the vendor Accept; this removes unversioned warnings and aligns with current REST guidance.
Also applies to: 14-15
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
package.json (1)
33-45: Remove v5 type package for React Router; v6 ships types
@types/react-router-dom@^5.3.3conflicts withreact-router-dom@^6.28.0(v6 bundles its own types). Keeping it can cause wrong typings.Apply:
@@ - "@types/react-router-dom": "^5.3.3",
🧹 Nitpick comments (3)
package.json (1)
19-28: Remove unusedoctokitdependency from package.json
Automated scans across the codebase found no ES-module imports or CommonJS requires of the legacyoctokitpackage, so you can safely delete the"octokit": "^4.0.2"entry (and clean up your lockfile).src/utils/constants.ts (2)
1-4: Allow env overrides for GH Enterprise and future version bumpsEnable configuring base URL and API version via env (Vite) while keeping current defaults.
Apply:
-export const GITHUB_API_VERSION = "2022-11-28"; +export const GITHUB_API_VERSION = + import.meta?.env?.VITE_GITHUB_API_VERSION ?? "2022-11-28"; -export const GITHUB_API_BASE = "https://api.github.com"; +export const GITHUB_API_BASE = + import.meta?.env?.VITE_GITHUB_API_BASE ?? "https://api.github.com";
3-7: Minor: centralize Accept header constant (optional)If multiple callers set
Accept, export a constant to avoid string drift.Example:
export const GITHUB_ACCEPT = "application/vnd.github+json";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
package.json(1 hunks)src/lib/octokit.ts(1 hunks)src/main.tsx(0 hunks)src/pages/ContributorProfile/ContributorProfile.tsx(4 hunks)src/utils/constants.ts(1 hunks)
💤 Files with no reviewable changes (1)
- src/main.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/pages/ContributorProfile/ContributorProfile.tsx
🧰 Additional context used
🧬 Code graph analysis (3)
package.json (1)
src/hooks/useGitHubData.ts (1)
getOctokit(3-71)
src/lib/octokit.ts (2)
src/hooks/useGitHubAuth.ts (3)
octokit(14-14)username(9-12)username(4-25)src/hooks/useGitHubData.ts (3)
getOctokit(3-71)octokit(12-26)username(29-58)
src/utils/constants.ts (1)
src/pages/Tracker/Tracker.tsx (1)
GitHubItem(38-46)
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/hooks/useGitHubData.ts (1)
61-81: Rate-limit detection is unreliable for GraphQL; handle 200 + errors and Octokit GraphqlResponseError.GraphQL often returns 200 with an errors array; err.status may be undefined.
- } catch (err: any) { - if (err.status === 403) { - setError('GitHub API rate limit exceeded. Please wait or use a token.'); - setRateLimited(true); // Prevent further fetches - } else { - setError(err.message || 'Failed to fetch data'); - } - } finally { + } catch (err: any) { + const msg = err?.message || 'Failed to fetch data'; + const isRateLimit = + err?.status === 403 || + /rate limit/i.test(msg) || + Array.isArray(err?.errors) && err.errors.some((e: any) => /rate limit/i.test(e?.message)); + if (isRateLimit) { + setError('GitHub API rate limit exceeded. Please wait or use a token.'); + setRateLimited(true); + } else { + setError(msg); + } + } finally {
♻️ Duplicate comments (1)
src/pages/ContributorProfile/ContributorProfile.tsx (1)
106-113: Await clipboard write to properly handle failures.- const handleCopyLink = () => { - try { - navigator.clipboard.writeText(window.location.href); + const handleCopyLink = async () => { + try { + await navigator.clipboard.writeText(window.location.href); toast.success("🔗 Shareable link copied to clipboard!"); } catch { toast.error("Could not copy link"); } };
🧹 Nitpick comments (1)
src/hooks/useGitHubData.ts (1)
3-3: Type the octokit factory and narrow “type”.Optional hardening: use proper types to catch regressions at compile time.
-export const useGitHubData = (getOctokit: () => any) => { +import type { Octokit } from '@octokit/core'; +export const useGitHubData = (getOctokit: () => Octokit) => { … - const fetchPaginated = async (octokit: any, username: string, type: string, page = 1, per_page = 10) => { + const fetchPaginated = async ( + octokit: Octokit, + username: string, + type: 'issue' | 'pr', + page = 1, + per_page = 10 + ) => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/hooks/useGitHubData.ts(1 hunks)src/pages/ContributorProfile/ContributorProfile.tsx(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/hooks/useGitHubData.ts (1)
src/pages/Tracker/Tracker.tsx (2)
GitHubItem(38-46)data(101-138)
src/pages/ContributorProfile/ContributorProfile.tsx (2)
src/pages/Contributors/Contributors.tsx (3)
contributors(27-142)Contributor(19-25)contributor(74-137)src/pages/Tracker/Tracker.tsx (1)
GitHubItem(38-46)
🔇 Additional comments (1)
src/pages/ContributorProfile/ContributorProfile.tsx (1)
79-83: Ensure Content-Type header is set for GraphQL POST.If githubFetch doesn’t add it, GitHub will reject the body.
- const prsRes = await gh("/graphql", token, { + const prsRes = await gh("/graphql", token, { method: "POST", - body: JSON.stringify(gqlQuery), + headers: { "Content-Type": "application/json" }, + body: JSON.stringify(gqlQuery), });If githubFetch already injects this header, skip this change.
…rrors; contributors repo resolution; clipboard await; router cleanup
…on and cursor handling
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/hooks/useGitHubData.ts (2)
47-52: Stop fabricating cursors; clamp page size and use real endCursor.btoa('cursor:...') is invalid and will 400/500 or page incorrectly. Use search.pageInfo.endCursor and cap first≤100. Also add a stable sort to prevent page drift.
Apply within these ranges:
- const queryString = `author:${username} is:${type}`; + const queryString = `author:${username} is:${type} sort:updated-desc`; const response = await octokit.graphql(query, { queryString, - first: per_page, - after: page > 1 ? btoa(`cursor:${(page - 1) * per_page}`) : null, + first: Math.min(100, per_page), + after: page > 1 ? (cursorsRef.current?.[type]?.[page - 1] ?? null) : null, });And outside this hunk:
- import { useState, useCallback } from 'react'; + import { useState, useCallback, useRef } from 'react';- const fetchPaginated = async (octokit: any, username: string, type: string, page = 1, per_page = 10) => { + const fetchPaginated = async ( + octokit: any, + username: string, + type: 'issue' | 'pr', + page = 1, + per_page = 10 + ) => {// add near other state in the hook const cursorsRef = useRef<{ issue: Record<number, string | null>; pr: Record<number, string | null> }>({ issue: {}, pr: {}, });
54-57: Return Tracker-shaped items and cache cursors.Tracker expects GitHubItem (id number, html_url, created_at, state, repository_url, pull_request.merged_at). Returning raw nodes breaks UI.
- return { - items: response.search.edges.map((edge: any) => edge.node), - total: response.search.issueCount, - }; + const endCursor = response.search.pageInfo?.endCursor ?? null; + // cache cursor for next page + cursorsRef.current[type] ||= {}; + cursorsRef.current[type][page] = endCursor; + + const items = response.search.edges.map((edge: any) => { + const n = edge.node; + const base = { + id: n.databaseId as number, + title: n.title as string, + html_url: n.url as string, + created_at: n.createdAt as string, + state: n.state as string, + repository_url: n.repository?.url as string, + }; + return 'mergedAt' in n && n.mergedAt + ? { ...base, pull_request: { merged_at: n.mergedAt as string } } + : base; + }); + return { items, total: response.search.issueCount as number };
🧹 Nitpick comments (6)
src/hooks/useGitHubData.ts (1)
81-86: Handle GraphQL rate limiting, not just HTTP 403.octokit.graphql often returns 200 with errors.type === 'RATE_LIMITED'. Detect that to set a meaningful error and allow retry after cooldown/token change.
- } catch (err: any) { - if (err.status === 403) { + } catch (err: any) { + const isRateLimited = + err?.status === 403 || + err?.errors?.some?.((e: any) => e?.type === 'RATE_LIMITED'); + if (isRateLimited) { setError('GitHub API rate limit exceeded. Please wait or use a token.'); setRateLimited(true); // Prevent further fetches } else { setError(err.message || 'Failed to fetch data'); }src/Routes/Router.tsx (1)
11-20: Add a catch-all 404 route.Prevents blank screens for unknown paths and improves UX.
{ path: "/contributor/:username", element: <ContributorProfile /> }, + { path: "*", element: <Home /> },src/pages/Contributors/Contributors.tsx (4)
10-17: Remove duplication: use getToken() below or delete it.You reimplement token retrieval later. Prefer a single helper.
- async function getToken() { + async function getToken() { return ( localStorage.getItem("gh_pat") || localStorage.getItem("github_pat") || localStorage.getItem("token") || "" ); }
19-50: Cache resolved repo to avoid repeated search calls.Persist to localStorage and reuse to reduce latency and rate-limit pressure.
- async function resolveRepoFullName(token: string): Promise<string | null> { + async function resolveRepoFullName(token: string): Promise<string | null> { + const cached = localStorage.getItem("gh_repo_full_name"); + if (cached) return cached; // Try to find the repo by searching both org and user scopes and dash/underscore variants const owners = ["GitMetricsLab", "gitmetricsslab", "ASR1015"]; // include common forks/owners const names = ["github-tracker", "github_tracker"]; // dash vs underscore @@ - if (match?.full_name) return match.full_name as string; + if (match?.full_name) { + localStorage.setItem("gh_repo_full_name", match.full_name as string); + return match.full_name as string; + } @@ - if (match2?.full_name) return match2.full_name as string; + if (match2?.full_name) { + localStorage.setItem("gh_repo_full_name", match2.full_name as string); + return match2.full_name as string; + }
56-61: Use getToken() here.Prevents drift if token sourcing changes.
- const token = - localStorage.getItem("gh_pat") || - localStorage.getItem("github_pat") || - localStorage.getItem("token") || - ""; + const token = await getToken();
67-70: Consider including anonymous contributors (optional).If you want complete counts, add anon=1.
- `/repos/${fullName}/contributors?per_page=50`, + `/repos/${fullName}/contributors?per_page=50&anon=1`,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
src/Routes/Router.tsx(2 hunks)src/hooks/useGitHubData.ts(1 hunks)src/pages/ContributorProfile/ContributorProfile.tsx(4 hunks)src/pages/Contributors/Contributors.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/pages/ContributorProfile/ContributorProfile.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
src/hooks/useGitHubData.ts (1)
src/pages/Tracker/Tracker.tsx (3)
GitHubItem(38-46)data(101-138)theme(48-349)
src/Routes/Router.tsx (1)
src/App.tsx (1)
App(8-42)
🔇 Additional comments (2)
src/hooks/useGitHubData.ts (1)
13-45: GraphQL selection now includes required fields and pageInfo — good.databaseId/state/repository.url/mergedAt and pageInfo are present; this unblocks Tracker mapping and real pagination.
src/pages/Contributors/Contributors.tsx (1)
62-66: LGTM: repo resolution with fallback.Dynamic repo lookup with a sane fallback matches the PR goal.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/pages/ContributorProfile/ContributorProfile.tsx (1)
96-99: Don’t cast raw GraphQL nodes; map to PR[] and handle GraphQL errors.Current cast leaves
repository_urlundefined and breaks the list (split on undefined). Map fields and surface GraphQL errors.- const prsData = await prsRes.json(); - const prs = prsData.data?.search?.nodes || []; - setPRs(prs as PR[]); + const prsData = await prsRes.json(); + if (Array.isArray(prsData.errors) && prsData.errors.length) { + throw new Error(prsData.errors.map((e: any) => e.message).join('; ')); + } + const nodes = prsData.data?.search?.nodes ?? []; + const mapped: PR[] = nodes + .filter((n: any) => n && typeof n.databaseId === 'number' && n.url && n.repository?.url) + .map((n: any) => ({ + id: n.databaseId, + title: n.title, + html_url: n.url, + repository_url: n.repository.url, + })); + setPRs(mapped);
🧹 Nitpick comments (6)
src/hooks/useGitHubData.ts (4)
79-89: Reset pagination cursors when user changes or on first page.Prevents carrying over stale cursors between users or after manual resets.
const fetchData = useCallback( async (username: string, page = 1, perPage = 10) => { const octokit = getOctokit(); if (!octokit || !username || rateLimited) return; setLoading(true); setError(''); + // reset cursors when switching user or when starting from page 1 + if (page === 1 || lastUserRef.current !== username) { + cursorsRef.current = { issue: {}, pr: {} }; + lastUserRef.current = username; + }Add alongside other refs (outside this hunk):
const lastUserRef = useRef<string>('');
4-5: Type the data shape end-to-end (avoid any[]).Match Tracker’s GitHubItem and catch shape drift at compile time.
- const [issues, setIssues] = useState([]); - const [prs, setPrs] = useState([]); + const [issues, setIssues] = useState<GitHubItem[]>([]); + const [prs, setPrs] = useState<GitHubItem[]>([]);Add (outside this hunk):
type GitHubItem = { id: number; title: string; state: string; created_at: string; pull_request?: { merged_at: string | null }; repository_url: string; html_url: string; };And cast the mapped array:
- const items = response.search.edges.map((edge: any) => { + const items: GitHubItem[] = response.search.edges.map((edge: any) => {Also applies to: 59-70
27-28: Remove unused edge.cursor to trim payload.- edges { - cursor + edges { node {
99-105: Surface GraphQL errors and widen rate-limit detection.Octokit GraphQL may return 200 with errors or throw 400 with an errors array.
- } catch (err: any) { - if (err.status === 403) { + } catch (err: any) { + const gqlErrors = err?.errors || err?.response?.data?.errors; + if (Array.isArray(gqlErrors) && gqlErrors.length) { + setError(gqlErrors.map((e: any) => e.message).join('; ')); + } else if (err.status === 403 || /rate limit/i.test(err?.message ?? '')) { setError('GitHub API rate limit exceeded. Please wait or use a token.'); setRateLimited(true); // Prevent further fetches } else { setError(err.message || 'Failed to fetch data'); }src/pages/ContributorProfile/ContributorProfile.tsx (2)
155-159: Defensive repo name parsing.Avoids surprises if URL format changes.
- {prs.map((pr) => { - const repoName = pr.repository_url.split("/").slice(-2).join("/"); + {prs.map((pr) => { + const repoPath = new URL(pr.repository_url).pathname.replace(/^\/|\/$/g, ""); + const parts = repoPath.split("/"); + const repoName = parts.slice(-2).join("/");
56-75: Optional: add pageInfo and support “Load more” later.You already sort by updated; fetching pageInfo here enables incremental pagination for PR lists.
- search(query: $query, type: ISSUE, first: 100) { + search(query: $query, type: ISSUE, first: 100) { + pageInfo { hasNextPage endCursor } nodes { ... on PullRequest {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
src/hooks/useGitHubData.ts(2 hunks)src/pages/ContributorProfile/ContributorProfile.tsx(4 hunks)src/pages/Contributors/Contributors.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/pages/Contributors/Contributors.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
src/pages/ContributorProfile/ContributorProfile.tsx (2)
src/pages/Tracker/Tracker.tsx (1)
GitHubItem(38-46)src/hooks/useGitHubAuth.ts (1)
username(4-25)
src/hooks/useGitHubData.ts (1)
src/pages/Tracker/Tracker.tsx (2)
GitHubItem(38-46)data(101-138)
🔇 Additional comments (1)
src/pages/ContributorProfile/ContributorProfile.tsx (1)
82-85: Set Content-Type for GraphQL POST (unless gh() already does).Avoids reliance on wrapper defaults and ensures proper parsing.
- const prsRes = await gh("/graphql", token, { + const prsRes = await gh("/graphql", token, { method: "POST", + headers: { "Content-Type": "application/json" }, body: JSON.stringify(gqlQuery), });If gh() injects this header, feel free to skip.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/hooks/useGitHubData.ts (1)
100-106: Handle GraphQL rate limits (v4 returns 200 + errors, not 403).Extend detection to cover GraphQL “RATE_LIMITED” errors and common messages so UX stays coherent.
- if (err.status === 403) { + if ( + err?.status === 403 || + err?.errors?.some((e: any) => e?.type === 'RATE_LIMITED') || + /rate limit/i.test(err?.message ?? '') + ) { setError('GitHub API rate limit exceeded. Please wait or use a token.'); setRateLimited(true); // Prevent further fetches } else { setError(err.message || 'Failed to fetch data'); }
♻️ Duplicate comments (1)
src/hooks/useGitHubData.ts (1)
54-55: Ensure X-GitHub-Api-Version header applies to graphql too.Confirm
getOctokit()comes from a factory that setsX-GitHub-Api-Version: 2022-11-28so GraphQL calls don’t warn.Run:
#!/bin/bash rg -n --glob '!**/node_modules/**' 'makeOctokit|new Octokit|X-GitHub-Api-Version|graphql\(' -C2
🧹 Nitpick comments (3)
src/hooks/useGitHubData.ts (3)
85-86: Avoid “stuck” rateLimited state; auto-reset or expose a reset.Once set, rateLimited blocks all future fetches for the session. Add a short TTL reset (e.g., 60s) or expose a clear function to the caller.
- setRateLimited(true); // Prevent further fetches + setRateLimited(true); // Prevent further fetches + setTimeout(() => setRateLimited(false), 60_000);
27-29: Trim unusedcursorfrom selection to reduce payload.You only use
pageInfo.endCursor. Dropedges.cursor.- edges { - cursor - node { + edges { + node {
5-7: Add strict types for items and state to catch regressions at compile time.Type states and the paginated result to match Tracker’s GitHubItem.
import { useState, useRef, useCallback } from 'react'; +type GitHubItem = { + id: number; + title: string; + state: string; + created_at: string; + repository_url: string; + html_url: string; + pull_request?: { merged_at: string | null }; +}; export const useGitHubData = (getOctokit: () => any) => { type SearchType = 'issue' | 'pr'; - const [issues, setIssues] = useState([]); - const [prs, setPrs] = useState([]); + const [issues, setIssues] = useState<GitHubItem[]>([]); + const [prs, setPrs] = useState<GitHubItem[]>([]); - const fetchPaginated = async (octokit: any, username: string, type: SearchType, page = 1, per_page = 10) => { + const fetchPaginated = async ( + octokit: any, + username: string, + type: SearchType, + page = 1, + per_page = 10 + ): Promise<{ items: GitHubItem[]; total: number }> => {Also applies to: 18-18, 60-71
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/hooks/useGitHubData.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-02T13:07:19.983Z
Learnt from: ASR1015
PR: GitMetricsLab/github_tracker#205
File: src/hooks/useGitHubData.ts:0-0
Timestamp: 2025-09-02T13:07:19.983Z
Learning: In src/hooks/useGitHubData.ts, cursor handling for GraphQL pagination uses a type-safe approach: SearchType = 'issue' | 'pr', cursorsRef as Record<SearchType, Record<number, string | null>>, and fetchPaginated caches endCursor per page per type for proper pagination state management.
Applied to files:
src/hooks/useGitHubData.ts
🧬 Code graph analysis (1)
src/hooks/useGitHubData.ts (1)
src/pages/Tracker/Tracker.tsx (2)
GitHubItem(38-46)data(101-138)
🔇 Additional comments (1)
src/hooks/useGitHubData.ts (1)
18-51: LGTM: Solid GraphQL migration and correct field mapping.Using search(type: ISSUE) with is:issue/is:pr filters, clamped page size, and mapping to Tracker’s shape looks correct.
Also applies to: 60-71
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/hooks/useGitHubData.ts (1)
82-114: Rate-limit detection won’t trigger for GraphQL errors; broaden checks.
GraphQL often throws with status 200 and error type RATE_LIMITED. Your 403 check will miss it.Apply:
- } catch (err: any) { - if (err.status === 403) { - setError('GitHub API rate limit exceeded. Please wait or use a token.'); - setRateLimited(true); // Prevent further fetches - } else { - setError(err.message || 'Failed to fetch data'); - } + } catch (err: any) { + const rateLimitedErr = + err?.status === 403 || + err?.response?.headers?.['x-ratelimit-remaining'] === '0' || + (Array.isArray(err?.errors) && + err.errors.some( + (e: any) => + e?.type === 'RATE_LIMITED' || /rate limit/i.test(e?.message ?? '') + )); + if (rateLimitedErr) { + setError('GitHub API rate limit exceeded. Please wait or use a token.'); + setRateLimited(true); + } else { + setError(err?.message ?? 'Failed to fetch data'); + }
🧹 Nitpick comments (5)
src/hooks/useGitHubData.ts (5)
13-16: Cursor cache scope: future-proof by keying with full query string.
Today you key by username + page size, which is fine. If qualifiers change later (repo/date/etc.), collisions can reappear. Safer: use queryString.Apply:
- const cursorKey = `${username}:${Math.min(100, per_page)}`; + const cursorKey = `${queryString}:${Math.min(100, per_page)}`;
18-51: Optional: include __typename to aid mapping/debug.
Adding __typename makes it trivial to tell Issue vs PullRequest without relying on field presence.Apply:
node { + __typename ... on Issue {
61-72: Optional: add explicit typing for mapped items to catch shape drift.Apply:
- const items = response.search.edges.map((edge: any) => { + type GitHubItem = { + id: number; + title: string; + state: string; + created_at: string; + repository_url: string; + html_url: string; + pull_request?: { merged_at: string | null }; + }; + const items: GitHubItem[] = response.search.edges.map((edge: any) => {
5-7: Type the state arrays for stronger guarantees.Apply:
- const [issues, setIssues] = useState([]); + const [issues, setIssues] = useState<GitHubItem[]>([]); ... - const [prs, setPrs] = useState([]); + const [prs, setPrs] = useState<GitHubItem[]>([]);Note: use the GitHubItem type from the previous comment (or centralize it in a shared types module).
82-114: Ensure getOctokit() provides a GraphQL-capable client with version header.
IfgetOctokit()returns a bare@octokit/corewithout graphql or withoutX-GitHub-Api-Version, this hook will fail or still warn. Please verifymakeOctokit(...)(with default headers) backsgetOctokit().
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/hooks/useGitHubData.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-02T13:07:19.983Z
Learnt from: ASR1015
PR: GitMetricsLab/github_tracker#205
File: src/hooks/useGitHubData.ts:0-0
Timestamp: 2025-09-02T13:07:19.983Z
Learning: In src/hooks/useGitHubData.ts, cursor handling for GraphQL pagination uses a type-safe approach: SearchType = 'issue' | 'pr', cursorsRef as Record<SearchType, Record<number, string | null>>, and fetchPaginated caches endCursor per page per type for proper pagination state management.
Applied to files:
src/hooks/useGitHubData.ts
🧬 Code graph analysis (1)
src/hooks/useGitHubData.ts (2)
src/pages/Tracker/Tracker.tsx (1)
GitHubItem(38-46)src/hooks/useGitHubAuth.ts (1)
username(4-25)
🔇 Additional comments (6)
src/hooks/useGitHubData.ts (6)
1-1: Good: correct React imports for hooks in use.
4-4: Type-safety win with SearchType.
53-58: Paging stability caveat.
sort:updated-desccan cause duplicates/skips across pages when items update between requests. Acceptable, but ensure UI tolerates minor jitter.
74-77: LGTM: cursor caching per type and scope avoids cross-user/page-size collisions.
79-79: Returning total from issueCount is correct for the filtered query.
116-125: Return shape looks good and stable for consumers.
…der, single-retry throttling
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
src/lib/octokit.ts (5)
11-11: Trim token before applying authAvoid treating whitespace-only tokens as valid.
Apply:
- auth: token || undefined, + auth: token?.trim() || undefined,
6-6: Export an Octokit instance type for downstream typingHelps consumers type variables/params without importing implementation details.
Apply:
const MyOctokit = Octokit.plugin(paginateRest, throttling); + +export type OctokitClient = InstanceType<typeof MyOctokit>;
1-4: Add types for throttling callback paramsImproves IntelliSense and guards against breaking changes.
Apply:
import { Octokit } from "@octokit/core"; import { paginateRest } from "@octokit/plugin-paginate-rest"; import { throttling } from "@octokit/plugin-throttling"; import { GITHUB_API_VERSION, GITHUB_API_BASE } from "../utils/constants"; +import type { RequestOptions } from "@octokit/types";
23-31: Type the rate-limit handler and use a default param instead of optionalCleaner than optional + nullish coalescing.
Apply:
- onRateLimit: (retryAfter, options, octokit, retryCount?: number) => { - const count = retryCount ?? 0; - if (count === 0) { + onRateLimit: ( + retryAfter: number, + options: RequestOptions, + _octokit: unknown, + retryCount = 0 + ) => { + if (retryCount === 0) { console.warn( `Rate Limit: ${options.method} ${options.url}. Retrying in ${retryAfter}s.` ); return true; // single retry } return false; },
34-39: Also type the secondary rate-limit handlerMinor DX improvement; consistent with the primary handler.
Apply:
- onSecondaryRateLimit: (_retryAfter, options) => { + onSecondaryRateLimit: (_retryAfter: number, options: RequestOptions) => { console.warn( `Secondary rate limit: ${options.method} ${options.url}.` ); return true; // single retry },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
src/hooks/useGitHubData.ts(2 hunks)src/lib/octokit.ts(1 hunks)src/pages/ContributorProfile/ContributorProfile.tsx(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/hooks/useGitHubData.ts
- src/pages/ContributorProfile/ContributorProfile.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
src/lib/octokit.ts (2)
src/utils/constants.ts (2)
GITHUB_API_BASE(3-3)GITHUB_API_VERSION(1-1)src/hooks/useGitHubAuth.ts (3)
octokit(14-14)username(9-12)username(4-25)
🔇 Additional comments (1)
src/lib/octokit.ts (1)
14-20: API versioning and headers are correctly centralizedUsing shared constants plus Accept header is correct and should silence deprecation warnings consistently.
|
@ASR1015 : Don't make big change in one commit, It is harder to review. Please split commits |
ok sir |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/pages/Contributors/Contributors.tsx (1)
66-70: Dynamic repo resolution + fallback: looks good.Nice improvement over hard-coding; matches PR intent.
🧹 Nitpick comments (5)
src/pages/Contributors/Contributors.tsx (5)
71-74: Paginate contributors; at least bump per_page to 100.Large repos will be truncated. Quick win: request 100 per page; ideal: follow Link: rel="next".
- `/repos/${fullName}/contributors?per_page=50`, + `/repos/${fullName}/contributors?per_page=100`,Example pagination helper (place near loadContributors):
async function fetchAll<T>(url: string, token: string): Promise<T[]> { const out: T[] = []; let next: string | null = url; while (next) { const res = await gh(next, token); if (res.status === 204) break; if (!res.ok) throw new Error(`GitHub ${res.status}: ${await res.text()}`); out.push(...(await res.json())); const link = res.headers.get("Link") || ""; const m = link.match(/<([^>]+)>;\s*rel="next"/); next = m ? m[1] : null; } return out; }
93-99: Don’t log AbortError on unmount.Noise in console when the component unmounts mid-request.
- } catch (err: any) { - if (isMounted) - setError( - err?.message || - "Failed to fetch contributors. Check owner/repo and token (use 'repo' scope if private)." - ); - console.error("[contributors] error", err); + } catch (err: unknown) { + if ((err as any)?.name === "AbortError") return; + if (isMounted) { + const msg = + err instanceof Error + ? err.message + : "Failed to fetch contributors. Check owner/repo and token (use 'repo' scope if private)."; + setError(msg); + } + console.error("[contributors] error", err);
123-124: Add noopener to external links.Prevents window.opener attacks.
- <a href={c.html_url} target="_blank" rel="noreferrer" className="text-blue-700 hover:underline"> + <a href={c.html_url} target="_blank" rel="noopener noreferrer" className="text-blue-700 hover:underline">
27-58: Cache resolved repo name to reduce Search API calls.Lightweight localStorage cache with TTL will cut rate-limit pressure and speed up mount.
const CACHE_KEY = "resolved_repo_full_name"; const CACHE_TTL = 24 * 60 * 60 * 1000; // 24h async function resolveRepoFullName(token: string): Promise<string | null> { const cached = localStorage.getItem(CACHE_KEY); if (cached) { try { const { v, ts } = JSON.parse(cached); if (Date.now() - ts < CACHE_TTL && typeof v === "string") return v; } catch {} } // ...existing search logic... if (match?.full_name) { localStorage.setItem(CACHE_KEY, JSON.stringify({ v: match.full_name, ts: Date.now() })); return match.full_name as string; } return null; }
18-19: Pass AbortController.signal into all ghFetch calls
ghFetch’s signature accepts a RequestInit. Wireac.signalinto eachgh(…)invocation (lines 36–39, 48–49, 71–74, 107–109) to enable request cancellation. Alternatively, drop the AbortController if you don’t intend to cancel requests.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/pages/Contributors/Contributors.tsx(1 hunks)
|
Fixed GitHub API deprecations and contributor profile issues. All checks passed, ready to merge |
✅ All checks passed This PR fully modernizes GitHub API usage, removes deprecated patterns, and ensures stability for contributors and PR fetching. |
2 participants
Summary
X-GitHub-Api-Version: 2022-11-28to all GitHub requests (stops “deprecated/unversioned” warnings).gh_pat/github_pat/token) and support optional field in UI.Testing
npm run devandnpm run previewboth OK.localStorage.setItem('gh_pat','<token>'), issues/PRs load.Notes
search/issuesendpoint. Functionality works, but API is scheduled for removal on Sep 4, 2025.Summary by CodeRabbit
New Features
Improvements
Refactor