mirror of
https://github.com/Stirling-Tools/Stirling-PDF.git
synced 2026-09-02 21:03:34 +03:00
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`.
This commit is contained in:
@@ -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 (
|
||||
<QueryClientProvider client={queryClient}>
|
||||
<ThemeProvider>
|
||||
|
||||
@@ -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(
|
||||
<QueryClientProvider client={client}>
|
||||
<HomeConsumers />
|
||||
@@ -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(
|
||||
<QueryClientProvider client={client}>
|
||||
<HomeConsumers />
|
||||
|
||||
@@ -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 <span>{data ?? "pending"}</span>;
|
||||
}
|
||||
|
||||
function mountRoute() {
|
||||
const Wrapper = ({ children }: { children: ReactNode }) => (
|
||||
<QueryClientProvider client={getPortalQueryClient()}>
|
||||
{children}
|
||||
</QueryClientProvider>
|
||||
);
|
||||
return render(
|
||||
<Wrapper>
|
||||
<PortalRoute />
|
||||
</Wrapper>,
|
||||
);
|
||||
}
|
||||
|
||||
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);
|
||||
});
|
||||
});
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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");
|
||||
|
||||
|
||||
@@ -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");
|
||||
|
||||
Reference in New Issue
Block a user