From 3200c5eb163d33c20f5efa374a1baec04d2cfc3d Mon Sep 17 00:00:00 2001 From: Romil Bijarnia Date: Wed, 11 Nov 2026 20:31:37 +1100 Subject: [PATCH] fix(web): open pull request details when browser storage is blocked --- .../usePullRequestDetail.browser.test.tsx | 118 ++++++++++++++++++ .../pullRequest/usePullRequestDetail.ts | 13 +- .../src/hooks/useOpenPanelPullRequestUrl.ts | 8 +- apps/web/src/state/pullRequests.ts | 8 +- 4 files changed, 127 insertions(+), 20 deletions(-) create mode 100644 apps/web/src/components/pullRequest/usePullRequestDetail.browser.test.tsx diff --git a/apps/web/src/components/pullRequest/usePullRequestDetail.browser.test.tsx b/apps/web/src/components/pullRequest/usePullRequestDetail.browser.test.tsx new file mode 100644 index 0000000000000..80a21007d9b89 --- /dev/null +++ b/apps/web/src/components/pullRequest/usePullRequestDetail.browser.test.tsx @@ -0,0 +1,118 @@ +import { + EnvironmentId, + ProjectId, + SourceControlProviderKind, + type PullRequestDetailView, +} from "@t3tools/contracts"; +import { expect, it, vi } from "vite-plus/test"; + +const queryState = vi.hoisted(() => ({ data: null as PullRequestDetailView | null })); + +vi.mock(import("~/state/query"), async (original) => ({ + ...(await original()), + useEnvironmentQuery: ((query: unknown) => ({ + data: query === null ? null : queryState.data, + dataUpdatedAt: queryState.data ? 1 : 0, + isPending: query !== null && queryState.data === null, + isSuccess: query !== null && queryState.data !== null, + refresh: () => undefined, + })) as never, +})); +vi.mock(import("~/state/entities"), async (original) => ({ + ...(await original()), + useServerConfigs: () => new Map(), +})); +vi.mock(import("~/state/session"), async (original) => ({ + ...(await original()), + useEnvironmentScope: () => true, +})); +vi.mock(import("~/state/pullRequests"), async (original) => ({ + ...(await original()), + useSharedPullRequestSummary: () => null, +})); + +import { render } from "../../../test/render"; +import { usePullRequestDetail } from "./usePullRequestDetail"; + +const liveDetailFixture: PullRequestDetailView = { + provider: SourceControlProviderKind.make("github"), + projectId: ProjectId.make("project"), + projectTitle: "Project", + workspaceRoot: "/workspace", + repository: "owner/repo", + number: 1, + title: "Test pull request", + body: "Original description", + url: "https://github.com/owner/repo/pull/1", + author: { login: "author", name: null, avatarUrl: null }, + viewer: "author", + state: "open", + isDraft: false, + mergeability: "conflicting", + additions: 1, + deletions: 0, + 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: [{ name: "Unit tests", status: "success", description: null, url: null }], + 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: true, comment: false }, + }, + viewerPermissions: { + actions: [], + comment: false, + resolve: false, + verdicts: [], + requestReviewers: false, + }, +}; + +function Detail({ number = 1 }: { number?: number }) { + const { detail } = usePullRequestDetail( + EnvironmentId.make("environment"), + { projectId: ProjectId.make("project"), repository: "owner/repo", number }, + null, + ); + return

{detail?.title ?? "Loading pull request"}

; +} + +function setup() { + queryState.data = null; +} + +it("loads live PR details and changes references when cache storage is denied", async () => { + setup(); + using _storage = vi.spyOn(window, "localStorage", "get").mockImplementation(() => { + throw new DOMException("Access to storage is denied", "SecurityError"); + }); + await using screen = await render(, { + // This tests the detail-loading hook, without navigation. + router: false, + }); + await expect.element(screen.getByText("Loading pull request")).toBeVisible(); + queryState.data = liveDetailFixture; + await screen.rerender(); + await expect.element(screen.getByText(liveDetailFixture.title)).toBeVisible(); + queryState.data = null; + await screen.rerender(); + await expect.element(screen.getByText("Loading pull request")).toBeVisible(); +}); diff --git a/apps/web/src/components/pullRequest/usePullRequestDetail.ts b/apps/web/src/components/pullRequest/usePullRequestDetail.ts index 7d6ab0c7a6154..7e5301db4f106 100644 --- a/apps/web/src/components/pullRequest/usePullRequestDetail.ts +++ b/apps/web/src/components/pullRequest/usePullRequestDetail.ts @@ -15,6 +15,7 @@ import { useEnvironmentScope } from "~/state/session"; import { newestPullRequestSummary, pullRequestEnvironment, + pullRequestSnapshotStorage, pullRequestListEntryToSummary, useSharedPullRequestSummary, } from "~/state/pullRequests"; @@ -84,22 +85,14 @@ export function usePullRequestDetail( checks: liveDetail?.checks ?? [], }); const [cachedDetail, setCachedDetail] = useState(() => - readPullRequestDetailSnapshot( - typeof window === "undefined" ? undefined : window.localStorage, - environmentId, - reference, - ), + readPullRequestDetailSnapshot(pullRequestSnapshotStorage(), environmentId, reference), ); const [cachedDetailKey, setCachedDetailKey] = useState(scopeKey); if (cachedDetailKey !== scopeKey) { setCachedDetailKey(scopeKey); setCachedDetail( detailQuery.data ?? - readPullRequestDetailSnapshot( - typeof window === "undefined" ? undefined : window.localStorage, - environmentId, - reference, - ), + readPullRequestDetailSnapshot(pullRequestSnapshotStorage(), environmentId, reference), ); } else if (detailQuery.data !== null && detailQuery.data !== cachedDetail) { setCachedDetail(detailQuery.data); diff --git a/apps/web/src/hooks/useOpenPanelPullRequestUrl.ts b/apps/web/src/hooks/useOpenPanelPullRequestUrl.ts index 7be2020a804d9..d16c87d671a4c 100644 --- a/apps/web/src/hooks/useOpenPanelPullRequestUrl.ts +++ b/apps/web/src/hooks/useOpenPanelPullRequestUrl.ts @@ -8,7 +8,7 @@ import { import { projectChangeRequestUrl } from "@t3tools/client-runtime/change-request-links"; import { selectActiveRightPanelSurface, useRightPanelStore } from "../rightPanelStore"; import { useProject } from "../state/entities"; -import { pullRequestEnvironment } from "../state/pullRequests"; +import { pullRequestEnvironment, pullRequestSnapshotStorage } from "../state/pullRequests"; import { useEnvironmentQuery } from "../state/query"; import { useSupportsMultiplePullRequests } from "./useSupportsMultiplePullRequests"; @@ -61,11 +61,7 @@ export function useOpenPanelPullRequestUrl(threadRef: ScopedThreadRef | null) { // Keep all reference/project reads before the query hook to preserve caching of reference and its detail atom. const cachedDetail = reference && environmentId - ? readPullRequestDetailSnapshot( - typeof window === "undefined" ? undefined : window.localStorage, - environmentId, - reference, - ) + ? readPullRequestDetailSnapshot(pullRequestSnapshotStorage(), environmentId, reference) : null; const fallbackUrl = reference ? (requestedReference?.url ?? diff --git a/apps/web/src/state/pullRequests.ts b/apps/web/src/state/pullRequests.ts index 13d71301e2035..7120a366367aa 100644 --- a/apps/web/src/state/pullRequests.ts +++ b/apps/web/src/state/pullRequests.ts @@ -222,7 +222,7 @@ function observePullRequestSummary( } // Reading localStorage throws where the browser blocks storage; that must not break the page. -function snapshotStorage(): Storage | undefined { +export function pullRequestSnapshotStorage(): Storage | undefined { if (typeof window === "undefined") return undefined; try { return window.localStorage; @@ -366,14 +366,14 @@ export function restorePullRequestList( export function restoredPullRequestList( targets: ReadonlyArray>, ): MergedPullRequestList | null { - return restorePullRequestList(snapshotStorage(), targets); + return restorePullRequestList(pullRequestSnapshotStorage(), targets); } // Answers are stamped on arrival: older servers leave no other ordering for same-dated reads. export const pullRequestEnvironment = createPullRequestEnvironmentAtoms(connectionAtomRuntime, { list: ({ environmentId, input }, answer, registry) => Effect.map(Clock.currentTimeMillis, (observedAt) => { - rememberPullRequestListAnswer(snapshotStorage(), { environmentId, input }, answer); + rememberPullRequestListAnswer(pullRequestSnapshotStorage(), { environmentId, input }, answer); for (const entry of answer.entries) { observePullRequestSummary( registry, @@ -387,7 +387,7 @@ export const pullRequestEnvironment = createPullRequestEnvironmentAtoms(connecti Effect.map(Clock.currentTimeMillis, (at) => { // Only fresh reads seed the cold-start snapshot; a stale one could replace a newer record. if (input.allowStale === false) { - writePullRequestDetailSnapshot(snapshotStorage(), environmentId, input, detail); + writePullRequestDetailSnapshot(pullRequestSnapshotStorage(), environmentId, input, detail); } observePullRequestSummary( registry,