Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions src/hooks/useGitHubData.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,11 +93,11 @@ export const useGitHubData = (
perPage = 10,
activeTab: 'issue' | 'pr' | 'both' = 'both',
filters: FetchFilters = {}
) => {
): Promise<boolean> => {
const octokit = getOctokit();

if (!octokit || !username.trim() || rateLimited) {
return;
return false;
}

const requestId = ++lastRequestId.current;
Expand Down Expand Up @@ -144,7 +144,7 @@ export const useGitHubData = (

// Ignore stale requests
if (requestId !== lastRequestId.current) {
return;
return false;
}

let resultIndex = 0;
Expand Down Expand Up @@ -186,9 +186,10 @@ export const useGitHubData = (
}

setRateLimited(false);
return !hasRejected;
} catch (err: unknown) {
if (requestId !== lastRequestId.current) {
return;
return false;
}

const error = err as {
Expand Down Expand Up @@ -230,6 +231,7 @@ export const useGitHubData = (
'Unable to fetch GitHub data. Please verify the username, token, or network connection.'
);
}
return false;
} finally {
if (requestId === lastRequestId.current) {
setLoading(false);
Expand Down
152 changes: 149 additions & 3 deletions src/pages/Tracker/Tracker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,16 @@ import {
MenuItem,
FormControl,
InputLabel,
Typography,
} from "@mui/material";
import { useTheme } from "@mui/material/styles";
import { useGitHubAuth } from "../../hooks/useGitHubAuth";
import { useGitHubData } from "../../hooks/useGitHubData";
import { KeyIcon } from "lucide-react";

const ROWS_PER_PAGE = 10;
const RECENT_SEARCHES_KEY = 'githubTrackerRecentSearches';
const MAX_RECENT_SEARCHES = 10;

interface GitHubItem {
id: number;
Expand Down Expand Up @@ -71,6 +74,7 @@ const Home: React.FC = () => {

const [tab, setTab] = useState(0);
const [page, setPage] = useState(0);
const [recentSearches, setRecentSearches] = useState<string[]>([]);

const [issueFilter, setIssueFilter] = useState("all");
const [prFilter, setPrFilter] = useState("all");
Expand All @@ -79,23 +83,119 @@ const Home: React.FC = () => {
const [startDate, setStartDate] = useState("");
const [endDate, setEndDate] = useState("");

// Fetch data when username, tab, or page changes
const normalizeSearchUsername = (value: string): string => value.trim();

const loadRecentSearches = (): string[] => {
if (typeof window === 'undefined') {
return [];
}

try {
const stored = localStorage.getItem(RECENT_SEARCHES_KEY);
if (!stored) {
return [];
}

const parsed = JSON.parse(stored);
if (!Array.isArray(parsed)) {
return [];
}

return parsed
.filter((item): item is string => typeof item === 'string')
.map((item) => item.trim())
.filter(Boolean)
.slice(0, MAX_RECENT_SEARCHES);
} catch {
return [];
}
};

const persistRecentSearches = (searches: string[]) => {
setRecentSearches(searches);
localStorage.setItem(RECENT_SEARCHES_KEY, JSON.stringify(searches));
};

const addRecentSearch = (value: string) => {
const normalized = normalizeSearchUsername(value);
if (!normalized) {
return;
}

const updated = [
normalized,
...recentSearches.filter(
(item) => item.toLowerCase() !== normalized.toLowerCase()
),
].slice(0, MAX_RECENT_SEARCHES);

persistRecentSearches(updated);
};

const clearRecentSearches = () => {
setRecentSearches([]);
localStorage.removeItem(RECENT_SEARCHES_KEY);
};

useEffect(() => {
setRecentSearches(loadRecentSearches());
}, []);

// Fetch data when tab or page changes.
// Do not fetch on intermediate username typing.
// eslint-disable-next-line react-hooks/exhaustive-deps
useEffect(() => {
if (username) {
fetchData(username, page + 1, ROWS_PER_PAGE);
}
}, [tab, page]);
Comment on lines 147 to 151
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
sed -n '147,197p' src/pages/Tracker/Tracker.tsx

Repository: GitMetricsLab/github_tracker

Length of output: 1165


Duplicate API calls when searching from non-zero pages

Both handleSubmit and handleRecentSearchClick call setPage(0) then immediately await fetchData(..., 1, ...). Setting page to 0 triggers the [tab, page] effect, which fetches the same page 1 data again. This wastes GitHub API quota on every search initiated from pages beyond the first.

Move all fetch logic into the effect, or add a flag to prevent the effect from running when handlers have already fetched.

Affected code
useEffect(() => {
  if (username) {
    fetchData(username, page + 1, ROWS_PER_PAGE);
  }
}, [tab, page]);

const handleSubmit = async (e: React.FormEvent<HTMLFormElement>): Promise<void> => {
  // ...
  setUsername(normalizedUsername);
  setPage(0);

  const wasSuccessful = await fetchData(
    normalizedUsername,
    1,
    ROWS_PER_PAGE
  );
  // ...
};

const handleRecentSearchClick = async (value: string) => {
  // ...
  setUsername(normalizedUsername);
  setPage(0);

  const wasSuccessful = await fetchData(
    normalizedUsername,
    1,
    ROWS_PER_PAGE
  );
  // ...
};
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/pages/Tracker/Tracker.tsx` around lines 147 - 151, The duplicate fetches
are caused by calling fetchData inside handleSubmit/handleRecentSearchClick
while also relying on the useEffect([tab, page]) to fetch; fix by moving fetch
control entirely into the effect: add username to the useEffect dependency array
(useEffect(() => { if (username) fetchData(username, page + 1, ROWS_PER_PAGE);
}, [tab, page, username])) and remove the immediate await fetchData(...) calls
from handleSubmit and handleRecentSearchClick (keep their setUsername and
setPage(0) calls so the effect drives the fetch). This consolidates fetch logic
into useEffect and prevents double requests.


const handleSubmit = (e: React.FormEvent<HTMLFormElement>): void => {
const handleSubmit = async (e: React.FormEvent<HTMLFormElement>): Promise<void> => {
e.preventDefault();

const normalizedUsername = normalizeSearchUsername(username);
if (!normalizedUsername) {
return;
}

setUsername(normalizedUsername);
setPage(0);
fetchData(username, 1, ROWS_PER_PAGE);

const wasSuccessful = await fetchData(
normalizedUsername,
1,
ROWS_PER_PAGE
);

if (wasSuccessful) {
addRecentSearch(normalizedUsername);
}
};

const handlePageChange = (_: unknown, newPage: number) => {
setPage(newPage);
};

const handleRecentSearchClick = async (value: string) => {
const normalizedUsername = normalizeSearchUsername(value);
if (!normalizedUsername) {
return;
}

setUsername(normalizedUsername);
setPage(0);

const wasSuccessful = await fetchData(
normalizedUsername,
1,
ROWS_PER_PAGE
);

if (wasSuccessful) {
addRecentSearch(normalizedUsername);
}
};

const formatDate = (dateString: string): string =>
new Date(dateString).toLocaleDateString();

Expand Down Expand Up @@ -240,6 +340,52 @@ const Home: React.FC = () => {
</Button>
</Box>
</form>

{recentSearches.length > 0 && (
<Box
sx={{
mt: 3,
display: 'flex',
flexDirection: 'column',
gap: 2,
}}
>
<Box
sx={{
display: 'flex',
justifyContent: 'space-between',
alignItems: 'center',
gap: 2,
flexWrap: 'wrap',
}}
>
<Typography variant="subtitle1" sx={{ color: theme.palette.text.primary }}>
Recent searches
</Typography>
<Button
size="small"
variant="outlined"
onClick={clearRecentSearches}
>
Clear History
</Button>
</Box>

<Box sx={{ display: 'flex', flexWrap: 'wrap', gap: 1 }}>
{recentSearches.map((item) => (
<Button
key={item}
variant="outlined"
size="small"
onClick={() => handleRecentSearchClick(item)}
sx={{ textTransform: 'none' }}
>
{item}
</Button>
))}
</Box>
</Box>
)}
</Paper>

{/* Filters */}
Expand Down
Loading