Refactor patch loading and increase responsiveness while loading/selecting patches

Author: jpenilla

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

@@ -1,4 +1,13 @@
-import { fetchGithubCommitDiff, fetchGithubComparison, fetchGithubPRComparison, type FileStatus, getGithubToken, type GithubDiff } from "./github.svelte";
+import {
+    fetchGithubCommitDiff,
+    fetchGithubComparison,
+    fetchGithubPRComparison,
+    type FileStatus,
+    getGithubToken,
+    type GithubDiff,
+    type GithubDiffResult,
+    splitMultiFilePatchGithub,
+} from "./github.svelte";
 import { type StructuredPatch } from "diff";
 import {
     ConciseDiffViewCachedState,
@@ -11,8 +20,8 @@ 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 } from "$lib/util";
-import { onDestroy } from "svelte";
+import { countOccurrences, type FileTreeNodeData, makeFileTree, type LazyPromise, lazyPromise, watchLocalStorage, animationFramePromise } from "$lib/util";
+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";
@@ -319,6 +328,7 @@ export class MultiFileDiffViewerState {
 
     fileTreeFilter: string = $state("");
     searchQuery: string = $state("");
+    // TODO remove parallel arrays to fix order-dependency issues
     collapsed: boolean[] = $state([]);
     checked: boolean[] = $state([]);
     fileDetails: FileDetails[] = $state([]);
@@ -455,26 +465,72 @@ export class MultiFileDiffViewerState {
         }
     }
 
-    loadPatches(patches: FileDetails[], meta: DiffMetadata | null) {
+    private clear(clearMeta: boolean = true) {
         // Reset state
         this.collapsed = [];
         this.checked = [];
-        this.diffMetadata = null;
+        if (clearMeta) {
+            this.diffMetadata = null;
+        }
         this.fileDetails = [];
         this.clearImages();
         this.vlist?.scrollToIndex(0, { align: "start" });
+    }
+
+    async loadPatches(meta: () => Promise<DiffMetadata>, patches: () => Promise<AsyncGenerator<FileDetails, void>>) {
+        try {
+            this.progressBar.setSpinning();
+            await tick();
+            await animationFramePromise();
+
+            this.diffMetadata = await meta();
+            await tick();
+            await animationFramePromise();
+
+            this.clear(false);
+            await tick();
+            await animationFramePromise();
 
-        // Load new state
-        this.diffMetadata = meta;
-        patches.sort(compareFileDetails);
-        this.fileDetails.push(...patches);
+            const generator = await patches();
 
-        // in case the caller didn't close the progress
-        if (!this.progressBar.isDone()) {
-            this.progressBar.setProgress(100, 100);
+            const tempDetails: FileDetails[] = [];
+            for await (const details of generator) {
+                // Pushing directly to the main array causes too many signals to update (lag)
+                tempDetails.push(details);
+            }
+            tempDetails.sort(compareFileDetails);
+            this.fileDetails.push(...tempDetails);
+            return true;
+        } catch (e) {
+            this.clear(); // Clear any partially loaded state
+            console.error("Failed to load patches:", e);
+            alert("Failed to load patches: " + e);
+            return false;
+        } finally {
+            if (!this.progressBar.isDone()) {
+                this.progressBar.setProgress(100, 100);
+            }
         }
     }
 
+    private async loadPatchesGithub(resultPromise: Promise<GithubDiffResult>) {
+        return await this.loadPatches(
+            async () => {
+                return { type: "github", details: (await resultPromise).info };
+            },
+            async () => {
+                const result = await resultPromise;
+                const split = splitMultiFilePatchGithub(result.info, result.response);
+                async function* generatePatches() {
+                    for (const patch of split) {
+                        yield patch;
+                    }
+                }
+                return generatePatches();
+            },
+        );
+    }
+
     // TODO fails for initial commit?
     // handle matched github url
     async loadFromGithubApi(match: Array<string>): Promise<boolean> {
@@ -483,15 +539,9 @@ export class MultiFileDiffViewerState {
 
         try {
             if (type === "commit") {
-                this.progressBar.setSpinning();
-                const { info, files } = await fetchGithubCommitDiff(token, owner, repo, id.split("/")[0]);
-                this.loadPatches(files, { type: "github", details: info });
-                return true;
+                return await this.loadPatchesGithub(fetchGithubCommitDiff(token, owner, repo, id.split("/")[0]));
             } else if (type === "pull") {
-                this.progressBar.setSpinning();
-                const { info, files } = await fetchGithubPRComparison(token, owner, repo, id.split("/")[0]);
-                this.loadPatches(files, { type: "github", details: info });
-                return true;
+                return await this.loadPatchesGithub(fetchGithubPRComparison(token, owner, repo, id.split("/")[0]));
             } else if (type === "compare") {
                 let refs = id.split("...");
                 if (refs.length !== 2) {
@@ -501,12 +551,9 @@ export class MultiFileDiffViewerState {
                         return false;
                     }
                 }
-                this.progressBar.setSpinning();
                 const base = refs[0];
                 const head = refs[1];
-                const { info, files } = await fetchGithubComparison(token, owner, repo, base, head);
-                this.loadPatches(files, { type: "github", details: info });
-                return true;
+                return await this.loadPatchesGithub(fetchGithubComparison(token, owner, repo, base, head));
             }
         } catch (error) {
             console.error(error);

web/src/lib/github.svelte.ts

@@ -1,7 +1,7 @@
 import { browser } from "$app/environment";
 import type { components } from "@octokit/openapi-types";
 import { splitMultiFilePatch, trimCommitHash } from "$lib/util";
-import { type FileDetails, makeImageDetails } from "$lib/diff-viewer-multi-file.svelte";
+import { 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";
@@ -22,7 +22,7 @@ export type GithubDiff = {
 
 export type GithubDiffResult = {
     info: GithubDiff;
-    files: FileDetails[];
+    response: string;
 };
 
 if (browser) {
@@ -139,7 +139,7 @@ export async function fetchGithubPRInfo(token: string | null, owner: string, rep
     }
 }
 
-function splitMultiFilePatchGithub(details: GithubDiff, patch: string) {
+export function splitMultiFilePatchGithub(details: GithubDiff, patch: string) {
     return splitMultiFilePatch(patch, (from, to, status) => {
         const token = getGithubToken();
         return makeImageDetails(
@@ -176,7 +176,7 @@ export async function fetchGithubComparison(
             url = `https://github.com/${owner}/${repo}/compare/${base}...${head}`;
         }
         const info = { owner, repo, base, head, description, backlink: url };
-        return { files: splitMultiFilePatchGithub(info, await response.text()), info };
+        return { response: await response.text(), info };
     } else {
         throw Error(`Failed to retrieve comparison (${response.status}): ${await response.text()}`);
     }
@@ -207,7 +207,7 @@ export async function fetchGithubCommitDiff(token: string | null, owner: string,
         const description = `${meta.commit.message.split("\n")[0]} (${trimCommitHash(commit)})`;
         const info = { owner, repo, base: firstParent, head: commit, description, backlink: meta.html_url };
         return {
-            files: splitMultiFilePatchGithub(info, await response.text()),
+            response: await response.text(),
             info,
         };
     } else {

web/src/lib/util.ts

@@ -455,6 +455,12 @@ export const resizeObserver: Action<HTMLElement, ResizeObserverCallback> = (node
     };
 };
 
+export function animationFramePromise() {
+    return new Promise((resolve) => {
+        requestAnimationFrame(resolve);
+    });
+}
+
 // from bits-ui internals
 export type ReadableBoxedValues<T> = {
     [K in keyof T]: ReadableBox<T[K]>;