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 <derex1987@gmail.com> Co-authored-by: Diego Perez <52579214+doprz@users.noreply.github.com>
This commit is contained in:
@@ -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<BACKGROUND_MESSAGES>({
|
||||
...userScheduleHandler,
|
||||
...CESHandler,
|
||||
...calendarBackgroundHandler,
|
||||
...gitHubStatsHandler,
|
||||
});
|
||||
|
||||
messageListener.listen();
|
||||
|
||||
31
src/pages/background/handler/gitHubStatsHandler.ts
Normal file
31
src/pages/background/handler/gitHubStatsHandler.ts
Normal file
@@ -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<GitHubStatsMessages> = {
|
||||
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;
|
||||
12
src/shared/messages/GitHubStatsMessages.ts
Normal file
12
src/shared/messages/GitHubStatsMessages.ts
Normal file
@@ -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;
|
||||
}
|
||||
@@ -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
|
||||
|
||||
40
src/shared/types/GitHubStats.ts
Normal file
40
src/shared/types/GitHubStats.ts
Normal file
@@ -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<string, GitHubStats>;
|
||||
userGitHubStats: Record<string, GitHubStats>;
|
||||
contributors: string[];
|
||||
names: Record<string, string>;
|
||||
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<T> = {
|
||||
data: T;
|
||||
dataFetched: Date;
|
||||
lastUpdated: Date;
|
||||
isCached: boolean;
|
||||
};
|
||||
14
src/views/components/settings/ContributorCardSkeleton.tsx
Normal file
14
src/views/components/settings/ContributorCardSkeleton.tsx
Normal file
@@ -0,0 +1,14 @@
|
||||
import React from 'react';
|
||||
|
||||
/**
|
||||
* Lightweight skeleton placeholder for contributor cards while data loads
|
||||
*/
|
||||
export const ContributorCardSkeleton: React.FC = () => (
|
||||
<div className='border border-gray-300 rounded bg-ut-gray/10 p-4 animate-pulse'>
|
||||
<div className='h-4 w-3/4 bg-gray-300 rounded mb-2' />
|
||||
<div className='h-3 w-1/2 bg-gray-300 rounded mb-1' />
|
||||
<div className='h-3 w-1/4 bg-gray-300 rounded' />
|
||||
</div>
|
||||
);
|
||||
|
||||
export default ContributorCardSkeleton;
|
||||
@@ -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<string[]>(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 {
|
||||
))}
|
||||
</div>
|
||||
</section>
|
||||
|
||||
<section className='my-8'>
|
||||
<h2 className='mb-4 text-xl text-ut-black font-semibold'>UTRP CONTRIBUTORS</h2>
|
||||
<div className='grid grid-cols-2 gap-4 2xl:grid-cols-4 md:grid-cols-3 xl:grid-cols-3'>
|
||||
@@ -376,7 +379,9 @@ export default function Settings(): JSX.Element {
|
||||
includeMergedPRs={INCLUDE_MERGED_PRS}
|
||||
/>
|
||||
))}
|
||||
{additionalContributors.map(username => (
|
||||
{githubStats === null
|
||||
? skeletonIdsRef.current.slice(0, 8).map(id => <ContributorCardSkeleton key={id} />)
|
||||
: additionalContributors.map(username => (
|
||||
<ContributorCard
|
||||
key={username}
|
||||
name={githubStats!.names[username] || username}
|
||||
|
||||
@@ -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<T> = {
|
||||
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<string, CachedData<unknown>>;
|
||||
|
||||
private storageLock: Promise<void> = Promise.resolve();
|
||||
|
||||
constructor(githubToken?: string) {
|
||||
this.octokit = githubToken ? new Octokit({ auth: githubToken }) : new Octokit();
|
||||
this.cache = {} as Record<string, CachedData<unknown>>;
|
||||
@@ -114,7 +92,16 @@ export class GitHubStatsService {
|
||||
return null;
|
||||
}
|
||||
|
||||
private async setCachedData<T>(key: string, data: T): Promise<void> {
|
||||
private async setCachedData<T>(key: string, data: T, persist = true): Promise<void> {
|
||||
// get the current lock
|
||||
const existingLock = this.storageLock;
|
||||
|
||||
// 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') {
|
||||
@@ -122,9 +109,17 @@ export class GitHubStatsService {
|
||||
}
|
||||
}
|
||||
|
||||
// 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<T>(fetchFn: () => Promise<T>, retries: number = 3, delay: number = 5000): Promise<T> {
|
||||
try {
|
||||
@@ -182,6 +177,7 @@ export class GitHubStatsService {
|
||||
|
||||
private async fetchContributorNames(contributors: string[]): Promise<Record<string, string>> {
|
||||
const names: Record<string, string> = {};
|
||||
|
||||
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<FetchResult<number>> {
|
||||
const cacheKey = `merged_prs_${username}`;
|
||||
const cachedCount = await this.getCachedData<number>(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,
|
||||
|
||||
Reference in New Issue
Block a user