diff --git a/apps/web/src/components/pullRequest/PullRequestCodeTab.browser.test.tsx b/apps/web/src/components/pullRequest/PullRequestCodeTab.browser.test.tsx new file mode 100644 index 000000000000..4d28e968f266 --- /dev/null +++ b/apps/web/src/components/pullRequest/PullRequestCodeTab.browser.test.tsx @@ -0,0 +1,147 @@ +import { + EnvironmentId, + ProjectId, + SourceControlProviderKind, + type PullRequestDetailView, + type PullRequestDiffResult, +} from "@t3tools/contracts"; +import { act } from "react"; +import { createRoot, type Root } from "react-dom/client"; +import { afterEach, beforeEach, expect, it, vi } from "vite-plus/test"; +import { page } from "vite-plus/test/browser"; + +import "~/index.css"; + +const { diffAtom } = await vi.hoisted(async () => { + const { AsyncResult, Atom } = await import("effect/reactivity"); + const firstPage: PullRequestDiffResult = { + patch: [ + "diff --git a/a.ts b/a.ts", + "index 1111111..2222222 100644", + "--- a/a.ts", + "+++ b/a.ts", + "@@ -1 +1 @@", + "-before()", + "+after()", + "", + ].join("\n"), + truncated: false, + nextCursor: null, + }; + // An environment query whose refetch returns equal content hands back the previous value + // itself, so a refresh publishes a new result around the very same page. + return { diffAtom: Atom.make(() => AsyncResult.success(firstPage)) }; +}); + +vi.mock(import("~/state/use-atom-command"), async (original) => ({ + ...(await original()), + useAtomCommand: (() => vi.fn()) as never, +})); +vi.mock(import("~/state/pullRequests"), async (original) => { + const actual = await original(); + return { + ...actual, + pullRequestEnvironment: { + ...actual.pullRequestEnvironment, + diff: (() => diffAtom) as never, + }, + }; +}); + +import PullRequestCodeTab from "./PullRequestCodeTab"; + +const detail: PullRequestDetailView = { + provider: SourceControlProviderKind.make("github"), + projectId: ProjectId.make("project"), + projectTitle: "Project", + workspaceRoot: "/workspace", + repository: "owner/repo", + number: 1, + title: "Test pull request", + body: "", + url: "https://github.com/owner/repo/pull/1", + author: { login: "author", name: null, avatarUrl: null }, + viewer: "author", + state: "open", + isDraft: false, + mergeability: "mergeable", + additions: 1, + deletions: 1, + changedFiles: 1, + headBranch: "feature", + baseBranch: "main", + createdAt: "2026-09-01T00:00:00Z", + updatedAt: "2026-09-01T00:00:00Z", + mergedAt: null, + closedAt: null, + reviewers: [], + labels: [], + checks: [], + comments: [], + commentCount: 0, + commentsTruncated: false, + reviewThreads: [], + commits: [], + mergeCapabilities: { merge: false, squash: false, rebase: false }, + capabilities: { + diff: true, + comment: false, + search: true, + actions: [], + mergeMethods: [], + review: { inlineComment: false, reply: false, resolve: false, verdicts: [] }, + reviewers: { request: false, listCandidates: false }, + edit: { changeRequest: false, comment: false }, + }, + viewerPermissions: { + actions: [], + comment: false, + resolve: false, + verdicts: [], + requestReviewers: false, + }, +}; + +let root: Root; +let container: HTMLDivElement; +beforeEach(() => { + vi.stubGlobal("IS_REACT_ACT_ENVIRONMENT", true); + container = document.createElement("div"); + container.className = "flex h-[700px] w-[900px]"; + document.body.append(container); + root = createRoot(container); +}); +afterEach(async () => { + await act(async () => root.unmount()); + container.remove(); + localStorage.clear(); + vi.unstubAllGlobals(); +}); + +const render = (refreshToken: number) => + act(async () => + root.render( + {}} + environmentId={EnvironmentId.make("environment")} + reference={detail} + detail={detail} + selectedCommitOid={null} + onSelectedCommitChange={() => {}} + onRefresh={() => {}} + refreshToken={refreshToken} + />, + ), + ); + +it("keeps the diff after a refresh that brings back the same page", async () => { + await render(0); + await expect.element(page.getByText("1 file", { exact: true })).toBeVisible(); + + await render(1); + + await expect + .element(page.getByText("This pull request has no file changes.")) + .not.toBeInTheDocument(); + await expect.element(page.getByText("1 file", { exact: true })).toBeVisible(); +}); diff --git a/apps/web/src/components/pullRequest/PullRequestCodeTab.tsx b/apps/web/src/components/pullRequest/PullRequestCodeTab.tsx index 751b34ecdc04..fa2cb3651154 100644 --- a/apps/web/src/components/pullRequest/PullRequestCodeTab.tsx +++ b/apps/web/src/components/pullRequest/PullRequestCodeTab.tsx @@ -145,6 +145,25 @@ const REPLACE_FILE_COUNTS_CSS = ` const NO_SLICES: ReadonlyArray = []; +function sameSlice(slice: DiffSlice, page: DiffSlice) { + return ( + slice.cursor === page.cursor && + slice.patch === page.patch && + slice.truncated === page.truncated && + slice.nextCursor === page.nextCursor && + slice.omittedFileStats.length === page.omittedFileStats.length && + slice.omittedFileStats.every((file, index) => { + const refreshed = page.omittedFileStats[index]; + return ( + refreshed !== undefined && + refreshed.path === file.path && + refreshed.additions === file.additions && + refreshed.deletions === file.deletions + ); + }) + ); +} + // Each slice is parsed once per cache key and the parse lives as long as the slice does, so a // slice arriving costs one parse rather than one per slice already on screen. const sliceParses = new WeakMap>(); @@ -336,55 +355,31 @@ function PullRequestCodeTab({ }), ); // Pages accumulate into slices as each answer arrives, so the slice list is adjusted while - // rendering the answer rather than one commit later. - const [appliedPage, setAppliedPage] = useState<{ - readonly data: typeof diffQuery.data; - readonly cursor: string | null; - readonly scopeKey: string; - } | null>(null); + // rendering the answer rather than one commit later. Whether an answer is in is read from the + // slices: an equal refetch keeps the answer's identity, so it cannot signal a reset's refill. const data = diffQuery.data; - if ( - data !== null && - (appliedPage?.data !== data || - appliedPage.cursor !== cursor || - appliedPage.scopeKey !== scopeKey) - ) { - setAppliedPage({ data, cursor, scopeKey }); + const page: DiffSlice | null = + data === null + ? null + : { + cursor, + patch: data.patch, + truncated: data.truncated, + nextCursor: data.nextCursor, + omittedFileStats: data.omittedFileStats ?? [], + }; + if (page !== null && !loadedSlices.some((slice) => sameSlice(slice, page))) { setSliceState((previous) => { const slices = previous.key === scopeKey ? previous.slices : NO_SLICES; - const next = { - cursor, - patch: data.patch, - truncated: data.truncated, - nextCursor: data.nextCursor, - omittedFileStats: data.omittedFileStats ?? [], - }; const index = slices.findIndex((slice) => slice.cursor === cursor); if (index === -1) { - return { key: scopeKey, cursor, slices: [...slices, next] }; + return { key: scopeKey, cursor, slices: [...slices, page] }; } const existing = slices[index]; - if ( - existing !== undefined && - existing.patch === next.patch && - existing.truncated === next.truncated && - existing.nextCursor === next.nextCursor && - existing.omittedFileStats.length === next.omittedFileStats.length && - existing.omittedFileStats.every((file, index) => { - const refreshed = next.omittedFileStats[index]; - return ( - refreshed !== undefined && - refreshed.path === file.path && - refreshed.additions === file.additions && - refreshed.deletions === file.deletions - ); - }) - ) { - return previous; - } + if (existing !== undefined && sameSlice(existing, page)) return previous; // A page that came back different means the diff moved under the review. The slices // after it go with the replacement: their cursors were positions in the old diff. - return { key: scopeKey, cursor, slices: [...slices.slice(0, index), next] }; + return { key: scopeKey, cursor, slices: [...slices.slice(0, index), page] }; }); } const refreshFirstDiffPage = useAtomRefresh(