From d55d8acbfa5f56174bf7ee00b7c6f53944180099 Mon Sep 17 00:00:00 2001 From: ConnorYoh <40631091+ConnorYoh@users.noreply.github.com> Date: Tue, 1 Sep 2026 08:56:35 +0000 Subject: [PATCH] fix(portal): keep the processor's cache across a trip to the editor (#7729) # Description of Changes ## The problem The portal's query client was created per mount: ```ts const [queryClient] = useState(createPortalQueryClient); ``` The portal is a route (`/processor/*`, a lazy element), and the switch to the editor is a client-side `navigate()`. So leaving the processor unmounts `PortalApp`, the client goes with the component, and the cache goes with the client. Coming back refetches everything, whether or not anything changed: four requests for the Users page alone (roster, grants, teams, auth config), and 21 `useQuery` sites across the portal. The editor's client sits above the router in `AppProviders` and survives the same trip. The round trip only ever cost in one direction. ## The fix The module already kept the instance in a module-level slot so `tryGetPortalQueryClient()` could find it. It just replaced it on every mount instead of reusing it, so the change is to create it lazily and hand out the same one: ```ts export function getPortalQueryClient(): QueryClient { current ??= new QueryClient({ defaultOptions: { queries: baseQueryOptions } }); return current; } ``` Still a separate instance from the editor's. The two namespace their keys apart (`["portal", ...]` against `["editor", ...]`) and invalidate independently, which this does not change. ## What this does not do `gcTime` is 5 minutes, from the shared `baseQueryOptions`. An entry with no observer is still collected on that timer, so this warms a quick trip to the editor and back, not a return after a long editing session. Raising the portal's `gcTime` is a separate decision and is not made here. ## Why it is safe **Signing out.** A cache that outlives a mount must not outlive a session, because the portal's holds the admin roster, emails and roles. Logout goes through `window.location.assign`, a full page load, so the whole JS context is discarded and no cache can survive it. Nothing in the codebase calls `queryClient.clear()` on sign-out, and nothing needs to. If logout ever becomes a client-side navigation, this needs an explicit reset, and `resetPortalQueryClient()` is the hook for it. **The one caller of the null check.** `resolveTeam` in `saas/portal/usersBackend.ts` uses `tryGetPortalQueryClient()` and falls back to a direct fetch when there is no client, which its comment describes as the unit-test path; the cache path is preferred because it honours both `staleTime` and invalidation. A longer-lived client means the preferred path is taken more often, not less. ## Testing Three tests in `queryClient.test.tsx`, and the first two fail if the client goes back to being created per call: | | | |---|---| | A remount is served from cache rather than refetching | the behaviour this changes | | Every caller gets the same instance | the mechanism | | No client is reported until the portal first mounts | the contract `resolveTeam` reads | The three existing portal caching suites called the factory expecting a fresh client per case. They now call `resetPortalQueryClient()` in a `beforeEach`, which is what keeps `sharing.test.tsx`'s "a later screen refetches nothing" case honest rather than passing on a leaked cache. `task frontend:check` passes typecheck, lint and oxfmt, and 2402 of 2404 editor tests. The two failures, `workbenchSession.test.ts` and `notificationActions.test.tsx`, are untouched here and fail the same way on `main`. --- frontend/editor/src/portal/PortalApp.tsx | 6 +- .../src/portal/queries/sharing.test.tsx | 12 +++- .../editor/src/portal/queryClient.test.tsx | 64 +++++++++++++++++++ frontend/editor/src/portal/queryClient.ts | 27 ++++++-- .../src/portal/views/Users.caching.test.tsx | 12 +++- .../src/portal/views/teamMyCache.saas.test.ts | 10 ++- 6 files changed, 116 insertions(+), 15 deletions(-) create mode 100644 frontend/editor/src/portal/queryClient.test.tsx diff --git a/frontend/editor/src/portal/PortalApp.tsx b/frontend/editor/src/portal/PortalApp.tsx index 3122016436..9109946929 100644 --- a/frontend/editor/src/portal/PortalApp.tsx +++ b/frontend/editor/src/portal/PortalApp.tsx @@ -1,11 +1,11 @@ -import { useState, type ReactNode } from "react"; +import { type ReactNode } from "react"; import { QueryClientProvider } from "@tanstack/react-query"; import { PortalAuthBoundary } from "@portal/auth/PortalAuthBoundary"; import { ThemeProvider, useTheme } from "@portal/contexts/ThemeContext"; import { SuiProvider } from "@portal/theme/SuiProvider"; import { PortalProviders } from "@portal/PortalProviders"; import { ToolRegistryProvider } from "@app/contexts/ToolRegistryProvider"; -import { createPortalQueryClient } from "@portal/queryClient"; +import { getPortalQueryClient } from "@portal/queryClient"; // Reset + typography, scoped to .portal-scope below. import "@portal/theme/base.css"; @@ -30,7 +30,7 @@ function ThemedSuiProvider({ children }: { children: ReactNode }) { * self-hosted mounts the account-link layer, SaaS does not. */ export function PortalApp() { - const [queryClient] = useState(createPortalQueryClient); + const queryClient = getPortalQueryClient(); return ( diff --git a/frontend/editor/src/portal/queries/sharing.test.tsx b/frontend/editor/src/portal/queries/sharing.test.tsx index 5cfb9d1629..97e23f9244 100644 --- a/frontend/editor/src/portal/queries/sharing.test.tsx +++ b/frontend/editor/src/portal/queries/sharing.test.tsx @@ -12,7 +12,10 @@ import { render, waitFor } from "@testing-library/react"; import { QueryClientProvider } from "@tanstack/react-query"; import { setupServer } from "msw/node"; import { http, HttpResponse } from "msw"; -import { createPortalQueryClient } from "@portal/queryClient"; +import { + getPortalQueryClient, + resetPortalQueryClient, +} from "@portal/queryClient"; import { usePoliciesOverview } from "@portal/queries/policies"; import { useProcessorFlow } from "@portal/queries/processorFlow"; @@ -70,9 +73,12 @@ function PoliciesConsumer() { return null; } +// The client outlives a mount now, so each case starts from a cold one. +beforeEach(resetPortalQueryClient); + describe("portal query sharing", () => { it("in-view: multiple consumers of the same endpoints fetch each once", async () => { - const client = createPortalQueryClient(); + const client = getPortalQueryClient(); render( @@ -86,7 +92,7 @@ describe("portal query sharing", () => { }); it("cross-view: a later screen reusing the data refetches nothing", async () => { - const client = createPortalQueryClient(); + const client = getPortalQueryClient(); const home = render( diff --git a/frontend/editor/src/portal/queryClient.test.tsx b/frontend/editor/src/portal/queryClient.test.tsx new file mode 100644 index 0000000000..a750eccc0e --- /dev/null +++ b/frontend/editor/src/portal/queryClient.test.tsx @@ -0,0 +1,64 @@ +import { describe, it, expect, beforeEach, vi } from "vitest"; +import type { ReactNode } from "react"; +import { render, screen } from "@testing-library/react"; +import { QueryClientProvider, useQuery } from "@tanstack/react-query"; +import { + getPortalQueryClient, + resetPortalQueryClient, + tryGetPortalQueryClient, +} from "@portal/queryClient"; + +const fetchThing = vi.fn(async () => "loaded"); + +/** Stands in for any portal view: mounts, reads one key, unmounts with the route. */ +function PortalRoute() { + const { data } = useQuery({ + queryKey: ["portal", "thing"], + queryFn: fetchThing, + }); + return {data ?? "pending"}; +} + +function mountRoute() { + const Wrapper = ({ children }: { children: ReactNode }) => ( + + {children} + + ); + return render( + + + , + ); +} + +describe("portal query client lifetime", () => { + beforeEach(() => { + resetPortalQueryClient(); + fetchThing.mockClear(); + }); + + it("serves a remount from cache instead of refetching", async () => { + const first = mountRoute(); + await screen.findByText("loaded"); + expect(fetchThing).toHaveBeenCalledTimes(1); + + // Switching to the editor unmounts the portal route. + first.unmount(); + + mountRoute(); + // Painted from cache, not after a round trip. + expect(screen.getByText("loaded")).toBeInTheDocument(); + expect(fetchThing).toHaveBeenCalledTimes(1); + }); + + it("hands every caller the same instance", () => { + expect(getPortalQueryClient()).toBe(getPortalQueryClient()); + }); + + it("reports no client until the portal first mounts", () => { + expect(tryGetPortalQueryClient()).toBeNull(); + const client = getPortalQueryClient(); + expect(tryGetPortalQueryClient()).toBe(client); + }); +}); diff --git a/frontend/editor/src/portal/queryClient.ts b/frontend/editor/src/portal/queryClient.ts index 5c0404f17d..bdd6b3b2ec 100644 --- a/frontend/editor/src/portal/queryClient.ts +++ b/frontend/editor/src/portal/queryClient.ts @@ -3,13 +3,32 @@ import { baseQueryOptions } from "@app/query/queryClient"; let current: QueryClient | null = null; -/** Own instance, shared defaults — the portal and editor are sibling routes. */ -export function createPortalQueryClient(): QueryClient { - current = new QueryClient({ defaultOptions: { queries: baseQueryOptions } }); +/** + * One client for the session, not one per mount. The portal is a route, so + * switching to the editor unmounts it, and a per-mount client would throw the + * cache away and refetch everything on the way back. The editor's own client + * sits above the router and never pays that. + * + * Still a separate instance from the editor's: the two namespace their keys + * apart and invalidate independently. + */ +export function getPortalQueryClient(): QueryClient { + current ??= new QueryClient({ + defaultOptions: { queries: baseQueryOptions }, + }); return current; } -/** Null until the portal mounts, so resolveTeam can fall back to a direct fetch. */ +/** Null until the portal first mounts, so resolveTeam can fall back to a direct fetch. */ export function tryGetPortalQueryClient(): QueryClient | null { return current; } + +/** + * Drops the cache and the instance holding it. For tests, which need a cold + * start between cases; the app never calls it, because signing out is a full + * page load. + */ +export function resetPortalQueryClient(): void { + current = null; +} diff --git a/frontend/editor/src/portal/views/Users.caching.test.tsx b/frontend/editor/src/portal/views/Users.caching.test.tsx index 64d839ac62..a619d883b7 100644 --- a/frontend/editor/src/portal/views/Users.caching.test.tsx +++ b/frontend/editor/src/portal/views/Users.caching.test.tsx @@ -17,7 +17,10 @@ import { teamSaasHandlers, resetTeamSaasStore, } from "@portal/mocks/handlers/teamSaas"; -import { createPortalQueryClient } from "@portal/queryClient"; +import { + getPortalQueryClient, + resetPortalQueryClient, +} from "@portal/queryClient"; import { qk } from "@portal/queries/keys"; /** @@ -98,11 +101,14 @@ function renderUsers(client: QueryClient): RenderResult { ); } +// The client outlives a mount now, so each case starts from a cold one. +beforeEach(resetPortalQueryClient); + describe("Users view caching", () => { it("serves the roster from cache on remount (no refetch)", async () => { // One client across both mounts — the real app keeps it at the portal root, // above the router, for exactly this reason. - const client = createPortalQueryClient(); + const client = getPortalQueryClient(); const first = renderUsers(client); expect(await screen.findByText("leader@acme.com")).toBeInTheDocument(); @@ -116,7 +122,7 @@ describe("Users view caching", () => { }); it("collapses the SaaS /team/my call to one per mount", async () => { - const client = createPortalQueryClient(); + const client = getPortalQueryClient(); renderUsers(client); await screen.findByText("leader@acme.com"); diff --git a/frontend/editor/src/portal/views/teamMyCache.saas.test.ts b/frontend/editor/src/portal/views/teamMyCache.saas.test.ts index a4ba296535..8534624c52 100644 --- a/frontend/editor/src/portal/views/teamMyCache.saas.test.ts +++ b/frontend/editor/src/portal/views/teamMyCache.saas.test.ts @@ -10,7 +10,10 @@ import { } from "vitest"; import { setupServer } from "msw/node"; import { http, HttpResponse } from "msw"; -import { createPortalQueryClient } from "@portal/queryClient"; +import { + getPortalQueryClient, + resetPortalQueryClient, +} from "@portal/queryClient"; import { qk } from "@portal/queries/keys"; import { usersBackend } from "@app/portal/usersBackend"; @@ -68,9 +71,12 @@ beforeEach(() => { teamName = "Old name"; }); +// The client outlives a mount now, so each case starts from a cold one. +beforeEach(resetPortalQueryClient); + describe("SaaS /team/my resolution cache", () => { it("dedupes within staleTime but re-resolves after invalidation", async () => { - const client = createPortalQueryClient(); + const client = getPortalQueryClient(); // Two resolves within staleTime → one network call (the collapse). expect((await usersBackend.fetchTeams())[0]?.name).toBe("Old name");