From 7bcb0987f9b2d1e2846463adb0cfa797a3bb04e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Tue, 23 Dec 2025 14:12:54 +0000 Subject: [PATCH 1/2] web: disable TanStack Query structural sharing Structural sharing is disabled to ensure QueryCache subscriptions receive 'updated' events even when refetched data is identical to previous data. With structural sharing enabled (default), TanStack Query reuses the previous data reference when new data is deeply equal, preventing the QueryCache from emitting update events. This makes it impossible to detect refetches via subscriptions when data hasn't changed. The custom useTrackQueriesRefetch hook (used by ProgressBackdrop) relies on these events to detect when queries have been refetched, enabling it to unblock the UI at the precise moment after a progress signal completes. Without these events, the UI cannot reliably determine when to unblock. The performance impact of disabling this optimization is expected to be minimal because: * Identical data responses are relatively rare in practice * React's reconciliation efficiently skips DOM updates when rendered output is identical, even if components re-render. Any heavy computations can be memoized to avoid re-execution when data hasn't changed Several alternatives were evaluated and rejected as unnecessarily complex for the value provided: * Adding artificial timestamps to JSON responses (which achieves nearly the same result as disabling this optimization, but at the cost of breaking types and polluting the data model) * Reverting useTrackQueriesRefetch to query observer pattern * Manually configuring notifyOnChangeProps globally or per-query * Implementing separate tracker queries alongside data queries This approach simply trades one render optimization for simpler, more reliable event detection. --- web/src/context/app.tsx | 54 ++++++++++++++++++++++++++++++++++++++--- 1 file changed, 51 insertions(+), 3 deletions(-) diff --git a/web/src/context/app.tsx b/web/src/context/app.tsx index c4afb90bf3..67279a255e 100644 --- a/web/src/context/app.tsx +++ b/web/src/context/app.tsx @@ -24,7 +24,12 @@ import React from "react"; import { InstallerClientProvider } from "./installer"; import { InstallerL10nProvider } from "./installerL10n"; import { StorageUiStateProvider } from "./storage-ui-state"; -import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { + DefaultOptions, + MutationOptions, + QueryClient, + QueryClientProvider, +} from "@tanstack/react-query"; import { localConnection } from "~/utils"; // Determines which "network mode" should Tanstack Query use @@ -40,13 +45,56 @@ const networkMode = (): "always" | "online" => { return localConnection() ? "always" : "online"; }; -const sharedOptions = { +const sharedOptions: DefaultOptions & MutationOptions = { networkMode: networkMode(), }; const queryClient = new QueryClient({ defaultOptions: { - queries: sharedOptions, + queries: { + ...sharedOptions, + /** + * Structural sharing is disabled to ensure QueryCache subscriptions + * receive 'updated' events even when refetched data is identical to + * previous data. + * + * With structural sharing enabled (default), TanStack Query reuses the + * previous data reference when new data is deeply equal, preventing the + * QueryCache from emitting update events. This makes it impossible to + * detect refetches via subscriptions when data hasn't changed. + * + * The custom useTrackQueriesRefetch hook (used by ProgressBackdrop) + * relies on these events to detect when queries have been refetched, + * enabling it to unblock the UI at the right moment when a progress has + * finished and data for rendering the interface is ready. Without these + * events, the UI cannot reliably determine when to unblock. + * + * The performance impact of disabling this optimization is expected to be + * minimal because: + * + * * Identical data responses are relatively rare in practice + * * React's reconciliation efficiently skips DOM updates when rendered + * output is identical, even if components re-render. Any heavy + * computations can be memoized to avoid re-execution when data hasn't + * changed + * + * Several alternatives were evaluated and rejected as unnecessarily complex + * for the value provided: + * + * * Adding artificial timestamps to JSON responses (which achieves + * nearly the same result as disabling this optimization, but at the + * cost of breaking types and polluting the data model) + * * Reverting useTrackQueriesRefetch to query observer pattern + * * Manually configuring notifyOnChangeProps globally or per-query + * * Implementing separate tracker queries alongside data queries + * + * This approach simply trades one render optimization for simpler, more + * reliable event detection. + * + * @see https://tanstack.com/query/v5/docs/framework/react/guides/render-optimizations + */ + structuralSharing: false, + }, mutations: sharedOptions, }, }); From cf9da48a8e883ede890496cdab6582c2a9027a65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Tue, 23 Dec 2025 14:32:45 +0000 Subject: [PATCH 2/2] web: fix ProgressBackdrop test Delete a no longer suitable test for ProgressBackdrop after making `scope` mandatory in previous commits. To cover a similar use case, add two simple unit tests at core/Page after conditionally mounting the ProgressBackdrop when progress prop is present --- web/src/components/core/Page.test.tsx | 16 ++++++++++++++++ web/src/components/core/Page.tsx | 2 +- .../components/core/ProgressBackdrop.test.tsx | 7 ------- 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/web/src/components/core/Page.test.tsx b/web/src/components/core/Page.test.tsx index 78a7c47b8f..85646a0fed 100644 --- a/web/src/components/core/Page.test.tsx +++ b/web/src/components/core/Page.test.tsx @@ -40,6 +40,8 @@ jest.mock("~/components/product/ProductRegistrationAlert", () => () => (
ProductRegistrationAlertMock
)); +jest.mock("~/components/core/ProgressBackdrop", () => () =>
ProgressBackdropMock
); + const mockUseTrackQueriesRefetch = jest.mocked(useTrackQueriesRefetch); describe("Page", () => { @@ -74,6 +76,20 @@ describe("Page", () => { screen.getByRole("heading", { name: "The Page Component" }); }); + describe("when no progress prop is provided", () => { + it("does not mount ProgressBackdrop", () => { + installerRender(); + expect(screen.queryByText("ProgressBackdropMock")).toBeNull(); + }); + }); + + describe("when progress prop is provided", () => { + it("mounts ProgressBackdrop", () => { + installerRender(); + screen.getByText("ProgressBackdropMock"); + }); + }); + describe("Page.Actions", () => { it("renders a footer sticky to bottom", () => { installerRender( diff --git a/web/src/components/core/Page.tsx b/web/src/components/core/Page.tsx index 4bfef41336..e5d2629517 100644 --- a/web/src/components/core/Page.tsx +++ b/web/src/components/core/Page.tsx @@ -350,7 +350,7 @@ const Page = ({ return ( {children} - + {progress && } ); }; diff --git a/web/src/components/core/ProgressBackdrop.test.tsx b/web/src/components/core/ProgressBackdrop.test.tsx index 57415057ca..0f2db8de4f 100644 --- a/web/src/components/core/ProgressBackdrop.test.tsx +++ b/web/src/components/core/ProgressBackdrop.test.tsx @@ -48,13 +48,6 @@ describe("ProgressBackdrop", () => { jest.clearAllMocks(); }); - describe("when no progress scope is provided", () => { - it("does not render the backdrop", () => { - installerRender(); - expect(screen.queryByRole("alert")).toBeNull(); - }); - }); - describe("when progress scope is provided but no matching progress exists", () => { it("does not render the backdrop", () => { installerRender();