From ea54d926ab4c7858ab02c087830b26055191fcb3 Mon Sep 17 00:00:00 2001 From: Sebastian Leiman <84030375+sebale16@users.noreply.github.com> Date: Fri, 30 Jan 2026 22:20:55 +0000 Subject: [PATCH] fix: settings page lag (#736) * feat: made a handler for github stats messages same way as the rest * fix: remove settingsPageLag through incremental rendering and efficient update of local storage * refactor: passed eslint * chore: added GitHubStats types * feat: added contributor card skeletons * refactor: pass eslint * feat: removed trickle rendering and added locking to setCachedData --------- Co-authored-by: Derek Co-authored-by: Diego Perez <52579214+doprz@users.noreply.github.com> --- src/pages/background/background.ts | 2 + .../background/handler/gitHubStatsHandler.ts | 31 +++++++ src/shared/messages/GitHubStatsMessages.ts | 12 +++ src/shared/messages/index.ts | 4 +- src/shared/types/GitHubStats.ts | 40 +++++++++ .../settings/ContributorCardSkeleton.tsx | 14 +++ src/views/components/settings/Settings.tsx | 31 ++++--- src/views/lib/getGitHubStats.ts | 85 +++++++++---------- 8 files changed, 161 insertions(+), 58 deletions(-) create mode 100644 src/pages/background/handler/gitHubStatsHandler.ts create mode 100644 src/shared/messages/GitHubStatsMessages.ts create mode 100644 src/shared/types/GitHubStats.ts create mode 100644 src/views/components/settings/ContributorCardSkeleton.tsx diff --git a/src/pages/background/background.ts b/src/pages/background/background.ts index e64d3913..f832a332 100644 --- a/src/pages/background/background.ts +++ b/src/pages/background/background.ts @@ -9,6 +9,7 @@ import onUpdate from './events/onUpdate'; import browserActionHandler from './handler/browserActionHandler'; import calendarBackgroundHandler from './handler/calendarBackgroundHandler'; import CESHandler from './handler/CESHandler'; +import gitHubStatsHandler from './handler/gitHubStatsHandler'; import tabManagementHandler from './handler/tabManagementHandler'; import userScheduleHandler from './handler/userScheduleHandler'; @@ -52,6 +53,7 @@ const messageListener = new MessageListener({ ...userScheduleHandler, ...CESHandler, ...calendarBackgroundHandler, + ...gitHubStatsHandler, }); messageListener.listen(); diff --git a/src/pages/background/handler/gitHubStatsHandler.ts b/src/pages/background/handler/gitHubStatsHandler.ts new file mode 100644 index 00000000..f6b73d5c --- /dev/null +++ b/src/pages/background/handler/gitHubStatsHandler.ts @@ -0,0 +1,31 @@ +import type GitHubStatsMessages from '@shared/messages/GitHubStatsMessages'; +import { GitHubStatsService } from '@views/lib/getGitHubStats'; +import type { MessageHandler } from 'chrome-extension-toolkit'; + +const gitHubStatsService = new GitHubStatsService(); + +/** + * Handler for GitHub stats related messages + */ +const gitHubStatsHandler: MessageHandler = { + async fetchGitHubStats({ data, sendResponse }) { + try { + const includeMergedPRs = data ?? false; + const stats = await gitHubStatsService.fetchGitHubStats({ includeMergedPRs }); + sendResponse(stats); + } catch (error) { + console.error('Error fetching GitHub stats in background:', error); + sendResponse({ + adminGitHubStats: {}, + userGitHubStats: {}, + contributors: [], + names: {}, + dataFetched: new Date(), + lastUpdated: new Date(), + isCached: false, + }); + } + }, +}; + +export default gitHubStatsHandler; diff --git a/src/shared/messages/GitHubStatsMessages.ts b/src/shared/messages/GitHubStatsMessages.ts new file mode 100644 index 00000000..d3971ade --- /dev/null +++ b/src/shared/messages/GitHubStatsMessages.ts @@ -0,0 +1,12 @@ +import type { GitHubStatsResult } from '@shared/types/GitHubStats'; + +/* eslint-disable jsdoc/require-jsdoc */ + +export default interface GitHubStatsMessages { + /** + * Fetch GitHub statistics for all contributors + * @param includeMergedPRs - Whether to include merged PR counts (optional, default: false) + * @returns GitHub stats including commits, lines added/deleted, and optionally merged PRs + */ + fetchGitHubStats: (includeMergedPRs?: boolean) => GitHubStatsResult; +} diff --git a/src/shared/messages/index.ts b/src/shared/messages/index.ts index 30a3a54e..a01af847 100644 --- a/src/shared/messages/index.ts +++ b/src/shared/messages/index.ts @@ -3,6 +3,7 @@ import { createMessenger } from 'chrome-extension-toolkit'; import type BrowserActionMessages from './BrowserActionMessages'; import type { CalendarBackgroundMessages, CalendarTabMessages } from './CalendarMessages'; import type CESMessage from './CESMessage'; +import type GitHubStatsMessages from './GitHubStatsMessages'; import type TabInfoMessages from './TabInfoMessages'; import type TabManagementMessages from './TabManagementMessages'; import type { UserScheduleMessages } from './UserScheduleMessages'; @@ -14,7 +15,8 @@ export type BACKGROUND_MESSAGES = BrowserActionMessages & TabManagementMessages & UserScheduleMessages & CESMessage & - CalendarBackgroundMessages; + CalendarBackgroundMessages & + GitHubStatsMessages; /** * This is a type with all the message definitions that can be sent TO specific tabs diff --git a/src/shared/types/GitHubStats.ts b/src/shared/types/GitHubStats.ts new file mode 100644 index 00000000..bc26463a --- /dev/null +++ b/src/shared/types/GitHubStats.ts @@ -0,0 +1,40 @@ +/* eslint-disable jsdoc/require-jsdoc */ +export type TeamMember = { + name: string; + role: string[]; + githubUsername: string; +}; + +export type GitHubStats = { + commits: number; + linesAdded: number; + linesDeleted: number; + mergedPRs?: number; +}; + +export type GitHubStatsResult = { + adminGitHubStats: Record; + userGitHubStats: Record; + contributors: string[]; + names: Record; + dataFetched: Date; + lastUpdated: Date; + isCached: boolean; +}; + +export type ContributorStats = { + total: number; + weeks: { w: number; a: number; d: number; c: number }[]; + author: { login: string }; +}; + +export type ContributorUser = { + name: string | undefined; +}; + +export type FetchResult = { + data: T; + dataFetched: Date; + lastUpdated: Date; + isCached: boolean; +}; diff --git a/src/views/components/settings/ContributorCardSkeleton.tsx b/src/views/components/settings/ContributorCardSkeleton.tsx new file mode 100644 index 00000000..bdc70913 --- /dev/null +++ b/src/views/components/settings/ContributorCardSkeleton.tsx @@ -0,0 +1,14 @@ +import React from 'react'; + +/** + * Lightweight skeleton placeholder for contributor cards while data loads + */ +export const ContributorCardSkeleton: React.FC = () => ( +
+
+
+
+
+); + +export default ContributorCardSkeleton; diff --git a/src/views/components/settings/Settings.tsx b/src/views/components/settings/Settings.tsx index daa62765..80dde962 100644 --- a/src/views/components/settings/Settings.tsx +++ b/src/views/components/settings/Settings.tsx @@ -20,7 +20,7 @@ import useChangelog from '@views/hooks/useChangelog'; import useSchedules from '@views/hooks/useSchedules'; import { GitHubStatsService, LONGHORN_DEVELOPERS_ADMINS, LONGHORN_DEVELOPERS_SWE } from '@views/lib/getGitHubStats'; // Misc -import React, { useCallback, useEffect, useMemo, useState } from 'react'; +import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react'; // Icons import IconoirGitFork from '~icons/iconoir/git-fork'; @@ -29,6 +29,7 @@ import { useMigrationDialog } from '../common/MigrationDialog'; import { AdvancedSettings } from './AdvancedSettings'; import { DEV_MODE_CLICK_TARGET, INCLUDE_MERGED_PRS, STATS_TOGGLE_KEY } from './constants'; import { ContributorCard } from './ContributorCard'; +import { ContributorCardSkeleton } from './ContributorCardSkeleton'; import DevMode from './DevMode'; import { useBirthdayCelebration } from './useBirthdayCelebration'; import { useDevMode } from './useDevMode'; @@ -62,6 +63,9 @@ export default function Settings(): JSX.Element { const [devMode, toggleDevMode] = useDevMode(DEV_MODE_CLICK_TARGET); const { showParticles, particlesInit, particlesOptions, triggerCelebration, isBirthday } = useBirthdayCelebration(); + // Stable skeleton ids to avoid using array index as keys + const skeletonIdsRef = useRef(Array.from({ length: 8 }, (_, i) => `skeleton-${i}`)); + // Initialize settings and listeners useEffect(() => { const fetchGitHubStats = async () => { @@ -361,7 +365,6 @@ export default function Settings(): JSX.Element { ))}
-

UTRP CONTRIBUTORS

@@ -376,17 +379,19 @@ export default function Settings(): JSX.Element { includeMergedPRs={INCLUDE_MERGED_PRS} /> ))} - {additionalContributors.map(username => ( - - ))} + {githubStats === null + ? skeletonIdsRef.current.slice(0, 8).map(id => ) + : additionalContributors.map(username => ( + + ))}
diff --git a/src/views/lib/getGitHubStats.ts b/src/views/lib/getGitHubStats.ts index 8934a645..0ea9cc1b 100644 --- a/src/views/lib/getGitHubStats.ts +++ b/src/views/lib/getGitHubStats.ts @@ -1,37 +1,13 @@ import { Octokit } from '@octokit/rest'; import { CacheStore } from '@shared/storage/CacheStore'; import type { CachedData } from '@shared/types/CachedData'; - -// Types -type TeamMember = { - name: string; - role: string[]; - githubUsername: string; -}; - -type GitHubStats = { - commits: number; - linesAdded: number; - linesDeleted: number; - mergedPRs?: number; -}; - -type ContributorStats = { - total: number; - weeks: { w: number; a: number; d: number; c: number }[]; - author: { login: string }; -}; - -type ContributorUser = { - name: string | undefined; -}; - -type FetchResult = { - data: T; - dataFetched: Date; - lastUpdated: Date; - isCached: boolean; -}; +import type { + ContributorStats, + ContributorUser, + FetchResult, + GitHubStats, + TeamMember, +} from '@shared/types/GitHubStats'; // Constants const CACHE_TTL = 1 * 60 * 60 * 1000; // 1 hour in milliseconds @@ -91,6 +67,8 @@ export class GitHubStatsService { private octokit: Octokit; private cache: Record>; + private storageLock: Promise = Promise.resolve(); + constructor(githubToken?: string) { this.octokit = githubToken ? new Octokit({ auth: githubToken }) : new Octokit(); this.cache = {} as Record>; @@ -114,16 +92,33 @@ export class GitHubStatsService { return null; } - private async setCachedData(key: string, data: T): Promise { - if (Object.keys(this.cache).length === 0) { - const githubCache = await CacheStore.get('github'); - if (githubCache && typeof githubCache === 'object') { - this.cache = githubCache as Record>; - } - } + private async setCachedData(key: string, data: T, persist = true): Promise { + // get the current lock + const existingLock = this.storageLock; - this.cache[key] = { data, dataFetched: Date.now() }; - await CacheStore.set('github', this.cache); + // update the lock with a new promise + this.storageLock = (async () => { + // wait for current lock to finish + await existingLock; + + // ensure cache is loaded before modifying + if (Object.keys(this.cache).length === 0) { + const githubCache = await CacheStore.get('github'); + if (githubCache && typeof githubCache === 'object') { + this.cache = githubCache as Record>; + } + } + + // update local memory + this.cache[key] = { data, dataFetched: Date.now() }; + + // only write to the physical storage API if persist is true + if (persist) { + await CacheStore.set('github', this.cache); + } + })(); + + return this.storageLock; } private async fetchWithRetry(fetchFn: () => Promise, retries: number = 3, delay: number = 5000): Promise { @@ -182,6 +177,7 @@ export class GitHubStatsService { private async fetchContributorNames(contributors: string[]): Promise> { const names: Record = {}; + await Promise.all( contributors.map(async contributor => { const cacheKey = `contributor_name_${contributor}`; @@ -198,18 +194,17 @@ export class GitHubStatsService { if (data.name) { name = data.name; } - await this.setCachedData(cacheKey, name); + // Pass 'false' to avoid writing to disk for every single name + await this.setCachedData(cacheKey, name, false); } catch (e) { console.error(e); } } - names[contributor] = name; }) ); return names; } - private async fetchMergedPRsCount(username: string): Promise> { const cacheKey = `merged_prs_${username}`; const cachedCount = await this.getCachedData(cacheKey); @@ -233,7 +228,7 @@ export class GitHubStatsService { lastUpdated: new Date(), isCached: false, }; - await this.setCachedData(cacheKey, fetchResult.data); + await this.setCachedData(cacheKey, fetchResult.data, false); return fetchResult; } @@ -300,6 +295,8 @@ export class GitHubStatsService { const names = await this.fetchContributorNames(contributors); + await CacheStore.set('github', this.cache); + return { adminGitHubStats, userGitHubStats,