-
Notifications
You must be signed in to change notification settings - Fork 960
perf(host-service): listBranches single git spawn instead of 4×N #4160
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
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
91 changes: 41 additions & 50 deletions
91
apps/desktop/src/renderer/hooks/host-service/useDiffStats/useDiffStats.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,62 +1,53 @@ | ||
| import { useCallback, useEffect, useState } from "react"; | ||
| import { getHostServiceClientByUrl } from "renderer/lib/host-service-client"; | ||
| import { workspaceTrpc } from "@superset/workspace-client"; | ||
| import { useCallback, useMemo } from "react"; | ||
| import { useWorkspaceEvent } from "../useWorkspaceEvent"; | ||
| import { useWorkspaceHostUrl } from "../useWorkspaceHostUrl"; | ||
|
|
||
| export interface DiffStats { | ||
| additions: number; | ||
| deletions: number; | ||
| } | ||
|
|
||
| /** | ||
| * Fetches diff stats for a single workspace, auto-updates on git changes. | ||
| * Just pass the workspaceId — host resolution is handled internally. | ||
| * Diff stats for a single workspace, derived from the shared `git.getStatus` | ||
| * query cache. Subscribes to `git:changed` and invalidates the query — React | ||
| * Query collapses concurrent invalidations from sibling consumers (e.g. | ||
| * `useGitStatus`, multiple sidebar tiles) into a single refetch. | ||
| */ | ||
| export function useDiffStats(workspaceId: string): DiffStats | null { | ||
| const [stats, setStats] = useState<DiffStats | null>(null); | ||
| const hostUrl = useWorkspaceHostUrl(workspaceId); | ||
|
|
||
| const fetchStats = useCallback(async () => { | ||
| if (!hostUrl) return; | ||
| try { | ||
| const client = getHostServiceClientByUrl(hostUrl); | ||
| const status = await client.git.getStatus.query({ workspaceId }); | ||
|
|
||
| // Deduplicate by path — a file can appear in multiple categories | ||
| const byPath = new Map< | ||
| string, | ||
| { additions: number; deletions: number } | ||
| >(); | ||
| for (const file of status.againstBase) { | ||
| byPath.set(file.path, file); | ||
| } | ||
| for (const file of status.staged) { | ||
| byPath.set(file.path, file); | ||
| } | ||
| for (const file of status.unstaged) { | ||
| byPath.set(file.path, file); | ||
| } | ||
|
|
||
| let additions = 0; | ||
| let deletions = 0; | ||
| for (const file of byPath.values()) { | ||
| additions += file.additions; | ||
| deletions += file.deletions; | ||
| } | ||
|
|
||
| setStats({ additions, deletions }); | ||
| } catch { | ||
| // Host unavailable or workspace deleted | ||
| const utils = workspaceTrpc.useUtils(); | ||
| const { data: status } = workspaceTrpc.git.getStatus.useQuery( | ||
| { workspaceId }, | ||
| { | ||
| enabled: Boolean(workspaceId), | ||
| // Match the pre-RQ behavior: only update on `git:changed`, never | ||
| // on focus. Multiple sidebar tiles each have their own query key, | ||
| // so focus refetch would re-fan out the very work this hook is | ||
| // supposed to consolidate. | ||
| refetchOnWindowFocus: false, | ||
| }, | ||
| ); | ||
|
|
||
| const invalidate = useCallback(() => { | ||
| void utils.git.getStatus.invalidate({ workspaceId }); | ||
| }, [utils, workspaceId]); | ||
|
|
||
| useWorkspaceEvent("git:changed", workspaceId, invalidate); | ||
|
|
||
| return useMemo<DiffStats | null>(() => { | ||
| if (!status) return null; | ||
|
|
||
| // Deduplicate by path — a file can appear in multiple categories. | ||
| const byPath = new Map<string, { additions: number; deletions: number }>(); | ||
| for (const file of status.againstBase) byPath.set(file.path, file); | ||
| for (const file of status.staged) byPath.set(file.path, file); | ||
| for (const file of status.unstaged) byPath.set(file.path, file); | ||
|
|
||
| let additions = 0; | ||
| let deletions = 0; | ||
| for (const file of byPath.values()) { | ||
| additions += file.additions; | ||
| deletions += file.deletions; | ||
| } | ||
| }, [hostUrl, workspaceId]); | ||
|
|
||
| useEffect(() => { | ||
| void fetchStats(); | ||
| }, [fetchStats]); | ||
|
|
||
| useWorkspaceEvent("git:changed", workspaceId, () => { | ||
| void fetchStats(); | ||
| }); | ||
|
|
||
| return stats; | ||
| return { additions, deletions }; | ||
| }, [status]); | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
refetchOnWindowFocusbehavior changeThe old implementation only fetched on mount and on
git:changedevents — there was no window-focus refetch.workspaceTrpc.git.getStatus.useQuery()without an explicitrefetchOnWindowFocus: falseinherits React Query's default oftrue, so every time the user switches back to the app, each visible workspace tile issues its owngetStatuscall.getStatusspawns several git processes (status, numstat, diff, ls-files), so on window focus with N workspace tiles open the host-service still sees N concurrentgetStatusspawns — exactly the multi-consumer refetch pattern the PR set out to fix. Passing{ refetchOnWindowFocus: false }alongside{ enabled: Boolean(workspaceId) }restores the previous event-only update semantics while keeping the shared-cache benefit.Prompt To Fix With AI