From 6abe5f84f8bb9744df3c1fc35710c26bb21c3219 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89mile=20R=C3=A9?= Date: Thu, 16 Jul 2026 13:12:56 +0200 Subject: [PATCH] Fix stale document viewer navigation races MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Dispose the viewer query when the alias changes so navigating between documents shows the skeleton instead of the previous document. Guard the export completion against the current id so a slow export can't overwrite a newer document's preview. Split the documents tab state into requested and loaded refs so an out-of-order or failed refetch can't leave the list showing a different tab than the toolbar. Signed-off-by: Émile Ré --- .../documents/DocumentViewerPageLoader.tsx | 7 +++-- .../src/pages/documents/DocumentsPage.tsx | 28 ++++++++++++++++--- .../pages/documents/_lib/useDocumentExport.ts | 21 +++++++++++--- 3 files changed, 46 insertions(+), 10 deletions(-) diff --git a/apps/compliance-portal/src/pages/documents/DocumentViewerPageLoader.tsx b/apps/compliance-portal/src/pages/documents/DocumentViewerPageLoader.tsx index b5f7dc792..1280408d6 100644 --- a/apps/compliance-portal/src/pages/documents/DocumentViewerPageLoader.tsx +++ b/apps/compliance-portal/src/pages/documents/DocumentViewerPageLoader.tsx @@ -28,13 +28,16 @@ import { DocumentViewerPageSkeleton } from "./DocumentViewerPageSkeleton"; export default function DocumentViewerPageLoader() { const { alias } = useParams(); - const [queryRef, loadQuery] = useQueryLoader(documentViewerPageQuery); + const [queryRef, loadQuery, disposeQuery] = useQueryLoader(documentViewerPageQuery); + // Dispose on alias change so navigating between documents shows the skeleton + // during the transition instead of the previous document's metadata/preview. useEffect(() => { if (alias) { loadQuery({ alias }); } - }, [loadQuery, alias]); + return () => disposeQuery(); + }, [loadQuery, disposeQuery, alias]); if (!queryRef) { return ; diff --git a/apps/compliance-portal/src/pages/documents/DocumentsPage.tsx b/apps/compliance-portal/src/pages/documents/DocumentsPage.tsx index 3bcee1b98..775947f2b 100644 --- a/apps/compliance-portal/src/pages/documents/DocumentsPage.tsx +++ b/apps/compliance-portal/src/pages/documents/DocumentsPage.tsx @@ -108,15 +108,35 @@ export function DocumentsPage({ queryRef }: DocumentsPageProps) { // initial preload was in flight, this reconciles by refetching rather than // showing the wrong slice. Refetch inside a transition so the toolbar and // current results stay mounted (dimmed via `isRefetching`) while it loads. - const fetchedVisibility = useRef(queryRef.variables.visibility ?? null); + // + // `requestedVisibility` de-dupes in-flight requests; `loadedVisibility` only + // advances when the *latest* refetch settles, so an out-of-order or failed + // refetch can't leave the list showing a different tab than the toolbar. + const initialVisibility = queryRef.variables.visibility ?? null; + const loadedVisibility = useRef(initialVisibility); + const requestedVisibility = useRef(initialVisibility); useEffect(() => { const target = toQueryVariables(tab).visibility ?? null; - if (target === fetchedVisibility.current) { + if (target === requestedVisibility.current) { return; } - fetchedVisibility.current = target; + requestedVisibility.current = target; startTransition(() => { - refetch(toQueryVariables(tab), { fetchPolicy: "store-or-network" }); + refetch(toQueryVariables(tab), { + fetchPolicy: "store-or-network", + onComplete: (error) => { + if (requestedVisibility.current !== target) { + // A newer tab was requested; ignore this stale settle. + return; + } + if (error) { + // Allow re-selecting this tab to retry after a failed refetch. + requestedVisibility.current = loadedVisibility.current; + return; + } + loadedVisibility.current = target; + }, + }); }); }, [refetch, tab]); diff --git a/apps/compliance-portal/src/pages/documents/_lib/useDocumentExport.ts b/apps/compliance-portal/src/pages/documents/_lib/useDocumentExport.ts index 909a0abe7..919f7c4c1 100644 --- a/apps/compliance-portal/src/pages/documents/_lib/useDocumentExport.ts +++ b/apps/compliance-portal/src/pages/documents/_lib/useDocumentExport.ts @@ -18,7 +18,7 @@ // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE // SOFTWARE. -import { useEffect, useState } from "react"; +import { useEffect, useRef, useState } from "react"; import { graphql } from "react-relay"; import { useMutation } from "#/lib/relay/useMutation"; @@ -77,28 +77,41 @@ export function useDocumentExport(kind: DocumentKind, id: string, enabled: boole setDataUri(null); } + // Track the current target so a slow export that resolves after the id + // changed cannot overwrite the preview with the previous document's bytes. + const currentId = useRef(id); + useEffect(() => { + currentId.current = id; + }, [id]); + useEffect(() => { if (!enabled || dataUri) { return; } + const apply = (targetId: string, data: string) => { + if (currentId.current === targetId) { + setDataUri(data); + } + }; + switch (kind) { case "Document": exportDocument({ variables: { input: { documentId: id } }, - onCompleted: response => setDataUri(response.exportDocumentPDF.data), + onCompleted: response => apply(id, response.exportDocumentPDF.data), }).catch(() => {}); break; case "TrustCenterFile": exportFile({ variables: { input: { trustCenterFileId: id } }, - onCompleted: response => setDataUri(response.exportTrustCenterFile.data), + onCompleted: response => apply(id, response.exportTrustCenterFile.data), }).catch(() => {}); break; case "AuditReport": exportReport({ variables: { input: { reportId: id } }, - onCompleted: response => setDataUri(response.exportReportPDF.data), + onCompleted: response => apply(id, response.exportReportPDF.data), }).catch(() => {}); break; }