Skip to content

Commit 82632bb

Browse files
jpenillajpenilla
authored
Add loading progress (#25)
* Add loading progress * Only await frame every 50ms * date.now -> performance.now * also yield every 100 patches * Add progress for dir comparison * Allow commit meta request to run in parallel with data request
1 parent 376c577 commit 82632bb

File tree

6 files changed

+154
-75
lines changed

6 files changed

+154
-75
lines changed

‎web/src/lib/components/progress-bar/ProgressBar.svelte

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
{@const percent = state.getPercent()}
2121
{#if percent !== undefined}
2222
<div
23-
class="h-full w-full rounded-full bg-primary drop-shadow-sm drop-shadow-primary/50 transition-all duration-250 ease-in-out"
23+
class="h-full w-full rounded-full bg-primary drop-shadow-sm drop-shadow-primary/50 transition-all duration-50 ease-in-out will-change-transform"
2424
style={`transform: translateX(-${100 - percent}%)`}
2525
></div>
2626
{:else}

‎web/src/lib/diff-viewer-multi-file.svelte.ts

Lines changed: 76 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,20 @@ import {
2020
import type { BundledTheme } from "shiki";
2121
import { browser } from "$app/environment";
2222
import { getEffectiveGlobalTheme } from "$lib/theme.svelte";
23-
import { countOccurrences, type FileTreeNodeData, makeFileTree, type LazyPromise, lazyPromise, watchLocalStorage, animationFramePromise } from "$lib/util";
23+
import {
24+
countOccurrences,
25+
type FileTreeNodeData,
26+
makeFileTree,
27+
type LazyPromise,
28+
lazyPromise,
29+
watchLocalStorage,
30+
animationFramePromise,
31+
yieldToBrowser,
32+
} from "$lib/util";
2433
import { onDestroy, tick } from "svelte";
2534
import { type TreeNode, TreeState } from "$lib/components/tree/index.svelte";
2635
import { VList } from "virtua/svelte";
27-
import { Context, Debounced } from "runed";
36+
import { Context, Debounced, watch } from "runed";
2837
import { MediaQuery } from "svelte/reactivity";
2938
import { ProgressBarState } from "$lib/components/progress-bar/index.svelte";
3039

@@ -338,8 +347,7 @@ export class MultiFileDiffViewerState {
338347
activeSearchResult: ActiveSearchResult | null = $state(null);
339348
sidebarCollapsed = $state(false);
340349
diffMetadata: DiffMetadata | null = $state(null);
341-
loading: boolean = $state(false);
342-
readonly progressBar = $state(new ProgressBarState(null, 100));
350+
readonly loadingState: LoadingState = $state(new LoadingState());
343351

344352
readonly fileTreeFilterDebounced = new Debounced(() => this.fileTreeFilter, 500);
345353
readonly searchQueryDebounced = new Debounced(() => this.searchQuery, 500);
@@ -479,30 +487,51 @@ export class MultiFileDiffViewerState {
479487
}
480488

481489
async loadPatches(meta: () => Promise<DiffMetadata>, patches: () => Promise<AsyncGenerator<FileDetails, void>>) {
482-
if (this.loading) {
490+
if (this.loadingState.loading) {
483491
alert("Already loading patches, please wait.");
484492
return false;
485493
}
486494
try {
487-
this.progressBar.setSpinning();
488-
this.loading = true;
495+
// Show progress bar
496+
this.loadingState.start();
489497
await tick();
490498
await animationFramePromise();
491499

492-
this.diffMetadata = await meta();
500+
// Start potential multiple web requests in parallel
501+
const metaPromise = meta();
502+
const generatorPromise = patches();
503+
504+
// Update metadata
505+
this.diffMetadata = await metaPromise;
493506
await tick();
494507
await animationFramePromise();
495508

509+
// Clear previous state
496510
this.clear(false);
497511
await tick();
498512
await animationFramePromise();
499513

500-
const generator = await patches();
514+
// Setup generator
515+
const generator = await generatorPromise;
516+
await tick();
517+
await animationFramePromise();
501518

519+
// Load patches
502520
const tempDetails: FileDetails[] = [];
521+
let lastYield = performance.now();
522+
let i = 0;
503523
for await (const details of generator) {
524+
i++;
525+
this.loadingState.loadedCount++;
526+
504527
// Pushing directly to the main array causes too many signals to update (lag)
505528
tempDetails.push(details);
529+
530+
if (performance.now() - lastYield > 50 || i % 100 === 0) {
531+
await tick();
532+
await yieldToBrowser();
533+
lastYield = performance.now();
534+
}
506535
}
507536
if (tempDetails.length === 0) {
508537
throw new Error("No valid patches found in the provided data.");
@@ -516,18 +545,23 @@ export class MultiFileDiffViewerState {
516545
alert("Failed to load patches: " + e);
517546
return false;
518547
} finally {
519-
this.loading = false;
548+
// Let the last progress update render before closing the loading state
549+
await tick();
550+
await animationFramePromise();
551+
552+
this.loadingState.done();
520553
}
521554
}
522555

523-
private async loadPatchesGithub(resultPromise: Promise<GithubDiffResult>) {
556+
private async loadPatchesGithub(resultOrPromise: Promise<GithubDiffResult> | GithubDiffResult) {
524557
return await this.loadPatches(
525558
async () => {
526-
return { type: "github", details: (await resultPromise).info };
559+
const result = resultOrPromise instanceof Promise ? await resultOrPromise : resultOrPromise;
560+
return { type: "github", details: await result.info };
527561
},
528562
async () => {
529-
const result = await resultPromise;
530-
return parseMultiFilePatchGithub(result.info, await result.response);
563+
const result = resultOrPromise instanceof Promise ? await resultOrPromise : resultOrPromise;
564+
return parseMultiFilePatchGithub(await result.info, await result.response, this.loadingState);
531565
},
532566
);
533567
}
@@ -661,6 +695,34 @@ export class MultiFileDiffViewerState {
661695
}
662696
}
663697

698+
export class LoadingState {
699+
loading: boolean = $state(false);
700+
loadedCount: number = $state(0);
701+
totalCount: number | null = $state(0);
702+
readonly progressBar = $state(new ProgressBarState(null, 100));
703+
704+
constructor() {
705+
watch([() => this.loadedCount, () => this.totalCount], ([loadedCount, totalCount]) => {
706+
if (totalCount === null || totalCount <= 0) {
707+
this.progressBar.setSpinning();
708+
} else {
709+
this.progressBar.setProgress(loadedCount, totalCount);
710+
}
711+
});
712+
}
713+
714+
start() {
715+
this.loadedCount = 0;
716+
this.totalCount = null;
717+
this.progressBar.setSpinning();
718+
this.loading = true;
719+
}
720+
721+
done() {
722+
this.loading = false;
723+
}
724+
}
725+
664726
export type ActiveSearchResult = {
665727
file: FileDetails;
666728
idx: number;

‎web/src/lib/github.svelte.ts

Lines changed: 63 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { browser } from "$app/environment";
22
import type { components } from "@octokit/openapi-types";
33
import { parseMultiFilePatch, trimCommitHash } from "$lib/util";
4-
import { makeImageDetails } from "$lib/diff-viewer-multi-file.svelte";
4+
import { LoadingState, makeImageDetails } from "$lib/diff-viewer-multi-file.svelte";
55
import { PUBLIC_GITHUB_APP_NAME, PUBLIC_GITHUB_CLIENT_ID } from "$env/static/public";
66

77
export const GITHUB_USERNAME_KEY = "github_username";
@@ -21,7 +21,7 @@ export type GithubDiff = {
2121
};
2222

2323
export type GithubDiffResult = {
24-
info: GithubDiff;
24+
info: Promise<GithubDiff>;
2525
response: Promise<string>;
2626
};
2727

@@ -112,7 +112,7 @@ export async function fetchGithubPRComparison(token: string | null, owner: strin
112112
const base = prInfo.base.sha;
113113
const head = prInfo.head.sha;
114114
const title = `${prInfo.title} (#${prInfo.number})`;
115-
return await fetchGithubComparison(token, owner, repo, base, head, title, prInfo.html_url);
115+
return fetchGithubComparison(token, owner, repo, base, head, title, prInfo.html_url);
116116
}
117117

118118
function injectOptionalToken(token: string | null, opts: RequestInit) {
@@ -124,7 +124,7 @@ function injectOptionalToken(token: string | null, opts: RequestInit) {
124124
}
125125
}
126126

127-
export async function fetchGithubPRInfo(token: string | null, owner: string, repo: string, prNumber: string): Promise<GithubPR> {
127+
async function fetchGithubPRInfo(token: string | null, owner: string, repo: string, prNumber: string): Promise<GithubPR> {
128128
const opts: RequestInit = {
129129
headers: {
130130
Accept: "application/json",
@@ -139,8 +139,8 @@ export async function fetchGithubPRInfo(token: string | null, owner: string, rep
139139
}
140140
}
141141

142-
export function parseMultiFilePatchGithub(details: GithubDiff, patch: string) {
143-
return parseMultiFilePatch(patch, (from, to, status) => {
142+
export function parseMultiFilePatchGithub(details: GithubDiff, patch: string, loadingState: LoadingState) {
143+
return parseMultiFilePatch(patch, loadingState, (from, to, status) => {
144144
const token = getGithubToken();
145145
return makeImageDetails(
146146
from,
@@ -152,67 +152,74 @@ export function parseMultiFilePatchGithub(details: GithubDiff, patch: string) {
152152
});
153153
}
154154

155-
export async function fetchGithubComparison(
155+
export function fetchGithubComparison(
156156
token: string | null,
157157
owner: string,
158158
repo: string,
159159
base: string,
160160
head: string,
161161
description?: string,
162162
url?: string,
163-
): Promise<GithubDiffResult> {
164-
const opts: RequestInit = {
165-
headers: {
166-
Accept: "application/vnd.github.v3.diff",
167-
},
163+
): GithubDiffResult {
164+
return {
165+
info: (async () => {
166+
if (!url) {
167+
url = `https://github.com/${owner}/${repo}/compare/${base}...${head}`;
168+
}
169+
if (!description) {
170+
description = `Comparing ${trimCommitHash(base)}...${trimCommitHash(head)}`;
171+
}
172+
return { owner, repo, base, head, description, backlink: url };
173+
})(),
174+
response: (async () => {
175+
const opts: RequestInit = {
176+
headers: {
177+
Accept: "application/vnd.github.v3.diff",
178+
},
179+
};
180+
injectOptionalToken(token, opts);
181+
const response = await fetch(`https://api.github.com/repos/${owner}/${repo}/compare/${base}...${head}`, opts);
182+
if (!response.ok) {
183+
throw Error(`Failed to retrieve comparison (${response.status}): ${await response.text()}`);
184+
}
185+
return await response.text();
186+
})(),
168187
};
169-
injectOptionalToken(token, opts);
170-
const response = await fetch(`https://api.github.com/repos/${owner}/${repo}/compare/${base}...${head}`, opts);
171-
if (response.ok) {
172-
if (!description) {
173-
description = `Comparing ${trimCommitHash(base)}...${trimCommitHash(head)}`;
174-
}
175-
if (!url) {
176-
url = `https://github.com/${owner}/${repo}/compare/${base}...${head}`;
177-
}
178-
const info = { owner, repo, base, head, description, backlink: url };
179-
return { response: response.text(), info };
180-
} else {
181-
throw Error(`Failed to retrieve comparison (${response.status}): ${await response.text()}`);
182-
}
183188
}
184189

185-
export async function fetchGithubCommitDiff(token: string | null, owner: string, repo: string, commit: string): Promise<GithubDiffResult> {
186-
const diffOpts: RequestInit = {
187-
headers: {
188-
Accept: "application/vnd.github.v3.diff",
189-
},
190-
};
191-
injectOptionalToken(token, diffOpts);
190+
export function fetchGithubCommitDiff(token: string | null, owner: string, repo: string, commit: string): GithubDiffResult {
192191
const url = `https://api.github.com/repos/${owner}/${repo}/commits/${commit}`;
193-
const response = await fetch(url, diffOpts);
194-
if (response.ok) {
195-
const metaOpts: RequestInit = {
196-
headers: {
197-
Accept: "application/vnd.github+json",
198-
},
199-
};
200-
injectOptionalToken(token, metaOpts);
201-
const metaResponse = await fetch(url, metaOpts);
202-
if (!metaResponse.ok) {
203-
throw Error(`Failed to retrieve commit meta (${metaResponse.status}): ${await metaResponse.text()}`);
204-
}
205-
const meta: GithubCommitDetails = await metaResponse.json();
206-
const firstParent = meta.parents[0].sha;
207-
const description = `${meta.commit.message.split("\n")[0]} (${trimCommitHash(commit)})`;
208-
const info = { owner, repo, base: firstParent, head: commit, description, backlink: meta.html_url };
209-
return {
210-
response: response.text(),
211-
info,
212-
};
213-
} else {
214-
throw Error(`Failed to retrieve commit diff (${response.status}): ${await response.text()}`);
215-
}
192+
return {
193+
info: (async () => {
194+
const metaOpts: RequestInit = {
195+
headers: {
196+
Accept: "application/vnd.github+json",
197+
},
198+
};
199+
injectOptionalToken(token, metaOpts);
200+
const metaResponse = await fetch(url, metaOpts);
201+
if (!metaResponse.ok) {
202+
throw Error(`Failed to retrieve commit meta (${metaResponse.status}): ${await metaResponse.text()}`);
203+
}
204+
const meta: GithubCommitDetails = await metaResponse.json();
205+
const firstParent = meta.parents[0].sha;
206+
const description = `${meta.commit.message.split("\n")[0]} (${trimCommitHash(commit)})`;
207+
return { owner, repo, base: firstParent, head: commit, description, backlink: meta.html_url };
208+
})(),
209+
response: (async () => {
210+
const diffOpts: RequestInit = {
211+
headers: {
212+
Accept: "application/vnd.github.v3.diff",
213+
},
214+
};
215+
injectOptionalToken(token, diffOpts);
216+
const response = await fetch(url, diffOpts);
217+
if (!response.ok) {
218+
throw Error(`Failed to retrieve commit diff (${response.status}): ${await response.text()}`);
219+
}
220+
return await response.text();
221+
})(),
222+
};
216223
}
217224

218225
export async function fetchGithubFile(token: string | null, owner: string, repo: string, path: string, ref: string): Promise<Blob> {

‎web/src/lib/util.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { type FileDetails, type ImageFileDetails, makeTextDetails } from "./diff-viewer-multi-file.svelte";
1+
import { type FileDetails, type ImageFileDetails, LoadingState, makeTextDetails } from "./diff-viewer-multi-file.svelte";
22
import type { FileStatus } from "./github.svelte";
33
import type { TreeNode } from "$lib/components/tree/index.svelte";
44
import type { BundledLanguage, SpecialLanguage } from "shiki";
@@ -146,9 +146,11 @@ function parseHeader(patch: string, fromFile: string, toFile: string): BasicHead
146146

147147
export function parseMultiFilePatch(
148148
patchContent: string,
149+
loadingState: LoadingState,
149150
imageFactory?: (fromFile: string, toFile: string, status: FileStatus) => ImageFileDetails | null,
150151
): AsyncGenerator<FileDetails> {
151152
const split = splitMultiFilePatch(patchContent);
153+
loadingState.totalCount = split.length;
152154
async function* detailsGenerator() {
153155
for (const [header, content] of split) {
154156
if (header.binary) {
@@ -469,6 +471,10 @@ export function animationFramePromise() {
469471
});
470472
}
471473

474+
export async function yieldToBrowser() {
475+
await new Promise((resolve) => setTimeout(resolve, 0));
476+
}
477+
472478
// from bits-ui internals
473479
export type ReadableBoxedValues<T> = {
474480
[K in keyof T]: ReadableBox<T[K]>;

‎web/src/routes/+page.svelte

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,9 +138,9 @@
138138
</SettingsPopover>
139139
{/snippet}
140140

141-
{#if viewer.loading}
141+
{#if viewer.loadingState.loading}
142142
<div class="absolute bottom-1/2 left-1/2 z-50 -translate-x-1/2 translate-y-1/2 rounded-full border bg-neutral p-2 shadow-md">
143-
<ProgressBar bind:state={viewer.progressBar} class="h-2 w-32" />
143+
<ProgressBar bind:state={viewer.loadingState.progressBar} class="h-2 w-32" />
144144
</div>
145145
{/if}
146146

0 commit comments

Comments
 (0)