From 7a7e46fbf6a62e965142670bd11109a783b28729 Mon Sep 17 00:00:00 2001 From: Jason Penilla <11360596+jpenilla@users.noreply.github.com> Date: Tue, 29 Jul 2025 11:28:49 -0700 Subject: [PATCH 1/6] Add loading progress --- web/src/lib/diff-viewer-multi-file.svelte.ts | 48 ++++++++++++++++---- web/src/lib/github.svelte.ts | 6 +-- web/src/lib/util.ts | 4 +- web/src/routes/+page.svelte | 4 +- web/src/routes/OpenDiffDialog.svelte | 2 +- 5 files changed, 49 insertions(+), 15 deletions(-) diff --git a/web/src/lib/diff-viewer-multi-file.svelte.ts b/web/src/lib/diff-viewer-multi-file.svelte.ts index 524dfa9..62bed92 100644 --- a/web/src/lib/diff-viewer-multi-file.svelte.ts +++ b/web/src/lib/diff-viewer-multi-file.svelte.ts @@ -24,7 +24,7 @@ import { countOccurrences, type FileTreeNodeData, makeFileTree, type LazyPromise import { onDestroy, tick } from "svelte"; import { type TreeNode, TreeState } from "$lib/components/tree/index.svelte"; import { VList } from "virtua/svelte"; -import { Context, Debounced } from "runed"; +import { Context, Debounced, watch } from "runed"; import { MediaQuery } from "svelte/reactivity"; import { ProgressBarState } from "$lib/components/progress-bar/index.svelte"; @@ -338,8 +338,7 @@ export class MultiFileDiffViewerState { activeSearchResult: ActiveSearchResult | null = $state(null); sidebarCollapsed = $state(false); diffMetadata: DiffMetadata | null = $state(null); - loading: boolean = $state(false); - readonly progressBar = $state(new ProgressBarState(null, 100)); + readonly loadingState: LoadingState = $state(new LoadingState()); readonly fileTreeFilterDebounced = new Debounced(() => this.fileTreeFilter, 500); readonly searchQueryDebounced = new Debounced(() => this.searchQuery, 500); @@ -479,13 +478,12 @@ export class MultiFileDiffViewerState { } async loadPatches(meta: () => Promise, patches: () => Promise>) { - if (this.loading) { + if (this.loadingState.loading) { alert("Already loading patches, please wait."); return false; } try { - this.progressBar.setSpinning(); - this.loading = true; + this.loadingState.start(); await tick(); await animationFramePromise(); @@ -501,8 +499,14 @@ export class MultiFileDiffViewerState { const tempDetails: FileDetails[] = []; for await (const details of generator) { + this.loadingState.loadedCount++; + // Pushing directly to the main array causes too many signals to update (lag) tempDetails.push(details); + + // TODO this makes it load one patch per frame + await tick(); + await animationFramePromise(); } if (tempDetails.length === 0) { throw new Error("No valid patches found in the provided data."); @@ -516,7 +520,7 @@ export class MultiFileDiffViewerState { alert("Failed to load patches: " + e); return false; } finally { - this.loading = false; + this.loadingState.done(); } } @@ -527,7 +531,7 @@ export class MultiFileDiffViewerState { }, async () => { const result = await resultPromise; - return parseMultiFilePatchGithub(result.info, await result.response); + return parseMultiFilePatchGithub(result.info, await result.response, this.loadingState); }, ); } @@ -661,6 +665,34 @@ export class MultiFileDiffViewerState { } } +export class LoadingState { + loading: boolean = $state(false); + loadedCount: number = $state(0); + totalCount: number | null = $state(0); + readonly progressBar = $state(new ProgressBarState(null, 100)); + + constructor() { + watch([() => this.loadedCount, () => this.totalCount], ([loadedCount, totalCount]) => { + if (totalCount === null || totalCount <= 0) { + this.progressBar.setSpinning(); + } else { + this.progressBar.setProgress(loadedCount, totalCount); + } + }); + } + + start() { + this.loadedCount = 0; + this.totalCount = null; + this.progressBar.setSpinning(); + this.loading = true; + } + + done() { + this.loading = false; + } +} + export type ActiveSearchResult = { file: FileDetails; idx: number; diff --git a/web/src/lib/github.svelte.ts b/web/src/lib/github.svelte.ts index 7c3a072..bce63cc 100644 --- a/web/src/lib/github.svelte.ts +++ b/web/src/lib/github.svelte.ts @@ -1,7 +1,7 @@ import { browser } from "$app/environment"; import type { components } from "@octokit/openapi-types"; import { parseMultiFilePatch, trimCommitHash } from "$lib/util"; -import { makeImageDetails } from "$lib/diff-viewer-multi-file.svelte"; +import { LoadingState, makeImageDetails } from "$lib/diff-viewer-multi-file.svelte"; import { PUBLIC_GITHUB_APP_NAME, PUBLIC_GITHUB_CLIENT_ID } from "$env/static/public"; export const GITHUB_USERNAME_KEY = "github_username"; @@ -139,8 +139,8 @@ export async function fetchGithubPRInfo(token: string | null, owner: string, rep } } -export function parseMultiFilePatchGithub(details: GithubDiff, patch: string) { - return parseMultiFilePatch(patch, (from, to, status) => { +export function parseMultiFilePatchGithub(details: GithubDiff, patch: string, loadingState: LoadingState) { + return parseMultiFilePatch(patch, loadingState, (from, to, status) => { const token = getGithubToken(); return makeImageDetails( from, diff --git a/web/src/lib/util.ts b/web/src/lib/util.ts index c86b414..1fcabd3 100644 --- a/web/src/lib/util.ts +++ b/web/src/lib/util.ts @@ -1,4 +1,4 @@ -import { type FileDetails, type ImageFileDetails, makeTextDetails } from "./diff-viewer-multi-file.svelte"; +import { type FileDetails, type ImageFileDetails, LoadingState, makeTextDetails } from "./diff-viewer-multi-file.svelte"; import type { FileStatus } from "./github.svelte"; import type { TreeNode } from "$lib/components/tree/index.svelte"; import type { BundledLanguage, SpecialLanguage } from "shiki"; @@ -146,9 +146,11 @@ function parseHeader(patch: string, fromFile: string, toFile: string): BasicHead export function parseMultiFilePatch( patchContent: string, + loadingState: LoadingState, imageFactory?: (fromFile: string, toFile: string, status: FileStatus) => ImageFileDetails | null, ): AsyncGenerator { const split = splitMultiFilePatch(patchContent); + loadingState.totalCount = split.length; async function* detailsGenerator() { for (const [header, content] of split) { if (header.binary) { diff --git a/web/src/routes/+page.svelte b/web/src/routes/+page.svelte index a2be111..8e4e9fc 100644 --- a/web/src/routes/+page.svelte +++ b/web/src/routes/+page.svelte @@ -138,9 +138,9 @@ {/snippet} -{#if viewer.loading} +{#if viewer.loadingState.loading}
- +
{/if} diff --git a/web/src/routes/OpenDiffDialog.svelte b/web/src/routes/OpenDiffDialog.svelte index 9f45c2f..4437535 100644 --- a/web/src/routes/OpenDiffDialog.svelte +++ b/web/src/routes/OpenDiffDialog.svelte @@ -282,7 +282,7 @@ return { type: "file", fileName: meta.name }; }, async () => { - return parseMultiFilePatch(text); + return parseMultiFilePatch(text, viewer.loadingState); }, ); if (!success) { From 850116767e3efde08b2ce788fabf46c6c912ae82 Mon Sep 17 00:00:00 2001 From: Jason Penilla <11360596+jpenilla@users.noreply.github.com> Date: Tue, 29 Jul 2025 11:41:27 -0700 Subject: [PATCH 2/6] Only await frame every 50ms --- web/src/lib/diff-viewer-multi-file.svelte.ts | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/web/src/lib/diff-viewer-multi-file.svelte.ts b/web/src/lib/diff-viewer-multi-file.svelte.ts index 62bed92..9116df2 100644 --- a/web/src/lib/diff-viewer-multi-file.svelte.ts +++ b/web/src/lib/diff-viewer-multi-file.svelte.ts @@ -483,30 +483,40 @@ export class MultiFileDiffViewerState { return false; } try { + // Show progress bar this.loadingState.start(); await tick(); await animationFramePromise(); + // Update metadata this.diffMetadata = await meta(); await tick(); await animationFramePromise(); + // Clear previous state this.clear(false); await tick(); await animationFramePromise(); + // Setup generator const generator = await patches(); + await tick(); + await animationFramePromise(); + // Load patches const tempDetails: FileDetails[] = []; + let lastYield = Date.now(); for await (const details of generator) { this.loadingState.loadedCount++; // Pushing directly to the main array causes too many signals to update (lag) tempDetails.push(details); - // TODO this makes it load one patch per frame - await tick(); - await animationFramePromise(); + if (Date.now() - lastYield > 50) { + await tick(); + await animationFramePromise(); + lastYield = Date.now(); + } } if (tempDetails.length === 0) { throw new Error("No valid patches found in the provided data."); @@ -520,6 +530,10 @@ export class MultiFileDiffViewerState { alert("Failed to load patches: " + e); return false; } finally { + // Let the last progress update render before closing the loading state + await tick(); + await animationFramePromise(); + this.loadingState.done(); } } From b7f85646a7b0d6b628bfa8a61a2eab76f4d92ac2 Mon Sep 17 00:00:00 2001 From: Jason Penilla <11360596+jpenilla@users.noreply.github.com> Date: Tue, 29 Jul 2025 11:47:42 -0700 Subject: [PATCH 3/6] date.now -> performance.now --- web/src/lib/diff-viewer-multi-file.svelte.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/web/src/lib/diff-viewer-multi-file.svelte.ts b/web/src/lib/diff-viewer-multi-file.svelte.ts index 9116df2..1859e41 100644 --- a/web/src/lib/diff-viewer-multi-file.svelte.ts +++ b/web/src/lib/diff-viewer-multi-file.svelte.ts @@ -505,17 +505,18 @@ export class MultiFileDiffViewerState { // Load patches const tempDetails: FileDetails[] = []; - let lastYield = Date.now(); + let lastYield = performance.now(); for await (const details of generator) { this.loadingState.loadedCount++; // Pushing directly to the main array causes too many signals to update (lag) tempDetails.push(details); - if (Date.now() - lastYield > 50) { + const now = performance.now(); + if (now - lastYield > 50) { await tick(); await animationFramePromise(); - lastYield = Date.now(); + lastYield = now; } } if (tempDetails.length === 0) { From 7f48a3858374f0ce141d12073bb6f51b8f0d686a Mon Sep 17 00:00:00 2001 From: Jason Penilla <11360596+jpenilla@users.noreply.github.com> Date: Tue, 29 Jul 2025 12:27:07 -0700 Subject: [PATCH 4/6] also yield every 100 patches --- .../components/progress-bar/ProgressBar.svelte | 2 +- web/src/lib/diff-viewer-multi-file.svelte.ts | 17 ++++++++++++++--- web/src/lib/util.ts | 4 ++++ 3 files changed, 19 insertions(+), 4 deletions(-) diff --git a/web/src/lib/components/progress-bar/ProgressBar.svelte b/web/src/lib/components/progress-bar/ProgressBar.svelte index ec945e9..56f9bfc 100644 --- a/web/src/lib/components/progress-bar/ProgressBar.svelte +++ b/web/src/lib/components/progress-bar/ProgressBar.svelte @@ -20,7 +20,7 @@ {@const percent = state.getPercent()} {#if percent !== undefined}
{:else} diff --git a/web/src/lib/diff-viewer-multi-file.svelte.ts b/web/src/lib/diff-viewer-multi-file.svelte.ts index 1859e41..8ee06e1 100644 --- a/web/src/lib/diff-viewer-multi-file.svelte.ts +++ b/web/src/lib/diff-viewer-multi-file.svelte.ts @@ -20,7 +20,16 @@ import { import type { BundledTheme } from "shiki"; import { browser } from "$app/environment"; import { getEffectiveGlobalTheme } from "$lib/theme.svelte"; -import { countOccurrences, type FileTreeNodeData, makeFileTree, type LazyPromise, lazyPromise, watchLocalStorage, animationFramePromise } from "$lib/util"; +import { + countOccurrences, + type FileTreeNodeData, + makeFileTree, + type LazyPromise, + lazyPromise, + watchLocalStorage, + animationFramePromise, + yieldToBrowser, +} from "$lib/util"; import { onDestroy, tick } from "svelte"; import { type TreeNode, TreeState } from "$lib/components/tree/index.svelte"; import { VList } from "virtua/svelte"; @@ -506,16 +515,18 @@ export class MultiFileDiffViewerState { // Load patches const tempDetails: FileDetails[] = []; let lastYield = performance.now(); + let i = 0; for await (const details of generator) { + i++; this.loadingState.loadedCount++; // Pushing directly to the main array causes too many signals to update (lag) tempDetails.push(details); const now = performance.now(); - if (now - lastYield > 50) { + if (now - lastYield > 50 || i % 100 === 0) { await tick(); - await animationFramePromise(); + await yieldToBrowser(); lastYield = now; } } diff --git a/web/src/lib/util.ts b/web/src/lib/util.ts index 1fcabd3..b5218e0 100644 --- a/web/src/lib/util.ts +++ b/web/src/lib/util.ts @@ -471,6 +471,10 @@ export function animationFramePromise() { }); } +export async function yieldToBrowser() { + await new Promise((resolve) => setTimeout(resolve, 0)); +} + // from bits-ui internals export type ReadableBoxedValues = { [K in keyof T]: ReadableBox; From 820810e75410cc2ab5d0ec43b8e049cbdeaf8314 Mon Sep 17 00:00:00 2001 From: Jason Penilla <11360596+jpenilla@users.noreply.github.com> Date: Tue, 29 Jul 2025 12:54:16 -0700 Subject: [PATCH 5/6] Add progress for dir comparison --- web/src/lib/diff-viewer-multi-file.svelte.ts | 5 ++--- web/src/routes/OpenDiffDialog.svelte | 4 ++++ 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/web/src/lib/diff-viewer-multi-file.svelte.ts b/web/src/lib/diff-viewer-multi-file.svelte.ts index 8ee06e1..df375ae 100644 --- a/web/src/lib/diff-viewer-multi-file.svelte.ts +++ b/web/src/lib/diff-viewer-multi-file.svelte.ts @@ -523,11 +523,10 @@ export class MultiFileDiffViewerState { // Pushing directly to the main array causes too many signals to update (lag) tempDetails.push(details); - const now = performance.now(); - if (now - lastYield > 50 || i % 100 === 0) { + if (performance.now() - lastYield > 50 || i % 100 === 0) { await tick(); await yieldToBrowser(); - lastYield = now; + lastYield = performance.now(); } } if (tempDetails.length === 0) { diff --git a/web/src/routes/OpenDiffDialog.svelte b/web/src/routes/OpenDiffDialog.svelte index 4437535..800bcdd 100644 --- a/web/src/routes/OpenDiffDialog.svelte +++ b/web/src/routes/OpenDiffDialog.svelte @@ -181,6 +181,8 @@ const entriesB: ProtoFileDetails[] = flatten(dirB).filter(blacklist); const entriesBMap = new Map(entriesB.map((entry) => [entry.path, entry])); + viewer.loadingState.totalCount = new Set([...entriesAMap.keys(), ...entriesBMap.keys()]).size; + for (const entry of entriesA) { const entryB = entriesBMap.get(entry.path); if (entryB) { @@ -190,6 +192,7 @@ if (aBinary || bBinary) { if (await bytesEqual(entry.file, entryB.file)) { // Files are identical + viewer.loadingState.loadedCount++; continue; } if (isImageFile(entry.file.name) && isImageFile(entryB.file.name)) { @@ -201,6 +204,7 @@ const [textA, textB] = await Promise.all([entry.file.text(), entryB.file.text()]); if (textA === textB) { // Files are identical + viewer.loadingState.loadedCount++; continue; } yield makeTextDetails(entry.path, entryB.path, "modified", createTwoFilesPatch(entry.path, entryB.path, textA, textB)); From 15c0d43ef7a5eea048a4607643b329ee6b4a6120 Mon Sep 17 00:00:00 2001 From: Jason Penilla <11360596+jpenilla@users.noreply.github.com> Date: Thu, 31 Jul 2025 13:58:53 -0700 Subject: [PATCH 6/6] Allow commit meta request to run in parallel with data request --- web/src/lib/diff-viewer-multi-file.svelte.ts | 17 ++- web/src/lib/github.svelte.ts | 113 ++++++++++--------- 2 files changed, 71 insertions(+), 59 deletions(-) diff --git a/web/src/lib/diff-viewer-multi-file.svelte.ts b/web/src/lib/diff-viewer-multi-file.svelte.ts index df375ae..f23602e 100644 --- a/web/src/lib/diff-viewer-multi-file.svelte.ts +++ b/web/src/lib/diff-viewer-multi-file.svelte.ts @@ -497,8 +497,12 @@ export class MultiFileDiffViewerState { await tick(); await animationFramePromise(); + // Start potential multiple web requests in parallel + const metaPromise = meta(); + const generatorPromise = patches(); + // Update metadata - this.diffMetadata = await meta(); + this.diffMetadata = await metaPromise; await tick(); await animationFramePromise(); @@ -508,7 +512,7 @@ export class MultiFileDiffViewerState { await animationFramePromise(); // Setup generator - const generator = await patches(); + const generator = await generatorPromise; await tick(); await animationFramePromise(); @@ -549,14 +553,15 @@ export class MultiFileDiffViewerState { } } - private async loadPatchesGithub(resultPromise: Promise) { + private async loadPatchesGithub(resultOrPromise: Promise | GithubDiffResult) { return await this.loadPatches( async () => { - return { type: "github", details: (await resultPromise).info }; + const result = resultOrPromise instanceof Promise ? await resultOrPromise : resultOrPromise; + return { type: "github", details: await result.info }; }, async () => { - const result = await resultPromise; - return parseMultiFilePatchGithub(result.info, await result.response, this.loadingState); + const result = resultOrPromise instanceof Promise ? await resultOrPromise : resultOrPromise; + return parseMultiFilePatchGithub(await result.info, await result.response, this.loadingState); }, ); } diff --git a/web/src/lib/github.svelte.ts b/web/src/lib/github.svelte.ts index bce63cc..980c06e 100644 --- a/web/src/lib/github.svelte.ts +++ b/web/src/lib/github.svelte.ts @@ -21,7 +21,7 @@ export type GithubDiff = { }; export type GithubDiffResult = { - info: GithubDiff; + info: Promise; response: Promise; }; @@ -112,7 +112,7 @@ export async function fetchGithubPRComparison(token: string | null, owner: strin const base = prInfo.base.sha; const head = prInfo.head.sha; const title = `${prInfo.title} (#${prInfo.number})`; - return await fetchGithubComparison(token, owner, repo, base, head, title, prInfo.html_url); + return fetchGithubComparison(token, owner, repo, base, head, title, prInfo.html_url); } function injectOptionalToken(token: string | null, opts: RequestInit) { @@ -124,7 +124,7 @@ function injectOptionalToken(token: string | null, opts: RequestInit) { } } -export async function fetchGithubPRInfo(token: string | null, owner: string, repo: string, prNumber: string): Promise { +async function fetchGithubPRInfo(token: string | null, owner: string, repo: string, prNumber: string): Promise { const opts: RequestInit = { headers: { Accept: "application/json", @@ -152,7 +152,7 @@ export function parseMultiFilePatchGithub(details: GithubDiff, patch: string, lo }); } -export async function fetchGithubComparison( +export function fetchGithubComparison( token: string | null, owner: string, repo: string, @@ -160,59 +160,66 @@ export async function fetchGithubComparison( head: string, description?: string, url?: string, -): Promise { - const opts: RequestInit = { - headers: { - Accept: "application/vnd.github.v3.diff", - }, +): GithubDiffResult { + return { + info: (async () => { + if (!url) { + url = `https://github.com/${owner}/${repo}/compare/${base}...${head}`; + } + if (!description) { + description = `Comparing ${trimCommitHash(base)}...${trimCommitHash(head)}`; + } + return { owner, repo, base, head, description, backlink: url }; + })(), + response: (async () => { + const opts: RequestInit = { + headers: { + Accept: "application/vnd.github.v3.diff", + }, + }; + injectOptionalToken(token, opts); + const response = await fetch(`https://api.github.com/repos/${owner}/${repo}/compare/${base}...${head}`, opts); + if (!response.ok) { + throw Error(`Failed to retrieve comparison (${response.status}): ${await response.text()}`); + } + return await response.text(); + })(), }; - injectOptionalToken(token, opts); - const response = await fetch(`https://api.github.com/repos/${owner}/${repo}/compare/${base}...${head}`, opts); - if (response.ok) { - if (!description) { - description = `Comparing ${trimCommitHash(base)}...${trimCommitHash(head)}`; - } - if (!url) { - url = `https://github.com/${owner}/${repo}/compare/${base}...${head}`; - } - const info = { owner, repo, base, head, description, backlink: url }; - return { response: response.text(), info }; - } else { - throw Error(`Failed to retrieve comparison (${response.status}): ${await response.text()}`); - } } -export async function fetchGithubCommitDiff(token: string | null, owner: string, repo: string, commit: string): Promise { - const diffOpts: RequestInit = { - headers: { - Accept: "application/vnd.github.v3.diff", - }, - }; - injectOptionalToken(token, diffOpts); +export function fetchGithubCommitDiff(token: string | null, owner: string, repo: string, commit: string): GithubDiffResult { const url = `https://api.github.com/repos/${owner}/${repo}/commits/${commit}`; - const response = await fetch(url, diffOpts); - if (response.ok) { - const metaOpts: RequestInit = { - headers: { - Accept: "application/vnd.github+json", - }, - }; - injectOptionalToken(token, metaOpts); - const metaResponse = await fetch(url, metaOpts); - if (!metaResponse.ok) { - throw Error(`Failed to retrieve commit meta (${metaResponse.status}): ${await metaResponse.text()}`); - } - const meta: GithubCommitDetails = await metaResponse.json(); - const firstParent = meta.parents[0].sha; - const description = `${meta.commit.message.split("\n")[0]} (${trimCommitHash(commit)})`; - const info = { owner, repo, base: firstParent, head: commit, description, backlink: meta.html_url }; - return { - response: response.text(), - info, - }; - } else { - throw Error(`Failed to retrieve commit diff (${response.status}): ${await response.text()}`); - } + return { + info: (async () => { + const metaOpts: RequestInit = { + headers: { + Accept: "application/vnd.github+json", + }, + }; + injectOptionalToken(token, metaOpts); + const metaResponse = await fetch(url, metaOpts); + if (!metaResponse.ok) { + throw Error(`Failed to retrieve commit meta (${metaResponse.status}): ${await metaResponse.text()}`); + } + const meta: GithubCommitDetails = await metaResponse.json(); + const firstParent = meta.parents[0].sha; + const description = `${meta.commit.message.split("\n")[0]} (${trimCommitHash(commit)})`; + return { owner, repo, base: firstParent, head: commit, description, backlink: meta.html_url }; + })(), + response: (async () => { + const diffOpts: RequestInit = { + headers: { + Accept: "application/vnd.github.v3.diff", + }, + }; + injectOptionalToken(token, diffOpts); + const response = await fetch(url, diffOpts); + if (!response.ok) { + throw Error(`Failed to retrieve commit diff (${response.status}): ${await response.text()}`); + } + return await response.text(); + })(), + }; } export async function fetchGithubFile(token: string | null, owner: string, repo: string, path: string, ref: string): Promise {