mirror of
https://github.com/getpaseo/paseo.git
synced 2026-11-10 10:25:03 +00:00
fix(app): keep the Changes diff on screen when its panel reopens (#9241)
Hiding the Changes panel releases its diff subscription. Reopening it starts a new stream, and since #8907 a new stream first publishes a loading snapshot (entries, no bodies) before its first revision commits. That snapshot replaced the settled diff in the query cache, so the panel showed the full loading spinner for about a second and then repainted the same diff. A loading snapshot now only counts on the first load. Once a diff has settled, shareCheckoutDiff keeps it until the new stream's ready revision lands, which replaces it in one step when the content changed.
This commit is contained in:
@@ -306,6 +306,24 @@ test("streamed Changes shows the tree before completion and applies a one-file r
|
||||
await page.screenshot({ path: testInfo.outputPath("refreshed.png") });
|
||||
});
|
||||
|
||||
test("reopening compact Changes keeps the painted diff while it resubscribes", async ({ page }) => {
|
||||
const workspace = await createWorkspaceWithMountedTabDiff();
|
||||
const stream = await holdDiffCompletion(page);
|
||||
const explorer = await openCompactChanges(page, workspace);
|
||||
stream.finish();
|
||||
const firstFile = explorer.getByTestId("diff-file-0");
|
||||
await expect(firstFile).toBeVisible({ timeout: 30_000 });
|
||||
const painted = await firstFile.getAttribute("aria-label");
|
||||
|
||||
await explorer.getByTestId("workspace-tab-files").click();
|
||||
stream.hold();
|
||||
await explorer.getByTestId("workspace-tab-changes_tree").click();
|
||||
await expect.poll(() => stream.held, { timeout: 30_000 }).toBeGreaterThan(0);
|
||||
await expect(page.getByRole("progressbar", { name: "Loading..." })).toHaveCount(0);
|
||||
await expect(firstFile).toHaveAccessibleName(painted ?? "");
|
||||
stream.finish();
|
||||
});
|
||||
|
||||
async function createLargeStreamedWorkspace(): Promise<DirtyWorkspace> {
|
||||
const workspace = await createWorkspaceWithManyTinyDiffs(252);
|
||||
await Promise.all(
|
||||
@@ -371,6 +389,12 @@ async function holdDiffCompletion(page: Page) {
|
||||
get completed() {
|
||||
return completed;
|
||||
},
|
||||
get held() {
|
||||
return completions.length;
|
||||
},
|
||||
hold: () => {
|
||||
holding = true;
|
||||
},
|
||||
finish: () => {
|
||||
holding = false;
|
||||
completions.splice(0).forEach((send) => send());
|
||||
|
||||
@@ -2,6 +2,8 @@ import type { DiffState, OwnedSubscription, SubscriptionObserver } from "@getpas
|
||||
import { QueryClient, QueryObserver, skipToken } from "@tanstack/react-query";
|
||||
import { describe, expect, it } from "vitest";
|
||||
import type { MutableDaemonConfig, SessionOutboundMessage } from "@getpaseo/protocol/messages";
|
||||
import { deriveCheckoutDiffResult, type CheckoutDiffSnapshot } from "@/git/diff-snapshot";
|
||||
import { shareCheckoutDiff } from "@/git/diff-sharing";
|
||||
import { checkoutDiffQueryKey } from "@/git/query-keys";
|
||||
import { buildTerminalsQueryKey } from "@/screens/workspace/terminals/state";
|
||||
import { daemonConfigQueryKey } from "@/data/daemon-config";
|
||||
@@ -34,6 +36,7 @@ type RouterMessage =
|
||||
type RouterMessageType = RouterMessage["type"];
|
||||
type RouterHandler = (message: RouterMessage) => void;
|
||||
type RouterClient = Parameters<typeof mountServerDataPushRouter>[0]["client"];
|
||||
type DiffFile = SubscribeCheckoutDiffResponseMessage["payload"]["files"][number];
|
||||
|
||||
const daemonConfig: MutableDaemonConfig = {
|
||||
relay: { enabled: false },
|
||||
@@ -233,6 +236,49 @@ function createFakeClient(config: { rejectCheckoutDiffSubscribe?: boolean } = {}
|
||||
};
|
||||
}
|
||||
|
||||
/** Matches the real diff observation: nothing is delivered until the stream publishes. */
|
||||
type SubscribedDiffState = DiffState & { subscriptionId: string };
|
||||
|
||||
function createDiffStreamClient() {
|
||||
const subscriptions: Array<{ publish(state: DiffState): void }> = [];
|
||||
const noSubscription = {
|
||||
subscriptionId: "none",
|
||||
ready: Promise.resolve(undefined as never),
|
||||
subscribe: () => () => {},
|
||||
release: async () => {},
|
||||
};
|
||||
const client: RouterClient = {
|
||||
async getProvidersSnapshot() {
|
||||
throw new Error("Unexpected snapshot pull");
|
||||
},
|
||||
observeEvents: () => noSubscription,
|
||||
observeTerminals: () => noSubscription,
|
||||
observeDiff() {
|
||||
const observers = new Set<SubscriptionObserver<SubscribedDiffState>>();
|
||||
let released = false;
|
||||
const subscriptionId = `diff-${subscriptions.length + 1}`;
|
||||
subscriptions.push({
|
||||
publish: (state) => {
|
||||
if (released) return;
|
||||
for (const observer of observers) observer.snapshot({ ...state, subscriptionId });
|
||||
},
|
||||
});
|
||||
return {
|
||||
subscriptionId,
|
||||
ready: new Promise<never>(() => {}),
|
||||
subscribe: (observer: SubscriptionObserver<SubscribedDiffState>) => {
|
||||
observers.add(observer);
|
||||
return () => observers.delete(observer);
|
||||
},
|
||||
release: async () => {
|
||||
released = true;
|
||||
},
|
||||
};
|
||||
},
|
||||
};
|
||||
return { client, subscriptions, latest: () => subscriptions[subscriptions.length - 1]! };
|
||||
}
|
||||
|
||||
function providerUpdate(generatedAt: string): ProvidersSnapshotUpdateMessage {
|
||||
return {
|
||||
type: "providers_snapshot_update",
|
||||
@@ -388,6 +434,160 @@ describe("server data push router", () => {
|
||||
}
|
||||
});
|
||||
|
||||
describe("a diff reopened from a retained panel", () => {
|
||||
const serverId = "server-1";
|
||||
const cwd = "/repo";
|
||||
const queryKey = checkoutDiffQueryKey(serverId, cwd, "uncommitted", undefined, false);
|
||||
|
||||
function diffFile(content: string): DiffFile {
|
||||
return {
|
||||
path: "README.md",
|
||||
isNew: false,
|
||||
isDeleted: false,
|
||||
additions: 1,
|
||||
deletions: 0,
|
||||
hunks: [
|
||||
{
|
||||
oldStart: 1,
|
||||
oldCount: 1,
|
||||
newStart: 1,
|
||||
newCount: 1,
|
||||
lines: [{ type: "add", content, tokens: [{ text: content, style: null }] }],
|
||||
},
|
||||
],
|
||||
};
|
||||
}
|
||||
|
||||
function loadingState(revision: number): DiffState {
|
||||
return {
|
||||
phase: "loading",
|
||||
revision,
|
||||
entries: [
|
||||
{
|
||||
path: "README.md",
|
||||
oldPath: null,
|
||||
change: "modified",
|
||||
content: "text",
|
||||
additions: 1,
|
||||
deletions: 0,
|
||||
},
|
||||
],
|
||||
};
|
||||
}
|
||||
|
||||
function readyState(revision: number, files: DiffFile[]): DiffState {
|
||||
return {
|
||||
phase: "ready",
|
||||
revision,
|
||||
resolved: { baseSha: "", headSha: "", mergeBaseSha: "" },
|
||||
files,
|
||||
diffTooLarge: false,
|
||||
};
|
||||
}
|
||||
|
||||
function mountChangesPanel(fake: ReturnType<typeof createDiffStreamClient>) {
|
||||
const queryClient = new QueryClient();
|
||||
const route = (enabled: boolean) => ({
|
||||
queryKey,
|
||||
queryFn: skipToken as typeof skipToken,
|
||||
gcTime: Infinity,
|
||||
structuralSharing: shareCheckoutDiff,
|
||||
meta: checkoutDiffPushRoute({
|
||||
enabled,
|
||||
serverId,
|
||||
cwd,
|
||||
subscriptionId: `checkoutDiff:${JSON.stringify(queryKey)}`,
|
||||
compare: { mode: "uncommitted", ignoreWhitespace: false },
|
||||
}),
|
||||
});
|
||||
const observer = new QueryObserver(queryClient, route(true));
|
||||
const unsubscribeObserver = observer.subscribe(() => {});
|
||||
const unmountRouter = mountServerDataPushRouter({
|
||||
queryClient,
|
||||
client: fake.client,
|
||||
serverId,
|
||||
});
|
||||
return {
|
||||
hide: () => observer.setOptions(route(false)),
|
||||
show: () => observer.setOptions(route(true)),
|
||||
// What the Changes pane renders from.
|
||||
diff: () =>
|
||||
deriveCheckoutDiffResult(
|
||||
queryClient.getQueryData<CheckoutDiffSnapshot>(queryKey) ?? null,
|
||||
),
|
||||
unmount: () => {
|
||||
unsubscribeObserver();
|
||||
unmountRouter();
|
||||
queryClient.clear();
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
it("keeps the painted diff through the reload when the daemon returns the same content", () => {
|
||||
const fake = createDiffStreamClient();
|
||||
const panel = mountChangesPanel(fake);
|
||||
try {
|
||||
fake.latest().publish(readyState(1, [diffFile("same")]));
|
||||
const painted = panel.diff().files;
|
||||
expect(fake.subscriptions).toHaveLength(1);
|
||||
|
||||
panel.hide();
|
||||
panel.show();
|
||||
expect(fake.subscriptions).toHaveLength(2);
|
||||
|
||||
fake.latest().publish(loadingState(1));
|
||||
expect(panel.diff()).toMatchObject({ isLoading: false, entries: null });
|
||||
expect(panel.diff().files).toBe(painted);
|
||||
|
||||
fake.latest().publish(readyState(1, [diffFile("same")]));
|
||||
expect(panel.diff().isLoading).toBe(false);
|
||||
expect(panel.diff().files).toBe(painted);
|
||||
} finally {
|
||||
panel.unmount();
|
||||
}
|
||||
});
|
||||
|
||||
it("replaces the painted diff in one step when the daemon returns changed content", () => {
|
||||
const fake = createDiffStreamClient();
|
||||
const panel = mountChangesPanel(fake);
|
||||
try {
|
||||
fake.latest().publish(readyState(1, [diffFile("before")]));
|
||||
const painted = panel.diff().files;
|
||||
|
||||
panel.hide();
|
||||
panel.show();
|
||||
fake.latest().publish(loadingState(1));
|
||||
expect(panel.diff().isLoading).toBe(false);
|
||||
expect(panel.diff().files).toBe(painted);
|
||||
|
||||
const changed = [diffFile("after")];
|
||||
fake.latest().publish(readyState(1, changed));
|
||||
expect(panel.diff().isLoading).toBe(false);
|
||||
expect(panel.diff().files).toEqual(changed);
|
||||
} finally {
|
||||
panel.unmount();
|
||||
}
|
||||
});
|
||||
|
||||
it("reports loading with the announced entries when no content has been painted", () => {
|
||||
const fake = createDiffStreamClient();
|
||||
const panel = mountChangesPanel(fake);
|
||||
try {
|
||||
expect(panel.diff().isLoading).toBe(true);
|
||||
|
||||
fake.latest().publish(loadingState(1));
|
||||
expect(panel.diff()).toMatchObject({ isLoading: true, files: [] });
|
||||
expect(panel.diff().entries).toHaveLength(1);
|
||||
|
||||
fake.latest().publish(readyState(1, [diffFile("first")]));
|
||||
expect(panel.diff()).toMatchObject({ isLoading: false, entries: null });
|
||||
expect(panel.diff().files).toHaveLength(1);
|
||||
} finally {
|
||||
panel.unmount();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
it("routes provider snapshot and daemon config payloads until detached", async () => {
|
||||
const queryClient = new QueryClient();
|
||||
const fake = createFakeClient();
|
||||
|
||||
@@ -25,6 +25,10 @@ export function shareCheckoutDiff(previous: unknown, incoming: unknown): DiffPay
|
||||
const next = incoming as DiffPayload;
|
||||
const old = previous as DiffPayload | undefined;
|
||||
if (!old || old === next) return next;
|
||||
// A new subscription announces its entries before the bodies arrive. That is
|
||||
// progress on the first load only: once a diff has settled it stays on screen
|
||||
// until the next revision commits, so an unchanged diff never repaints.
|
||||
if (isLoadingSnapshot(next) && !isLoadingSnapshot(old)) return old;
|
||||
const files = retainDiffFiles(old.files, next.files);
|
||||
const error = replaceEqualDeep(old.error, next.error);
|
||||
return files === old.files &&
|
||||
@@ -37,6 +41,10 @@ export function shareCheckoutDiff(previous: unknown, incoming: unknown): DiffPay
|
||||
: { ...next, files, error };
|
||||
}
|
||||
|
||||
function isLoadingSnapshot(payload: DiffPayload): boolean {
|
||||
return payload.entries !== undefined;
|
||||
}
|
||||
|
||||
export function shareCommitFileDiff(
|
||||
previous: unknown,
|
||||
incoming: unknown,
|
||||
|
||||
Reference in New Issue
Block a user