Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

useSuspenseQuery: remove promiseCache #11363

Merged
merged 12 commits into from
Dec 1, 2023
21 changes: 9 additions & 12 deletions .api-reports/api-report-react.md
Original file line number Diff line number Diff line change
Expand Up @@ -512,13 +512,6 @@ namespace Cache_2 {
import Fragment = DataProxy.Fragment;
}

// @public (undocumented)
type CacheKey = [
query: DocumentNode,
stringifiedVariables: string,
...queryKey: any[]
];

// @public (undocumented)
const enum CacheWriteBehavior {
// (undocumented)
Expand Down Expand Up @@ -916,10 +909,10 @@ class InternalQueryReference<TData = unknown> {
//
// (undocumented)
fetchMore(options: FetchMoreOptions<TData>): Promise<ApolloQueryResult<TData>>;
// Warning: (ae-forgotten-export) The symbol "CacheKey" needs to be exported by the entry point index.d.ts
// Warning: (ae-forgotten-export) The symbol "QueryKey" needs to be exported by the entry point index.d.ts
//
// (undocumented)
readonly key: CacheKey;
readonly key: QueryKey;
// Warning: (ae-forgotten-export) The symbol "Listener" needs to be exported by the entry point index.d.ts
//
// (undocumented)
Expand All @@ -929,7 +922,7 @@ class InternalQueryReference<TData = unknown> {
// (undocumented)
promise: Promise<ApolloQueryResult<TData>>;
// (undocumented)
promiseCache?: Map<CacheKey, Promise<ApolloQueryResult<TData>>>;
promiseCache?: Map<QueryKey, Promise<ApolloQueryResult<TData>>>;
// (undocumented)
refetch(variables: OperationVariables | undefined): Promise<ApolloQueryResult<TData>>;
// (undocumented)
Expand All @@ -945,8 +938,6 @@ interface InternalQueryReferenceOptions {
// (undocumented)
autoDisposeTimeoutMs?: number;
// (undocumented)
key: CacheKey;
// (undocumented)
onDispose?: () => void;
}

Expand Down Expand Up @@ -1537,6 +1528,12 @@ class QueryInfo {
variables?: Record<string, any>;
}

// @public (undocumented)
interface QueryKey {
// (undocumented)
__queryKey?: string;
}

// @public (undocumented)
export interface QueryLazyOptions<TVariables> {
// (undocumented)
Expand Down
21 changes: 9 additions & 12 deletions .api-reports/api-report-react_hooks.md
Original file line number Diff line number Diff line change
Expand Up @@ -488,13 +488,6 @@ namespace Cache_2 {
import Fragment = DataProxy.Fragment;
}

// @public (undocumented)
type CacheKey = [
query: DocumentNode,
stringifiedVariables: string,
...queryKey: any[]
];

// @public (undocumented)
const enum CacheWriteBehavior {
// (undocumented)
Expand Down Expand Up @@ -863,10 +856,10 @@ class InternalQueryReference<TData = unknown> {
//
// (undocumented)
fetchMore(options: FetchMoreOptions<TData>): Promise<ApolloQueryResult<TData>>;
// Warning: (ae-forgotten-export) The symbol "CacheKey" needs to be exported by the entry point index.d.ts
// Warning: (ae-forgotten-export) The symbol "QueryKey" needs to be exported by the entry point index.d.ts
//
// (undocumented)
readonly key: CacheKey;
readonly key: QueryKey;
// Warning: (ae-forgotten-export) The symbol "Listener" needs to be exported by the entry point index.d.ts
//
// (undocumented)
Expand All @@ -876,7 +869,7 @@ class InternalQueryReference<TData = unknown> {
// (undocumented)
promise: Promise<ApolloQueryResult<TData>>;
// (undocumented)
promiseCache?: Map<CacheKey, Promise<ApolloQueryResult<TData>>>;
promiseCache?: Map<QueryKey, Promise<ApolloQueryResult<TData>>>;
// (undocumented)
refetch(variables: OperationVariables | undefined): Promise<ApolloQueryResult<TData>>;
// (undocumented)
Expand All @@ -892,8 +885,6 @@ interface InternalQueryReferenceOptions {
// (undocumented)
autoDisposeTimeoutMs?: number;
// (undocumented)
key: CacheKey;
// (undocumented)
onDispose?: () => void;
}

Expand Down Expand Up @@ -1464,6 +1455,12 @@ class QueryInfo {
variables?: Record<string, any>;
}

// @public (undocumented)
interface QueryKey {
// (undocumented)
__queryKey?: string;
}

// @public (undocumented)
type QueryListener = (queryInfo: QueryInfo) => void;

Expand Down
21 changes: 9 additions & 12 deletions .api-reports/api-report.md
Original file line number Diff line number Diff line change
Expand Up @@ -502,13 +502,6 @@ class CacheGroup {
resetCaching(): void;
}

// @public (undocumented)
type CacheKey = [
query: DocumentNode,
stringifiedVariables: string,
...queryKey: any[]
];

// @public (undocumented)
const enum CacheWriteBehavior {
// (undocumented)
Expand Down Expand Up @@ -1275,10 +1268,10 @@ class InternalQueryReference<TData = unknown> {
//
// (undocumented)
fetchMore(options: FetchMoreOptions_2<TData>): Promise<ApolloQueryResult<TData>>;
// Warning: (ae-forgotten-export) The symbol "CacheKey" needs to be exported by the entry point index.d.ts
// Warning: (ae-forgotten-export) The symbol "QueryKey" needs to be exported by the entry point index.d.ts
//
// (undocumented)
readonly key: CacheKey;
readonly key: QueryKey;
// Warning: (ae-forgotten-export) The symbol "Listener" needs to be exported by the entry point index.d.ts
//
// (undocumented)
Expand All @@ -1288,7 +1281,7 @@ class InternalQueryReference<TData = unknown> {
// (undocumented)
promise: Promise<ApolloQueryResult<TData>>;
// (undocumented)
promiseCache?: Map<CacheKey, Promise<ApolloQueryResult<TData>>>;
promiseCache?: Map<QueryKey, Promise<ApolloQueryResult<TData>>>;
// (undocumented)
refetch(variables: OperationVariables | undefined): Promise<ApolloQueryResult<TData>>;
// (undocumented)
Expand All @@ -1304,8 +1297,6 @@ interface InternalQueryReferenceOptions {
// (undocumented)
autoDisposeTimeoutMs?: number;
// (undocumented)
key: CacheKey;
// (undocumented)
onDispose?: () => void;
}

Expand Down Expand Up @@ -2093,6 +2084,12 @@ class QueryInfo {
variables?: Record<string, any>;
}

// @public (undocumented)
interface QueryKey {
// (undocumented)
__queryKey?: string;
}

// @public (undocumented)
export interface QueryLazyOptions<TVariables> {
// (undocumented)
Expand Down
2 changes: 1 addition & 1 deletion .size-limits.json
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
{
"dist/apollo-client.min.cjs": 38693,
"dist/apollo-client.min.cjs": 38675,
"import { ApolloClient, InMemoryCache, HttpLink } from \"dist/index.js\" (production)": 32306
}
8 changes: 3 additions & 5 deletions src/react/cache/QueryReference.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
createFulfilledPromise,
createRejectedPromise,
} from "../../utilities/index.js";
import type { CacheKey } from "./types.js";
import type { QueryKey } from "./types.js";
import type { useBackgroundQuery, useReadQuery } from "../hooks/index.js";

type Listener<TData> = (promise: Promise<ApolloQueryResult<TData>>) => void;
Expand All @@ -32,7 +32,6 @@ export interface QueryReference<TData = unknown> {
}

interface InternalQueryReferenceOptions {
key: CacheKey;
onDispose?: () => void;
autoDisposeTimeoutMs?: number;
}
Expand Down Expand Up @@ -65,10 +64,10 @@ type ObservedOptions = Pick<

export class InternalQueryReference<TData = unknown> {
public result: ApolloQueryResult<TData>;
public readonly key: CacheKey;
public readonly key: QueryKey = {};
public readonly observable: ObservableQuery<TData>;

public promiseCache?: Map<CacheKey, Promise<ApolloQueryResult<TData>>>;
public promiseCache?: Map<QueryKey, Promise<ApolloQueryResult<TData>>>;
public promise: Promise<ApolloQueryResult<TData>>;

private subscription: ObservableSubscription;
Expand All @@ -92,7 +91,6 @@ export class InternalQueryReference<TData = unknown> {
// Don't save this result as last result to prevent delivery of last result
// when first subscribing
this.result = observable.getCurrentResult(false);
this.key = options.key;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the key is no longer needed from options, could you remove this from InternalQueryReferenceOptions and stop passing it from SuspenseCache?


if (options.onDispose) {
this.onDispose = options.onDispose;
Expand Down
1 change: 0 additions & 1 deletion src/react/cache/SuspenseCache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ export class SuspenseCache {

if (!ref.current) {
ref.current = new InternalQueryReference(createObservable(), {
key: cacheKey,
autoDisposeTimeoutMs: this.options.autoDisposeTimeoutMs,
onDispose: () => {
delete ref.current;
Expand Down
4 changes: 4 additions & 0 deletions src/react/cache/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,7 @@ export type CacheKey = [
stringifiedVariables: string,
...queryKey: any[],
];

export interface QueryKey {
__queryKey?: string;
}
3 changes: 1 addition & 2 deletions src/react/hooks/__tests__/useSuspenseQuery.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3650,8 +3650,7 @@ describe("useSuspenseQuery", () => {
});

await waitFor(() => expect(renders.errorCount).toBe(1));

expect(client.getObservableQueries().size).toBe(0);
await waitFor(() => expect(client.getObservableQueries().size).toBe(0));
});

it('throws network errors when errorPolicy is set to "none"', async () => {
Expand Down
38 changes: 14 additions & 24 deletions src/react/hooks/useSuspenseQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import { getSuspenseCache } from "../cache/index.js";
import { canonicalStringify } from "../../cache/index.js";
import { skipToken } from "./constants.js";
import type { SkipToken } from "./constants.js";
import type { CacheKey } from "../cache/types.js";
import type { CacheKey, QueryKey } from "../cache/types.js";

export interface UseSuspenseQueryResult<
TData = unknown,
Expand Down Expand Up @@ -196,29 +196,26 @@ export function useSuspenseQuery<
client.watchQuery(watchQueryOptions)
);

const [promiseCache, setPromiseCache] = React.useState(
() => new Map([[queryRef.key, queryRef.promise]])
);

let promise = promiseCache.get(queryRef.key);
let [current, setPromise] = React.useState<
[QueryKey, Promise<ApolloQueryResult<any>>]
>([queryRef.key, queryRef.promise]);
jerelmiller marked this conversation as resolved.
Show resolved Hide resolved

if (queryRef.didChangeOptions(watchQueryOptions)) {
promise = queryRef.applyOptions(watchQueryOptions);
promiseCache.set(queryRef.key, promise);
// This saves us a re-execution of the render function when a variable changed.
if (current[0] !== queryRef.key) {
phryneas marked this conversation as resolved.
Show resolved Hide resolved
current[0] = queryRef.key;
current[1] = queryRef.promise;
}
let promise = current[1];

if (!promise) {
promise = queryRef.promise;
promiseCache.set(queryRef.key, promise);
if (queryRef.didChangeOptions(watchQueryOptions)) {
current[1] = promise = queryRef.applyOptions(watchQueryOptions);
}

React.useEffect(() => {
const dispose = queryRef.retain();

const removeListener = queryRef.listen((promise) => {
setPromiseCache((promiseCache) =>
new Map(promiseCache).set(queryRef.key, promise)
);
setPromise([queryRef.key, promise]);
});

return () => {
Expand All @@ -239,14 +236,10 @@ export function useSuspenseQuery<
}, [queryRef.result]);

const result = fetchPolicy === "standby" ? skipResult : __use(promise);

const fetchMore = React.useCallback(
((options) => {
const promise = queryRef.fetchMore(options);

setPromiseCache((previousPromiseCache) =>
new Map(previousPromiseCache).set(queryRef.key, queryRef.promise)
);
setPromise([queryRef.key, queryRef.promise]);

return promise;
}) satisfies FetchMoreFunction<
Expand All @@ -259,10 +252,7 @@ export function useSuspenseQuery<
const refetch: RefetchFunction<TData, TVariables> = React.useCallback(
(variables) => {
const promise = queryRef.refetch(variables);

setPromiseCache((previousPromiseCache) =>
new Map(previousPromiseCache).set(queryRef.key, queryRef.promise)
);
setPromise([queryRef.key, queryRef.promise]);

return promise;
},
Expand Down