From 8eead4bcd1386ca7591323ea1f0de6f6d6ab0644 Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Thu, 28 Aug 2025 14:29:33 +0200 Subject: [PATCH 1/4] Ensure that `PreloadedQueryRef` instances are unsubscribed when garbage collected --- .changeset/clever-students-guess.md | 5 + package.json | 4 + .../__tests__/createQueryPreloader.test.tsx | 98 ++++++++++++++++++- .../query-preloader/createQueryPreloader.ts | 55 ++++++++++- .../polyfilled/FinalizationRegistry.ts | 55 +++++++++++ .../__tests__/FinalizationRegistry.test.ts | 84 ++++++++++++++++ .../internal/polyfilled/index.react-native.ts | 1 + src/utilities/internal/polyfilled/index.ts | 2 + 8 files changed, 302 insertions(+), 2 deletions(-) create mode 100644 .changeset/clever-students-guess.md create mode 100644 src/utilities/internal/polyfilled/FinalizationRegistry.ts create mode 100644 src/utilities/internal/polyfilled/__tests__/FinalizationRegistry.test.ts create mode 100644 src/utilities/internal/polyfilled/index.react-native.ts create mode 100644 src/utilities/internal/polyfilled/index.ts diff --git a/.changeset/clever-students-guess.md b/.changeset/clever-students-guess.md new file mode 100644 index 00000000000..2ac7b4163e5 --- /dev/null +++ b/.changeset/clever-students-guess.md @@ -0,0 +1,5 @@ +--- +"@apollo/client": patch +--- + +Ensure that `PreloadedQueryRef` instances are unsubscribed when garbage collected diff --git a/package.json b/package.json index 814443231dc..a164bdbe61c 100644 --- a/package.json +++ b/package.json @@ -66,6 +66,10 @@ "production": "./src/utilities/internal/index.production.ts", "default": "./src/utilities/internal/index.ts" }, + "./utilities/polyfilled": { + "react-native": "./src/utilities/internal/polyfilled/index.react-native.ts", + "default": "./src/utilities/internal/polyfilled/index.ts" + }, "./utilities/internal/globals": "./src/utilities/internal/globals/index.ts", "./utilities/subscriptions/relay": "./src/utilities/subscriptions/relay/index.ts", "./utilities/invariant": { diff --git a/src/react/query-preloader/__tests__/createQueryPreloader.test.tsx b/src/react/query-preloader/__tests__/createQueryPreloader.test.tsx index f05defde306..afb57b2784d 100644 --- a/src/react/query-preloader/__tests__/createQueryPreloader.test.tsx +++ b/src/react/query-preloader/__tests__/createQueryPreloader.test.tsx @@ -1,4 +1,4 @@ -import { act, screen } from "@testing-library/react"; +import { act, screen, waitFor } from "@testing-library/react"; import { createRenderStream, disableActEnvironment, @@ -2007,6 +2007,102 @@ test("does not mask results by default", async () => { } }); +describe("PreloadedQueryRef` disposal", () => { + test("when the `PreloadedQueryRef` is disposed of, the ObservableQuery is unsubscribed", async () => { + const { query, mocks } = setupVariablesCase(); + const client = new ApolloClient({ + cache: new InMemoryCache(), + link: new MockLink(mocks), + }); + const preloadQuery = createQueryPreloader(client); + + let queryRef: PreloadedQueryRef | null = preloadQuery(query, { + variables: { id: "1" }, + }); + const internalQueryRef = unwrapQueryRef(queryRef)!; + + expect(internalQueryRef.observable.hasObservers()).toBe(true); + expect(internalQueryRef["softReferences"]).toBe(1); + queryRef = null; + + await waitFor(() => { + global.gc!(); + expect(internalQueryRef.observable.hasObservers()).toBe(false); + }); + expect(internalQueryRef["softReferences"]).toBe(0); + }); + + test("when the `PreloadedQueryRef` is disposed of, while the initial request is still ongoing the ObservableQuery stays subscribed to", async () => { + const { query } = setupVariablesCase(); + const link = new MockSubscriptionLink(); + const client = new ApolloClient({ + cache: new InMemoryCache(), + link, + }); + const preloadQuery = createQueryPreloader(client); + + let queryRef: PreloadedQueryRef | null = preloadQuery(query, { + variables: { id: "1" }, + }); + const internalQueryRef = unwrapQueryRef(queryRef)!; + + expect(internalQueryRef.observable.hasObservers()).toBe(true); + expect(internalQueryRef["softReferences"]).toBe(1); + queryRef = null; + + await expect( + waitFor(() => { + global.gc!(); + expect(internalQueryRef.observable.hasObservers()).toBe(false); + }) + ).rejects.toThrow(); + expect(internalQueryRef["softReferences"]).toBe(1); + + link.simulateResult( + { + result: { + data: { + character: { __typename: "Character", id: "1", name }, + }, + }, + }, + true + ); + + await waitFor(() => { + global.gc!(); + expect(internalQueryRef.observable.hasObservers()).toBe(false); + }); + expect(internalQueryRef["softReferences"]).toBe(0); + }); + + test("when retained by a component, the soft retain lets go", async () => { + const { query, mocks } = setupVariablesCase(); + const client = new ApolloClient({ + cache: new InMemoryCache(), + link: new MockLink(mocks), + }); + const preloadQuery = createQueryPreloader(client); + + const queryRef = preloadQuery(query, { + variables: { id: "1" }, + }); + const internalQueryRef = unwrapQueryRef(queryRef)!; + + expect(internalQueryRef["softReferences"]).toBe(1); + + using _disabledAct = disableActEnvironment(); + const { renderStream } = await renderDefaultTestApp({ + client, + queryRef, + }); + await renderStream.takeRender(); + await renderStream.takeRender(); + + expect(internalQueryRef["softReferences"]).toBe(0); + }); +}); + describe.skip("type tests", () => { const client = new ApolloClient({ cache: new InMemoryCache(), diff --git a/src/react/query-preloader/createQueryPreloader.ts b/src/react/query-preloader/createQueryPreloader.ts index 5866d0ce8ad..71eaeb7045e 100644 --- a/src/react/query-preloader/createQueryPreloader.ts +++ b/src/react/query-preloader/createQueryPreloader.ts @@ -197,10 +197,12 @@ const _createQueryPreloader: typeof createQueryPreloader = (client) => { } ); - return wrapQueryRef(queryRef) as unknown as PreloadedQueryRef< + const wrapped = wrapQueryRef(queryRef) as unknown as PreloadedQueryRef< TData, TVariables >; + softRetainWhileReferenced(wrapped, queryRef); + return wrapped; } return Object.assign(preloadQuery, { @@ -212,3 +214,54 @@ const _createQueryPreloader: typeof createQueryPreloader = (client) => { }, }); }; + +/** + * Soft-retains the underlying `InternalQueryReference` while the `PreloadedQueryRef` + * is still reachable. + * When the `PreloadedQueryRef` is garbage collected, the soft retain is + * disposed of, but only after the initial query has finished loading. + * Once the `InternalQueryReference` is properly retained, the check for garbage + * collection is unregistered and the soft retain is disposed of immediately. + */ +// this is an individual function to avoid closing over any values more than necessary +function softRetainWhileReferenced( + wrapped: PreloadedQueryRef, + queryRef: InternalQueryReference +) { + const { softDispose, delayedSoftDispose } = getCleanup(queryRef); + registry.register(wrapped, delayedSoftDispose, queryRef); + // This will unregister the cleanup from the finalization registry when + // the queryRef is properly retained. + // This is mostly done to keep the FinalizationRegistry from holding too many + // cleanup functions, as our React Native polyfill has to iterate all of them regularly. + queryRef.retain = unregisterOnRetain(queryRef.retain, softDispose); +} + +// this is an individual function to avoid closing over any values more than necessary +function unregisterOnRetain( + originalRetain: InternalQueryReference["retain"], + softDispose: () => void +) { + return function ( + this: InternalQueryReference, + ...args: Parameters + ) { + registry.unregister(this); + const dispose = originalRetain.apply(this, args); + softDispose(); + return dispose; + }; +} + +// this is an individual function to avoid closing over any values more than necessary +function getCleanup(queryRef: InternalQueryReference) { + const softDispose = queryRef.softRetain(); + const initialPromise = queryRef.promise; + + return { + softDispose, + delayedSoftDispose: () => initialPromise.finally(softDispose), + }; +} + +const registry = new FinalizationRegistry<() => void>((cleanup) => cleanup()); diff --git a/src/utilities/internal/polyfilled/FinalizationRegistry.ts b/src/utilities/internal/polyfilled/FinalizationRegistry.ts new file mode 100644 index 00000000000..88c323c560d --- /dev/null +++ b/src/utilities/internal/polyfilled/FinalizationRegistry.ts @@ -0,0 +1,55 @@ +import { invariant } from "@apollo/client/utilities/invariant"; + +interface Entry { + targetRef: WeakRef; + value: T; +} + +/** + * An approximation of `FinalizationRegistry` based on `WeakRef`. + * Checks every 500ms if registered values have been garbage collected. + */ +export const FinalizationRegistry: typeof globalThis.FinalizationRegistry = class FinalizationRegistry< + T, +> { + private intervalLength = 500; + private callback: (value: T) => void; + private references = new Set>(); + private unregisterTokens = new WeakMap>(); + private interval: ReturnType | null = null; + constructor(callback: (value: T) => void) { + this.callback = callback; + this.handler = this.handler.bind(this); + } + handler() { + if (this.references.size === 0) { + clearInterval(this.interval!); + this.interval = null; + return; + } + this.references.forEach((entry) => { + if (entry.targetRef.deref() === undefined) { + this.references.delete(entry); + this.callback(entry.value); + } + }); + } + register(target: WeakKey, value: T, unregisterToken?: WeakKey): void { + const entry = { targetRef: new WeakRef(target), value }; + this.references.add(entry); + if (unregisterToken) { + // some simplifications here as it's an internal polyfill + // we don't allow the same unregisterToken to be reused + invariant(!this.unregisterTokens.has(unregisterToken)); + this.unregisterTokens.set(unregisterToken, entry); + } + if (!this.interval) { + this.interval = setInterval(this.handler, this.intervalLength); + } + } + unregister(unregisterToken: WeakKey): boolean { + this.references.delete(this.unregisterTokens.get(unregisterToken)!); + return this.unregisterTokens.delete(unregisterToken); + } + [Symbol.toStringTag] = "FinalizationRegistry" as const; +}; diff --git a/src/utilities/internal/polyfilled/__tests__/FinalizationRegistry.test.ts b/src/utilities/internal/polyfilled/__tests__/FinalizationRegistry.test.ts new file mode 100644 index 00000000000..2f987ee5ac7 --- /dev/null +++ b/src/utilities/internal/polyfilled/__tests__/FinalizationRegistry.test.ts @@ -0,0 +1,84 @@ +import { waitFor } from "@testing-library/react"; + +// eslint-disable-next-line local-rules/no-relative-imports +import { FinalizationRegistry } from "../FinalizationRegistry.js"; + +test("register", async () => { + const cleanedUp: number[] = []; + const registry = new FinalizationRegistry((value) => { + cleanedUp.push(value); + }); + // @ts-ignore we want to speed this up a bit + registry["intervalLength"] = 1; + + let obj1: {} | null = {}; + let obj2: {} | null = {}; + let obj3: {} | null = {}; + + registry.register(obj1, 1); + registry.register(obj2, 2); + registry.register(obj3, 3); + + expect(cleanedUp).toStrictEqual([]); + + obj1 = null; + await waitFor(() => { + global.gc!(); + expect(cleanedUp).toStrictEqual([1]); + }); + + obj3 = null; + await waitFor(() => { + global.gc!(); + expect(cleanedUp).toStrictEqual([1, 3]); + }); + + obj2 = null; + await waitFor(() => { + global.gc!(); + expect(cleanedUp).toStrictEqual([1, 3, 2]); + }); +}); + +test("unregister", async () => { + const cleanedUp: number[] = []; + const registry = new FinalizationRegistry((value) => { + cleanedUp.push(value); + }); + // @ts-ignore we want to speed this up a bit + registry["intervalLength"] = 1; + + let obj1: {} | null = {}; + const token1 = {}; + let obj2: {} | null = {}; + const token2 = {}; + let obj3: {} | null = {}; + const token3 = {}; + + registry.register(obj1, 1, token1); + registry.register(obj2, 2, token2); + registry.register(obj3, 3, token3); + + expect(cleanedUp).toStrictEqual([]); + + obj1 = null; + await waitFor(() => { + global.gc!(); + expect(cleanedUp).toStrictEqual([1]); + }); + + registry.unregister(token3); + obj3 = null; + await expect( + waitFor(() => { + global.gc!(); + expect(cleanedUp).toStrictEqual([1, 3]); + }) + ).rejects.toThrow(); + + obj2 = null; + await waitFor(() => { + global.gc!(); + expect(cleanedUp).toStrictEqual([1, 2]); + }); +}); diff --git a/src/utilities/internal/polyfilled/index.react-native.ts b/src/utilities/internal/polyfilled/index.react-native.ts new file mode 100644 index 00000000000..8b0e1d50f90 --- /dev/null +++ b/src/utilities/internal/polyfilled/index.react-native.ts @@ -0,0 +1 @@ +export { FinalizationRegistry } from "./FinalizationRegistry.js"; diff --git a/src/utilities/internal/polyfilled/index.ts b/src/utilities/internal/polyfilled/index.ts new file mode 100644 index 00000000000..a63c1e74935 --- /dev/null +++ b/src/utilities/internal/polyfilled/index.ts @@ -0,0 +1,2 @@ +const F = FinalizationRegistry; +export { F as FinalizationRegistry }; From 0a9d568c20d993080a4fee8723a2fbd6851e5dad Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Thu, 28 Aug 2025 14:34:08 +0200 Subject: [PATCH 2/4] fixup --- package.json | 2 +- src/react/query-preloader/createQueryPreloader.ts | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/package.json b/package.json index a164bdbe61c..f49a321225f 100644 --- a/package.json +++ b/package.json @@ -66,7 +66,7 @@ "production": "./src/utilities/internal/index.production.ts", "default": "./src/utilities/internal/index.ts" }, - "./utilities/polyfilled": { + "./utilities/internal/polyfilled": { "react-native": "./src/utilities/internal/polyfilled/index.react-native.ts", "default": "./src/utilities/internal/polyfilled/index.ts" }, diff --git a/src/react/query-preloader/createQueryPreloader.ts b/src/react/query-preloader/createQueryPreloader.ts index 71eaeb7045e..4bfe227350c 100644 --- a/src/react/query-preloader/createQueryPreloader.ts +++ b/src/react/query-preloader/createQueryPreloader.ts @@ -19,6 +19,7 @@ import type { NoInfer, VariablesOption, } from "@apollo/client/utilities/internal"; +import { FinalizationRegistry } from "@apollo/client/utilities/internal/polyfilled"; import { wrapHook } from "../hooks/internal/index.js"; From 9d0f4ea92d400951da66d7102ec65bfd1a9d964b Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Thu, 28 Aug 2025 14:35:36 +0200 Subject: [PATCH 3/4] rename to `ponyfills` --- package.json | 6 +++--- src/react/query-preloader/createQueryPreloader.ts | 2 +- .../{polyfilled => ponyfills}/FinalizationRegistry.ts | 0 .../__tests__/FinalizationRegistry.test.ts | 0 .../{polyfilled => ponyfills}/index.react-native.ts | 0 src/utilities/internal/{polyfilled => ponyfills}/index.ts | 0 6 files changed, 4 insertions(+), 4 deletions(-) rename src/utilities/internal/{polyfilled => ponyfills}/FinalizationRegistry.ts (100%) rename src/utilities/internal/{polyfilled => ponyfills}/__tests__/FinalizationRegistry.test.ts (100%) rename src/utilities/internal/{polyfilled => ponyfills}/index.react-native.ts (100%) rename src/utilities/internal/{polyfilled => ponyfills}/index.ts (100%) diff --git a/package.json b/package.json index f49a321225f..b7763b12197 100644 --- a/package.json +++ b/package.json @@ -66,9 +66,9 @@ "production": "./src/utilities/internal/index.production.ts", "default": "./src/utilities/internal/index.ts" }, - "./utilities/internal/polyfilled": { - "react-native": "./src/utilities/internal/polyfilled/index.react-native.ts", - "default": "./src/utilities/internal/polyfilled/index.ts" + "./utilities/internal/ponyfills": { + "react-native": "./src/utilities/internal/ponyfills/index.react-native.ts", + "default": "./src/utilities/internal/ponyfills/index.ts" }, "./utilities/internal/globals": "./src/utilities/internal/globals/index.ts", "./utilities/subscriptions/relay": "./src/utilities/subscriptions/relay/index.ts", diff --git a/src/react/query-preloader/createQueryPreloader.ts b/src/react/query-preloader/createQueryPreloader.ts index 4bfe227350c..5899a41143b 100644 --- a/src/react/query-preloader/createQueryPreloader.ts +++ b/src/react/query-preloader/createQueryPreloader.ts @@ -19,7 +19,7 @@ import type { NoInfer, VariablesOption, } from "@apollo/client/utilities/internal"; -import { FinalizationRegistry } from "@apollo/client/utilities/internal/polyfilled"; +import { FinalizationRegistry } from "@apollo/client/utilities/internal/ponyfills"; import { wrapHook } from "../hooks/internal/index.js"; diff --git a/src/utilities/internal/polyfilled/FinalizationRegistry.ts b/src/utilities/internal/ponyfills/FinalizationRegistry.ts similarity index 100% rename from src/utilities/internal/polyfilled/FinalizationRegistry.ts rename to src/utilities/internal/ponyfills/FinalizationRegistry.ts diff --git a/src/utilities/internal/polyfilled/__tests__/FinalizationRegistry.test.ts b/src/utilities/internal/ponyfills/__tests__/FinalizationRegistry.test.ts similarity index 100% rename from src/utilities/internal/polyfilled/__tests__/FinalizationRegistry.test.ts rename to src/utilities/internal/ponyfills/__tests__/FinalizationRegistry.test.ts diff --git a/src/utilities/internal/polyfilled/index.react-native.ts b/src/utilities/internal/ponyfills/index.react-native.ts similarity index 100% rename from src/utilities/internal/polyfilled/index.react-native.ts rename to src/utilities/internal/ponyfills/index.react-native.ts diff --git a/src/utilities/internal/polyfilled/index.ts b/src/utilities/internal/ponyfills/index.ts similarity index 100% rename from src/utilities/internal/polyfilled/index.ts rename to src/utilities/internal/ponyfills/index.ts From 54bf3552085c96e5680addf383fc5d16e415ff16 Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Thu, 28 Aug 2025 14:53:54 +0200 Subject: [PATCH 4/4] chores --- .../api-report-utilities_internal_ponyfills.api.md | 12 ++++++++++++ src/__tests__/__snapshots__/exports.ts.snap | 6 ++++++ src/__tests__/exports.ts | 5 +++++ .../ponyfills/__tests__/FinalizationRegistry.test.ts | 2 +- 4 files changed, 24 insertions(+), 1 deletion(-) create mode 100644 .api-reports/api-report-utilities_internal_ponyfills.api.md diff --git a/.api-reports/api-report-utilities_internal_ponyfills.api.md b/.api-reports/api-report-utilities_internal_ponyfills.api.md new file mode 100644 index 00000000000..acbeb7f69a3 --- /dev/null +++ b/.api-reports/api-report-utilities_internal_ponyfills.api.md @@ -0,0 +1,12 @@ +## API Report File for "@apollo/client" + +> Do not edit this file. It is a report generated by [API Extractor](https://api-extractor.com/). + +```ts + +// @public (undocumented) +export const FinalizationRegistry: FinalizationRegistryConstructor; + +// (No @packageDocumentation comment for this package) + +``` diff --git a/src/__tests__/__snapshots__/exports.ts.snap b/src/__tests__/__snapshots__/exports.ts.snap index c7343506bff..9c7aa805dd9 100644 --- a/src/__tests__/__snapshots__/exports.ts.snap +++ b/src/__tests__/__snapshots__/exports.ts.snap @@ -480,6 +480,12 @@ Array [ ] `; +exports[`exports of public entry points @apollo/client/utilities/internal/ponyfills 1`] = ` +Array [ + "FinalizationRegistry", +] +`; + exports[`exports of public entry points @apollo/client/utilities/invariant 1`] = ` Array [ "ApolloErrorMessageHandler", diff --git a/src/__tests__/exports.ts b/src/__tests__/exports.ts index a31b07ab5c9..474f4acf5e4 100644 --- a/src/__tests__/exports.ts +++ b/src/__tests__/exports.ts @@ -42,6 +42,7 @@ import * as utilities from "@apollo/client/utilities"; import * as utilitiesEnvironment from "@apollo/client/utilities/environment"; import * as utilitiesInternal from "@apollo/client/utilities/internal"; import * as utilitiesInternalGlobals from "@apollo/client/utilities/internal/globals"; +import * as utilitiesInternalPonyfills from "@apollo/client/utilities/internal/ponyfills"; import * as utilitiesInvariant from "@apollo/client/utilities/invariant"; import * as v4_migration from "@apollo/client/v4-migration"; @@ -104,6 +105,10 @@ describe("exports of public entry points", () => { check("@apollo/client/utilities", utilities); check("@apollo/client/utilities/internal", utilitiesInternal); check("@apollo/client/utilities/internal/globals", utilitiesInternalGlobals); + check( + "@apollo/client/utilities/internal/ponyfills", + utilitiesInternalPonyfills + ); check("@apollo/client/utilities/invariant", utilitiesInvariant); check("@apollo/client/utilities/environment", utilitiesEnvironment); check("@apollo/client/v4-migration", v4_migration); diff --git a/src/utilities/internal/ponyfills/__tests__/FinalizationRegistry.test.ts b/src/utilities/internal/ponyfills/__tests__/FinalizationRegistry.test.ts index 2f987ee5ac7..a6443df37cd 100644 --- a/src/utilities/internal/ponyfills/__tests__/FinalizationRegistry.test.ts +++ b/src/utilities/internal/ponyfills/__tests__/FinalizationRegistry.test.ts @@ -1,6 +1,6 @@ import { waitFor } from "@testing-library/react"; -// eslint-disable-next-line local-rules/no-relative-imports +// eslint-disable-next-line import { FinalizationRegistry } from "../FinalizationRegistry.js"; test("register", async () => {