diff --git a/packages/app/e2e/browser/changes-first-edit.spec.ts b/packages/app/e2e/browser/changes-first-edit.spec.ts new file mode 100644 index 0000000000..72c3890936 --- /dev/null +++ b/packages/app/e2e/browser/changes-first-edit.spec.ts @@ -0,0 +1,122 @@ +import { appendFile } from "node:fs/promises"; +import path from "node:path"; +import type { Page } from "@playwright/test"; +import { test, expect } from "../support/fixtures"; +import { openChangesTreePanel } from "../support/helpers/workspace-tabs"; + +function changesTree(page: Page) { + return page.getByTestId("changes-tree-panel").filter({ visible: true }); +} + +async function expectEmptyChanges(page: Page) { + await expect(changesTree(page).getByText("No changes to display", { exact: true })).toBeVisible(); +} + +async function editReadme(repoPath: string, text: string) { + await appendFile(path.join(repoPath, "README.md"), text); +} + +async function expectReadmeChanges(page: Page, additions: number) { + await expect(changesTree(page).getByText("README.md", { exact: true })).toBeVisible({ + timeout: 30_000, + }); + await expect(changesTree(page)).toContainText(`+${additions}`); +} + +async function openEmptyDiff(page: Page) { + await changesTree(page).getByTestId("changes-options-menu").click(); + await page.getByTestId("changes-open-tab").click(); + await expect( + page + .getByTestId("working-diff-panel") + .filter({ visible: true }) + .getByText("No changes to display", { exact: true }), + ).toBeVisible(); +} + +async function expectReadmeDiff(page: Page, additions: number) { + await expect( + page.getByTestId("working-diff-panel").filter({ visible: true }).getByTestId("diff-file-0"), + ).toHaveAccessibleName(`README.md, +${additions}, -0`, { timeout: 30_000 }); +} + +async function openDiffFromNewTabMenu(page: Page) { + await page.getByTestId("workspace-new-tab-button").filter({ visible: true }).first().click(); + await page.getByTestId("workspace-new-tab-menu-diff").filter({ visible: true }).first().click(); + await expect( + page + .getByTestId("working-diff-panel") + .filter({ visible: true }) + .getByText("No changes to display", { exact: true }), + ).toBeVisible(); +} + +async function refreshDiff(page: Page) { + await page + .getByTestId("working-diff-panel") + .filter({ visible: true }) + .getByTestId("changes-refresh") + .click(); +} + +async function reloadWorkspace(page: Page) { + await page.reload(); +} + +test("Changes follows the first edit to a clean workspace without reloading", async ({ + page, + withWorkspace, +}) => { + const workspace = await withWorkspace(); + await page.setViewportSize({ width: 1400, height: 900 }); + await workspace.navigateTo(); + await openChangesTreePanel(page); + await expectEmptyChanges(page); + + await editReadme(workspace.repoPath, "first edit\n"); + await expectReadmeChanges(page, 1); + + await editReadme(workspace.repoPath, "second edit\n"); + await expectReadmeChanges(page, 2); +}); + +test("the full diff follows the first edit to a clean workspace without reloading", async ({ + page, + withWorkspace, +}) => { + const workspace = await withWorkspace(); + await page.setViewportSize({ width: 1400, height: 900 }); + await workspace.navigateTo(); + await openChangesTreePanel(page); + await expectEmptyChanges(page); + await openEmptyDiff(page); + + await editReadme(workspace.repoPath, "first edit\n"); + await expectReadmeDiff(page, 1); + + await editReadme(workspace.repoPath, "second edit\n"); + await expectReadmeDiff(page, 2); +}); + +test("a directly opened Diff tab follows the first edit without an active Changes tree", async ({ + page, + withWorkspace, +}) => { + const workspace = await withWorkspace(); + await page.setViewportSize({ width: 1400, height: 900 }); + await workspace.navigateTo(); + await openDiffFromNewTabMenu(page); + + await editReadme(workspace.repoPath, "first edit\n"); + await expectReadmeDiff(page, 1); + + await editReadme(workspace.repoPath, "second edit\n"); + await expectReadmeDiff(page, 2); + + await refreshDiff(page); + await expectReadmeDiff(page, 2); + await reloadWorkspace(page); + await expectReadmeDiff(page, 2); + await editReadme(workspace.repoPath, "third edit\n"); + await expectReadmeDiff(page, 3); +}); diff --git a/packages/app/src/data/push-router.test.ts b/packages/app/src/data/push-router.test.ts index 01fba418a9..3bf9e99f9b 100644 --- a/packages/app/src/data/push-router.test.ts +++ b/packages/app/src/data/push-router.test.ts @@ -199,6 +199,71 @@ function providerUpdate(generatedAt: string): ProvidersSnapshotUpdateMessage { } describe("server data push router", () => { + it("keeps a visible diff subscribed when a hidden panel switches to the same comparison", () => { + const queryClient = new QueryClient(); + const fake = createFakeClient(); + const serverId = "server-1"; + const cwd = "/repo"; + const options = (mode: "base" | "uncommitted", enabled: boolean) => { + const queryKey = checkoutDiffQueryKey( + serverId, + cwd, + mode, + mode === "base" ? "main" : undefined, + ); + return { + queryKey, + queryFn: skipToken as typeof skipToken, + gcTime: Infinity, + meta: checkoutDiffPushRoute({ + enabled, + serverId, + cwd, + subscriptionId: `checkoutDiff:${JSON.stringify(queryKey)}`, + compare: { mode, ...(mode === "base" ? { baseRef: "main" } : {}) }, + }), + }; + }; + const visible = new QueryObserver(queryClient, options("base", true)); + const hidden = new QueryObserver(queryClient, options("base", false)); + const unsubscribeVisible = visible.subscribe(() => {}); + const unsubscribeHidden = hidden.subscribe(() => {}); + const unmount = mountServerDataPushRouter({ queryClient, client: fake.client, serverId }); + try { + visible.setOptions(options("uncommitted", true)); + hidden.setOptions(options("uncommitted", false)); + const subscription = fake.subscribeCheckoutDiffCalls.findLast( + (call) => call.compare.mode === "uncommitted", + )!; + const files = [ + { + path: "README.md", + isNew: false, + isDeleted: false, + additions: 1, + deletions: 0, + hunks: [], + }, + ]; + fake.emit({ + type: "checkout_diff_update", + payload: { subscriptionId: subscription.subscriptionId, cwd, files, error: null }, + }); + expect(queryClient.getQueryData(options("uncommitted", true).queryKey)).toMatchObject({ + files, + }); + expect(fake.unsubscribeCheckoutDiffCalls).not.toContain(subscription.subscriptionId); + + unsubscribeVisible(); + expect(fake.unsubscribeCheckoutDiffCalls).toContain(subscription.subscriptionId); + } finally { + unsubscribeVisible(); + unsubscribeHidden(); + unmount(); + queryClient.clear(); + } + }); + it("retains equal diff files without reconstructing changed file bodies", () => { const queryClient = new QueryClient(); const fake = createFakeClient(); diff --git a/packages/app/src/data/push-router.ts b/packages/app/src/data/push-router.ts index fc6d51ce4f..5ab3069bd0 100644 --- a/packages/app/src/data/push-router.ts +++ b/packages/app/src/data/push-router.ts @@ -494,6 +494,14 @@ function getActiveServerDataRoute( if (query.getObserversCount() === 0) { return null; } + // Query metadata belongs to the last observer that updated its options. A + // retained hidden panel must not cancel demand from another visible panel. + const observerRoutes = query.observers + .map((observer) => getServerDataRoute({ meta: observer.options.meta })) + .filter((route): route is ServerDataRoute => route !== null); + if (observerRoutes.length > 0) { + return observerRoutes.find((route) => route.enabled && route.serverId === serverId) ?? null; + } const route = getServerDataRoute(query); if (route) { return route.enabled && route.serverId === serverId ? route : null; @@ -590,7 +598,7 @@ function canEventChangeDesiredSubscriptions(type: QueryCacheNotifyEvent["type"]) ); } -function getServerDataRoute(query: Query): ServerDataRoute | null { +function getServerDataRoute(query: Pick): ServerDataRoute | null { const meta = query.meta; if (!isRecord(meta) || !isRecord(meta.serverData)) { return null;