From 8ecd36facc64dbb4f042207f8a53d11b8f63cac6 Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Wed, 15 Nov 2023 15:12:51 +0100 Subject: [PATCH 01/23] `useSuspenseQuery`: remove `promiseCache` --- .../hooks/__tests__/useSuspenseQuery.test.tsx | 5 ++-- src/react/hooks/useSuspenseQuery.ts | 29 ++++++------------- 2 files changed, 11 insertions(+), 23 deletions(-) diff --git a/src/react/hooks/__tests__/useSuspenseQuery.test.tsx b/src/react/hooks/__tests__/useSuspenseQuery.test.tsx index 468fe8f4fc5..2ebd57db009 100644 --- a/src/react/hooks/__tests__/useSuspenseQuery.test.tsx +++ b/src/react/hooks/__tests__/useSuspenseQuery.test.tsx @@ -3600,7 +3600,7 @@ describe("useSuspenseQuery", () => { jest.useRealTimers(); }); - it("tears down subscription when throwing an error on refetch", async () => { + it("xxx tears down subscription when throwing an error on refetch", async () => { using _consoleSpy = spyOnConsole("error"); const query = gql` @@ -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 () => { diff --git a/src/react/hooks/useSuspenseQuery.ts b/src/react/hooks/useSuspenseQuery.ts index ed203d4cfee..ca8830aed2e 100644 --- a/src/react/hooks/useSuspenseQuery.ts +++ b/src/react/hooks/useSuspenseQuery.ts @@ -196,29 +196,25 @@ export function useSuspenseQuery< client.watchQuery(watchQueryOptions) ); - const [promiseCache, setPromiseCache] = React.useState( - () => new Map([[queryRef.key, queryRef.promise]]) - ); + let [current, setPromise] = React.useState< + [CacheKey, Promise>] + >([queryRef.key, queryRef.promise]); - let promise = promiseCache.get(queryRef.key); + let promise = current[0] === queryRef.key ? current[1] : undefined; if (queryRef.didChangeOptions(watchQueryOptions)) { - promise = queryRef.applyOptions(watchQueryOptions); - promiseCache.set(queryRef.key, promise); + current[1] = promise = queryRef.applyOptions(watchQueryOptions); } if (!promise) { - promise = queryRef.promise; - promiseCache.set(queryRef.key, promise); + current[1] = promise = queryRef.promise; } 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 () => { @@ -239,14 +235,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< @@ -259,10 +251,7 @@ export function useSuspenseQuery< const refetch: RefetchFunction = 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; }, From b8c51afa545d4cd04257acafea0ec9672c1afee9 Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Wed, 15 Nov 2023 15:15:47 +0100 Subject: [PATCH 02/23] missed cleanup --- src/react/hooks/__tests__/useSuspenseQuery.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/react/hooks/__tests__/useSuspenseQuery.test.tsx b/src/react/hooks/__tests__/useSuspenseQuery.test.tsx index 2ebd57db009..7e53013855d 100644 --- a/src/react/hooks/__tests__/useSuspenseQuery.test.tsx +++ b/src/react/hooks/__tests__/useSuspenseQuery.test.tsx @@ -3600,7 +3600,7 @@ describe("useSuspenseQuery", () => { jest.useRealTimers(); }); - it("xxx tears down subscription when throwing an error on refetch", async () => { + it("tears down subscription when throwing an error on refetch", async () => { using _consoleSpy = spyOnConsole("error"); const query = gql` From 79577b5fef434bcf593d7e46e4abfc5f879db15f Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Wed, 15 Nov 2023 18:05:10 +0100 Subject: [PATCH 03/23] WIP status: useBackgroundQuery --- src/react/cache/QueryReference.ts | 22 +++++++++--- .../__tests__/useBackgroundQuery.test.tsx | 20 +++++------ src/react/hooks/useBackgroundQuery.ts | 34 ++++++++----------- src/react/hooks/useReadQuery.ts | 26 +++++--------- 4 files changed, 52 insertions(+), 50 deletions(-) diff --git a/src/react/cache/QueryReference.ts b/src/react/cache/QueryReference.ts index 5b2fc14c643..b15879e4a29 100644 --- a/src/react/cache/QueryReference.ts +++ b/src/react/cache/QueryReference.ts @@ -22,6 +22,7 @@ type FetchMoreOptions = Parameters< >[0]; const QUERY_REFERENCE_SYMBOL: unique symbol = Symbol(); +const PROMISE_SYMBOL: unique symbol = Symbol(); /** * A `QueryReference` is an opaque object returned by {@link useBackgroundQuery}. * A child component reading the `QueryReference` via {@link useReadQuery} will @@ -29,6 +30,7 @@ const QUERY_REFERENCE_SYMBOL: unique symbol = Symbol(); */ export interface QueryReference { [QUERY_REFERENCE_SYMBOL]: InternalQueryReference; + [PROMISE_SYMBOL]: Promise>; } interface InternalQueryReferenceOptions { @@ -38,15 +40,26 @@ interface InternalQueryReferenceOptions { } export function wrapQueryRef( - internalQueryRef: InternalQueryReference + internalQueryRef: InternalQueryReference, + promise: Promise> ): QueryReference { - return { [QUERY_REFERENCE_SYMBOL]: internalQueryRef }; + return { + [QUERY_REFERENCE_SYMBOL]: internalQueryRef, + [PROMISE_SYMBOL]: promise, + }; } export function unwrapQueryRef( queryRef: QueryReference -): InternalQueryReference { - return queryRef[QUERY_REFERENCE_SYMBOL]; +): [InternalQueryReference, () => Promise>] { + return [queryRef[QUERY_REFERENCE_SYMBOL], () => queryRef[PROMISE_SYMBOL]]; +} + +export function updateWrappedQueryRef( + queryRef: QueryReference, + promise: Promise> +) { + queryRef[PROMISE_SYMBOL] = promise; } const OBSERVED_CHANGED_OPTIONS = [ @@ -69,6 +82,7 @@ export class InternalQueryReference { public readonly observable: ObservableQuery; public promiseCache?: Map>>; + public lastObservedPromise?: Promise>; public promise: Promise>; private subscription: ObservableSubscription; diff --git a/src/react/hooks/__tests__/useBackgroundQuery.test.tsx b/src/react/hooks/__tests__/useBackgroundQuery.test.tsx index 08121af7f27..ea77002780c 100644 --- a/src/react/hooks/__tests__/useBackgroundQuery.test.tsx +++ b/src/react/hooks/__tests__/useBackgroundQuery.test.tsx @@ -641,7 +641,7 @@ describe("useBackgroundQuery", () => { const [queryRef] = result.current; - const _result = await unwrapQueryRef(queryRef).promise; + const _result = await unwrapQueryRef(queryRef)[0].promise; expect(_result).toEqual({ data: { hello: "world 1" }, @@ -678,7 +678,7 @@ describe("useBackgroundQuery", () => { const [queryRef] = result.current; - const _result = await unwrapQueryRef(queryRef).promise; + const _result = await unwrapQueryRef(queryRef)[0].promise; await waitFor(() => { expect(_result).toEqual({ @@ -719,7 +719,7 @@ describe("useBackgroundQuery", () => { const [queryRef] = result.current; - const _result = await unwrapQueryRef(queryRef).promise; + const _result = await unwrapQueryRef(queryRef)[0].promise; await waitFor(() => { expect(_result).toMatchObject({ @@ -779,7 +779,7 @@ describe("useBackgroundQuery", () => { const [queryRef] = result.current; - const _result = await unwrapQueryRef(queryRef).promise; + const _result = await unwrapQueryRef(queryRef)[0].promise; const resultSet = new Set(_result.data.results); const values = Array.from(resultSet).map((item) => item.value); @@ -840,7 +840,7 @@ describe("useBackgroundQuery", () => { const [queryRef] = result.current; - const _result = await unwrapQueryRef(queryRef).promise; + const _result = await unwrapQueryRef(queryRef)[0].promise; const resultSet = new Set(_result.data.results); const values = Array.from(resultSet).map((item) => item.value); @@ -882,7 +882,7 @@ describe("useBackgroundQuery", () => { const [queryRef] = result.current; - const _result = await unwrapQueryRef(queryRef).promise; + const _result = await unwrapQueryRef(queryRef)[0].promise; expect(_result).toEqual({ data: { hello: "from link" }, @@ -922,7 +922,7 @@ describe("useBackgroundQuery", () => { const [queryRef] = result.current; - const _result = await unwrapQueryRef(queryRef).promise; + const _result = await unwrapQueryRef(queryRef)[0].promise; expect(_result).toEqual({ data: { hello: "from cache" }, @@ -969,7 +969,7 @@ describe("useBackgroundQuery", () => { const [queryRef] = result.current; - const _result = await unwrapQueryRef(queryRef).promise; + const _result = await unwrapQueryRef(queryRef)[0].promise; expect(_result).toEqual({ data: { foo: "bar", hello: "from link" }, @@ -1009,7 +1009,7 @@ describe("useBackgroundQuery", () => { const [queryRef] = result.current; - const _result = await unwrapQueryRef(queryRef).promise; + const _result = await unwrapQueryRef(queryRef)[0].promise; expect(_result).toEqual({ data: { hello: "from link" }, @@ -1052,7 +1052,7 @@ describe("useBackgroundQuery", () => { const [queryRef] = result.current; - const _result = await unwrapQueryRef(queryRef).promise; + const _result = await unwrapQueryRef(queryRef)[0].promise; expect(_result).toEqual({ data: { hello: "from link" }, diff --git a/src/react/hooks/useBackgroundQuery.ts b/src/react/hooks/useBackgroundQuery.ts index 20b2e978aa7..8659ed18c86 100644 --- a/src/react/hooks/useBackgroundQuery.ts +++ b/src/react/hooks/useBackgroundQuery.ts @@ -7,7 +7,11 @@ import type { WatchQueryOptions, } from "../../core/index.js"; import { useApolloClient } from "./useApolloClient.js"; -import { wrapQueryRef } from "../cache/QueryReference.js"; +import { + unwrapQueryRef, + updateWrappedQueryRef, + wrapQueryRef, +} from "../cache/QueryReference.js"; import type { QueryReference } from "../cache/QueryReference.js"; import type { BackgroundQueryHookOptions, NoInfer } from "../types/types.js"; import { __use } from "./internal/index.js"; @@ -202,13 +206,16 @@ export function useBackgroundQuery< client.watchQuery(watchQueryOptions as WatchQueryOptions) ); - const [promiseCache, setPromiseCache] = React.useState( - () => new Map([[queryRef.key, queryRef.promise]]) - ); - + const [wrapped, setWrappedQueryRef] = React.useState({ + current: wrapQueryRef(queryRef, queryRef.promise), + }); + if (unwrapQueryRef(wrapped.current)[0] !== queryRef) { + wrapped.current = wrapQueryRef(queryRef, queryRef.promise); + } + let wrappedQueryRef = wrapped.current; if (queryRef.didChangeOptions(watchQueryOptions)) { const promise = queryRef.applyOptions(watchQueryOptions); - promiseCache.set(queryRef.key, promise); + updateWrappedQueryRef(wrappedQueryRef, promise); } React.useEffect(() => queryRef.retain(), [queryRef]); @@ -217,9 +224,7 @@ export function useBackgroundQuery< (options) => { const promise = queryRef.fetchMore(options as FetchMoreQueryOptions); - setPromiseCache((promiseCache) => - new Map(promiseCache).set(queryRef.key, queryRef.promise) - ); + setWrappedQueryRef({ current: wrapQueryRef(queryRef, queryRef.promise) }); return promise; }, @@ -230,22 +235,13 @@ export function useBackgroundQuery< (variables) => { const promise = queryRef.refetch(variables); - setPromiseCache((promiseCache) => - new Map(promiseCache).set(queryRef.key, queryRef.promise) - ); + setWrappedQueryRef({ current: wrapQueryRef(queryRef, queryRef.promise) }); return promise; }, [queryRef] ); - queryRef.promiseCache = promiseCache; - - const wrappedQueryRef = React.useMemo( - () => wrapQueryRef(queryRef), - [queryRef] - ); - return [ didFetchResult.current ? wrappedQueryRef : void 0, { fetchMore, refetch }, diff --git a/src/react/hooks/useReadQuery.ts b/src/react/hooks/useReadQuery.ts index 803535c4878..41e1efe47e5 100644 --- a/src/react/hooks/useReadQuery.ts +++ b/src/react/hooks/useReadQuery.ts @@ -1,9 +1,11 @@ import * as React from "rehackt"; -import { unwrapQueryRef } from "../cache/QueryReference.js"; +import { + unwrapQueryRef, + updateWrappedQueryRef, +} from "../cache/QueryReference.js"; import type { QueryReference } from "../cache/QueryReference.js"; import { __use } from "./internal/index.js"; import { toApolloError } from "./useSuspenseQuery.js"; -import { invariant } from "../../utilities/globals/index.js"; import { useSyncExternalStore } from "./useSyncExternalStore.js"; import type { ApolloError } from "../../errors/index.js"; import type { NetworkStatus } from "../../core/index.js"; @@ -36,32 +38,22 @@ export interface UseReadQueryResult { export function useReadQuery( queryRef: QueryReference ): UseReadQueryResult { - const internalQueryRef = unwrapQueryRef(queryRef); - invariant( - internalQueryRef.promiseCache, - "It appears that `useReadQuery` was used outside of `useBackgroundQuery`. " + - "`useReadQuery` is only supported for use with `useBackgroundQuery`. " + - "Please ensure you are passing the `queryRef` returned from `useBackgroundQuery`." - ); - - const { promiseCache, key } = internalQueryRef; + const [internalQueryRef, getPromise] = unwrapQueryRef(queryRef); - if (!promiseCache.has(key)) { - promiseCache.set(key, internalQueryRef.promise); - } + console.log(internalQueryRef); const promise = useSyncExternalStore( React.useCallback( (forceUpdate) => { return internalQueryRef.listen((promise) => { - internalQueryRef.promiseCache!.set(internalQueryRef.key, promise); + updateWrappedQueryRef(queryRef, promise); forceUpdate(); }); }, [internalQueryRef] ), - () => promiseCache.get(key)!, - () => promiseCache.get(key)! + getPromise, + getPromise ); const result = __use(promise); From 1510019dfb948f64f581c754799936f6859b7f84 Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Thu, 16 Nov 2023 12:20:01 +0100 Subject: [PATCH 04/23] working reimplementation --- src/react/cache/QueryReference.ts | 52 ++++++++++++------- .../__tests__/useBackgroundQuery.test.tsx | 16 +++--- src/react/hooks/useBackgroundQuery.ts | 2 +- src/react/hooks/useReadQuery.ts | 6 +-- src/utilities/index.ts | 2 + src/utilities/promises/decoration.ts | 32 ++++++++++-- 6 files changed, 73 insertions(+), 37 deletions(-) diff --git a/src/react/cache/QueryReference.ts b/src/react/cache/QueryReference.ts index b15879e4a29..9346501fc7c 100644 --- a/src/react/cache/QueryReference.ts +++ b/src/react/cache/QueryReference.ts @@ -7,15 +7,23 @@ import type { WatchQueryOptions, } from "../../core/index.js"; import { isNetworkRequestSettled } from "../../core/index.js"; -import type { ObservableSubscription } from "../../utilities/index.js"; +import type { + ObservableSubscription, + PromiseWithState, + WithSequence, +} from "../../utilities/index.js"; import { createFulfilledPromise, createRejectedPromise, } from "../../utilities/index.js"; import type { CacheKey } from "./types.js"; import type { useBackgroundQuery, useReadQuery } from "../hooks/index.js"; +import { withSequence, wrapPromiseWithState } from "../../utilities/index.js"; + +type QueryRefPromise = PromiseWithState> & + WithSequence; -type Listener = (promise: Promise>) => void; +type Listener = (promise: QueryRefPromise) => void; type FetchMoreOptions = Parameters< ObservableQuery["fetchMore"] @@ -30,7 +38,7 @@ const PROMISE_SYMBOL: unique symbol = Symbol(); */ export interface QueryReference { [QUERY_REFERENCE_SYMBOL]: InternalQueryReference; - [PROMISE_SYMBOL]: Promise>; + [PROMISE_SYMBOL]: QueryRefPromise; } interface InternalQueryReferenceOptions { @@ -41,7 +49,7 @@ interface InternalQueryReferenceOptions { export function wrapQueryRef( internalQueryRef: InternalQueryReference, - promise: Promise> + promise: QueryRefPromise ): QueryReference { return { [QUERY_REFERENCE_SYMBOL]: internalQueryRef, @@ -51,13 +59,13 @@ export function wrapQueryRef( export function unwrapQueryRef( queryRef: QueryReference -): [InternalQueryReference, () => Promise>] { +): [InternalQueryReference, () => QueryRefPromise] { return [queryRef[QUERY_REFERENCE_SYMBOL], () => queryRef[PROMISE_SYMBOL]]; } export function updateWrappedQueryRef( queryRef: QueryReference, - promise: Promise> + promise: QueryRefPromise ) { queryRef[PROMISE_SYMBOL] = promise; } @@ -81,9 +89,7 @@ export class InternalQueryReference { public readonly key: CacheKey; public readonly observable: ObservableQuery; - public promiseCache?: Map>>; - public lastObservedPromise?: Promise>; - public promise: Promise>; + public promise: QueryRefPromise; private subscription: ObservableSubscription; private listeners = new Set>(); @@ -120,10 +126,14 @@ export class InternalQueryReference { this.promise = createFulfilledPromise(this.result); this.status = "idle"; } else { - this.promise = new Promise((resolve, reject) => { - this.resolve = resolve; - this.reject = reject; - }); + this.promise = withSequence( + wrapPromiseWithState( + new Promise((resolve, reject) => { + this.resolve = resolve; + this.reject = reject; + }) + ) + ); } this.subscription = observable @@ -284,23 +294,27 @@ export class InternalQueryReference { break; } case "idle": { - this.promise = createRejectedPromise(error); + this.promise = createRejectedPromise>(error); this.deliver(this.promise); } } } - private deliver(promise: Promise>) { + private deliver(promise: QueryRefPromise) { this.listeners.forEach((listener) => listener(promise)); } private initiateFetch(returnedPromise: Promise>) { this.status = "loading"; - this.promise = new Promise((resolve, reject) => { - this.resolve = resolve; - this.reject = reject; - }); + this.promise = withSequence( + wrapPromiseWithState( + new Promise((resolve, reject) => { + this.resolve = resolve; + this.reject = reject; + }) + ) + ); this.promise.catch(() => {}); diff --git a/src/react/hooks/__tests__/useBackgroundQuery.test.tsx b/src/react/hooks/__tests__/useBackgroundQuery.test.tsx index ea77002780c..9570e16e1f9 100644 --- a/src/react/hooks/__tests__/useBackgroundQuery.test.tsx +++ b/src/react/hooks/__tests__/useBackgroundQuery.test.tsx @@ -3184,13 +3184,9 @@ describe("useBackgroundQuery", () => { const user = userEvent.setup(); await act(() => user.click(button)); - { - // parent component re-suspends - const { snapshot } = await ProfiledApp.takeRender(); - expect(snapshot.suspenseCount).toBe(2); - } { const { snapshot, withinDOM } = await ProfiledApp.takeRender(); + expect(snapshot.suspenseCount).toBe(1); // @jerelmiller can you please verify that this is still in the spirit of the test? // This seems to have moved onto the next render - or before the test skipped one. expect(snapshot.count).toBe(2); @@ -3258,8 +3254,8 @@ describe("useBackgroundQuery", () => { await screen.findByText("2 - Captain America") ).toBeInTheDocument(); - // parent component re-suspends - expect(renders.suspenseCount).toBe(2); + // parent component didn't re-suspend + expect(renders.suspenseCount).toBe(1); expect(renders.count).toBe(3); // extra render puts an additional frame into the array @@ -3295,8 +3291,8 @@ describe("useBackgroundQuery", () => { const user = userEvent.setup(); await act(() => user.click(button)); - // parent component re-suspends - expect(renders.suspenseCount).toBe(2); + // parent component didn't re-suspend + expect(renders.suspenseCount).toBe(1); expect(renders.count).toBe(2); expect( @@ -3306,7 +3302,7 @@ describe("useBackgroundQuery", () => { await act(() => user.click(button)); // parent component re-suspends - expect(renders.suspenseCount).toBe(3); + expect(renders.suspenseCount).toBe(1); expect(renders.count).toBe(3); expect( diff --git a/src/react/hooks/useBackgroundQuery.ts b/src/react/hooks/useBackgroundQuery.ts index 8659ed18c86..fadc9f93c78 100644 --- a/src/react/hooks/useBackgroundQuery.ts +++ b/src/react/hooks/useBackgroundQuery.ts @@ -210,7 +210,7 @@ export function useBackgroundQuery< current: wrapQueryRef(queryRef, queryRef.promise), }); if (unwrapQueryRef(wrapped.current)[0] !== queryRef) { - wrapped.current = wrapQueryRef(queryRef, queryRef.promise); + setWrappedQueryRef({ current: wrapQueryRef(queryRef, queryRef.promise) }); } let wrappedQueryRef = wrapped.current; if (queryRef.didChangeOptions(watchQueryOptions)) { diff --git a/src/react/hooks/useReadQuery.ts b/src/react/hooks/useReadQuery.ts index 41e1efe47e5..6e2968716e1 100644 --- a/src/react/hooks/useReadQuery.ts +++ b/src/react/hooks/useReadQuery.ts @@ -9,6 +9,7 @@ import { toApolloError } from "./useSuspenseQuery.js"; import { useSyncExternalStore } from "./useSyncExternalStore.js"; import type { ApolloError } from "../../errors/index.js"; import type { NetworkStatus } from "../../core/index.js"; +import { secondIfNewerFulfilledOrFirst } from "../../utilities/promises/decoration.js"; export interface UseReadQueryResult { /** @@ -40,9 +41,7 @@ export function useReadQuery( ): UseReadQueryResult { const [internalQueryRef, getPromise] = unwrapQueryRef(queryRef); - console.log(internalQueryRef); - - const promise = useSyncExternalStore( + let promise = useSyncExternalStore( React.useCallback( (forceUpdate) => { return internalQueryRef.listen((promise) => { @@ -55,6 +54,7 @@ export function useReadQuery( getPromise, getPromise ); + promise = secondIfNewerFulfilledOrFirst(promise, internalQueryRef.promise); const result = __use(promise); diff --git a/src/utilities/index.ts b/src/utilities/index.ts index da8affb4b5a..0e70e465fdb 100644 --- a/src/utilities/index.ts +++ b/src/utilities/index.ts @@ -98,11 +98,13 @@ export type { } from "./observables/Observable.js"; export { Observable } from "./observables/Observable.js"; +export type { PromiseWithState, WithSequence } from "./promises/decoration.js"; export { isStatefulPromise, createFulfilledPromise, createRejectedPromise, wrapPromiseWithState, + withSequence, } from "./promises/decoration.js"; export * from "./common/mergeDeep.js"; diff --git a/src/utilities/promises/decoration.ts b/src/utilities/promises/decoration.ts index 9022aeed23c..a19125185cc 100644 --- a/src/utilities/promises/decoration.ts +++ b/src/utilities/promises/decoration.ts @@ -12,22 +12,46 @@ export interface RejectedPromise extends Promise { reason: unknown; } +export interface WithSequence { + [sequence]: number; +} + +export function secondIfNewerFulfilledOrFirst( + first: PromiseWithState & WithSequence, + second: PromiseWithState & WithSequence +): PromiseWithState & WithSequence { + return second.status === "fulfilled" && second[sequence] > first[sequence] + ? second + : first; +} + +let current = 0; +const sequence = Symbol("sequence"); + export type PromiseWithState = | PendingPromise | FulfilledPromise | RejectedPromise; +export function withSequence>( + promise: T +): T & WithSequence { + return Object.assign(promise, { [sequence]: current++ }); +} + export function createFulfilledPromise(value: TValue) { - const promise = Promise.resolve(value) as FulfilledPromise; + const promise = Promise.resolve(value) as FulfilledPromise & + WithSequence; promise.status = "fulfilled"; promise.value = value; - return promise; + return withSequence(promise); } export function createRejectedPromise(reason: unknown) { - const promise = Promise.reject(reason) as RejectedPromise; + const promise = Promise.reject(reason) as RejectedPromise & + WithSequence; // prevent potential edge cases leaking unhandled error rejections promise.catch(() => {}); @@ -35,7 +59,7 @@ export function createRejectedPromise(reason: unknown) { promise.status = "rejected"; promise.reason = reason; - return promise; + return withSequence(promise); } export function isStatefulPromise( From 532505ac1f1f75ef4b52928487917f26e7123289 Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Thu, 16 Nov 2023 13:58:24 +0100 Subject: [PATCH 05/23] update `current[0]` correctly --- src/react/hooks/useSuspenseQuery.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/react/hooks/useSuspenseQuery.ts b/src/react/hooks/useSuspenseQuery.ts index ca8830aed2e..daabb7e3eeb 100644 --- a/src/react/hooks/useSuspenseQuery.ts +++ b/src/react/hooks/useSuspenseQuery.ts @@ -200,16 +200,16 @@ export function useSuspenseQuery< [CacheKey, Promise>] >([queryRef.key, queryRef.promise]); - let promise = current[0] === queryRef.key ? current[1] : undefined; + if (current[0] !== queryRef.key) { + current[0] = queryRef.key; + current[1] = queryRef.promise; + } + let promise = current[1]; if (queryRef.didChangeOptions(watchQueryOptions)) { current[1] = promise = queryRef.applyOptions(watchQueryOptions); } - if (!promise) { - current[1] = promise = queryRef.promise; - } - React.useEffect(() => { const dispose = queryRef.retain(); From 50c2149f8691f9f48e3c0214ef671a075a0f87b9 Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Thu, 16 Nov 2023 13:58:49 +0100 Subject: [PATCH 06/23] change `key` to empty object we only care about identity, not about contents --- src/react/cache/QueryReference.ts | 7 +++---- src/react/cache/types.ts | 4 ++++ src/react/hooks/useSuspenseQuery.ts | 4 ++-- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/react/cache/QueryReference.ts b/src/react/cache/QueryReference.ts index 5b2fc14c643..b4069fa9f36 100644 --- a/src/react/cache/QueryReference.ts +++ b/src/react/cache/QueryReference.ts @@ -12,7 +12,7 @@ import { createFulfilledPromise, createRejectedPromise, } from "../../utilities/index.js"; -import type { CacheKey } from "./types.js"; +import type { CacheKey, QueryKey } from "./types.js"; import type { useBackgroundQuery, useReadQuery } from "../hooks/index.js"; type Listener = (promise: Promise>) => void; @@ -65,10 +65,10 @@ type ObservedOptions = Pick< export class InternalQueryReference { public result: ApolloQueryResult; - public readonly key: CacheKey; + public readonly key: QueryKey = {}; public readonly observable: ObservableQuery; - public promiseCache?: Map>>; + public promiseCache?: Map>>; public promise: Promise>; private subscription: ObservableSubscription; @@ -92,7 +92,6 @@ export class InternalQueryReference { // 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; if (options.onDispose) { this.onDispose = options.onDispose; diff --git a/src/react/cache/types.ts b/src/react/cache/types.ts index e9e0ba8b826..40f3c4cc8fc 100644 --- a/src/react/cache/types.ts +++ b/src/react/cache/types.ts @@ -5,3 +5,7 @@ export type CacheKey = [ stringifiedVariables: string, ...queryKey: any[], ]; + +export interface QueryKey { + __queryKey?: string; +} diff --git a/src/react/hooks/useSuspenseQuery.ts b/src/react/hooks/useSuspenseQuery.ts index daabb7e3eeb..fd1df0129f2 100644 --- a/src/react/hooks/useSuspenseQuery.ts +++ b/src/react/hooks/useSuspenseQuery.ts @@ -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, @@ -197,7 +197,7 @@ export function useSuspenseQuery< ); let [current, setPromise] = React.useState< - [CacheKey, Promise>] + [QueryKey, Promise>] >([queryRef.key, queryRef.promise]); if (current[0] !== queryRef.key) { From c9c31774731a30841da22e6de211e101d3431077 Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Tue, 21 Nov 2023 15:33:26 +0100 Subject: [PATCH 07/23] move `secondIfNewerFulfilledOrFirst` call into `unwrapQueryRef` --- src/react/cache/QueryReference.ts | 13 +++++++++++-- src/react/hooks/useReadQuery.ts | 6 ++++-- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/src/react/cache/QueryReference.ts b/src/react/cache/QueryReference.ts index 11e1c1919a1..c9ec24dac89 100644 --- a/src/react/cache/QueryReference.ts +++ b/src/react/cache/QueryReference.ts @@ -19,6 +19,7 @@ import { import type { CacheKey, QueryKey } from "./types.js"; import type { useBackgroundQuery, useReadQuery } from "../hooks/index.js"; import { withSequence, wrapPromiseWithState } from "../../utilities/index.js"; +import { secondIfNewerFulfilledOrFirst } from "../../utilities/promises/decoration.js"; type QueryRefPromise = PromiseWithState> & WithSequence; @@ -37,7 +38,7 @@ const PROMISE_SYMBOL: unique symbol = Symbol(); * suspend until the promise resolves. */ export interface QueryReference { - [QUERY_REFERENCE_SYMBOL]: InternalQueryReference; + readonly [QUERY_REFERENCE_SYMBOL]: InternalQueryReference; [PROMISE_SYMBOL]: QueryRefPromise; } @@ -60,7 +61,15 @@ export function wrapQueryRef( export function unwrapQueryRef( queryRef: QueryReference ): [InternalQueryReference, () => QueryRefPromise] { - return [queryRef[QUERY_REFERENCE_SYMBOL], () => queryRef[PROMISE_SYMBOL]]; + const reference = queryRef[QUERY_REFERENCE_SYMBOL]; + return [ + reference, + () => + secondIfNewerFulfilledOrFirst( + queryRef[PROMISE_SYMBOL], + reference.promise + ), + ]; } export function updateWrappedQueryRef( diff --git a/src/react/hooks/useReadQuery.ts b/src/react/hooks/useReadQuery.ts index 6e2968716e1..e180e3c234a 100644 --- a/src/react/hooks/useReadQuery.ts +++ b/src/react/hooks/useReadQuery.ts @@ -39,7 +39,10 @@ export interface UseReadQueryResult { export function useReadQuery( queryRef: QueryReference ): UseReadQueryResult { - const [internalQueryRef, getPromise] = unwrapQueryRef(queryRef); + const [internalQueryRef, getPromise] = React.useMemo( + () => unwrapQueryRef(queryRef), + [queryRef] + ); let promise = useSyncExternalStore( React.useCallback( @@ -54,7 +57,6 @@ export function useReadQuery( getPromise, getPromise ); - promise = secondIfNewerFulfilledOrFirst(promise, internalQueryRef.promise); const result = __use(promise); From 4a5c3e2b6be4eff2d91ebc88af003a8b3b0ff1d0 Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Tue, 21 Nov 2023 15:40:32 +0100 Subject: [PATCH 08/23] remove unused import --- src/react/hooks/useReadQuery.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/react/hooks/useReadQuery.ts b/src/react/hooks/useReadQuery.ts index e180e3c234a..70a2f3fc3fb 100644 --- a/src/react/hooks/useReadQuery.ts +++ b/src/react/hooks/useReadQuery.ts @@ -9,7 +9,6 @@ import { toApolloError } from "./useSuspenseQuery.js"; import { useSyncExternalStore } from "./useSyncExternalStore.js"; import type { ApolloError } from "../../errors/index.js"; import type { NetworkStatus } from "../../core/index.js"; -import { secondIfNewerFulfilledOrFirst } from "../../utilities/promises/decoration.js"; export interface UseReadQueryResult { /** From 083af14709fa2280ccf96f6555994b219b99b174 Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Thu, 30 Nov 2023 16:39:44 +0100 Subject: [PATCH 09/23] PR feedback --- src/react/hooks/useBackgroundQuery.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/react/hooks/useBackgroundQuery.ts b/src/react/hooks/useBackgroundQuery.ts index fadc9f93c78..37207130111 100644 --- a/src/react/hooks/useBackgroundQuery.ts +++ b/src/react/hooks/useBackgroundQuery.ts @@ -206,13 +206,13 @@ export function useBackgroundQuery< client.watchQuery(watchQueryOptions as WatchQueryOptions) ); - const [wrapped, setWrappedQueryRef] = React.useState({ - current: wrapQueryRef(queryRef, queryRef.promise), - }); - if (unwrapQueryRef(wrapped.current)[0] !== queryRef) { - setWrappedQueryRef({ current: wrapQueryRef(queryRef, queryRef.promise) }); + const [wrapped, setWrappedQueryRef] = React.useState( + wrapQueryRef(queryRef, queryRef.promise) + ); + if (unwrapQueryRef(wrapped)[0] !== queryRef) { + setWrappedQueryRef(wrapQueryRef(queryRef, queryRef.promise)); } - let wrappedQueryRef = wrapped.current; + let wrappedQueryRef = wrapped; if (queryRef.didChangeOptions(watchQueryOptions)) { const promise = queryRef.applyOptions(watchQueryOptions); updateWrappedQueryRef(wrappedQueryRef, promise); @@ -224,7 +224,7 @@ export function useBackgroundQuery< (options) => { const promise = queryRef.fetchMore(options as FetchMoreQueryOptions); - setWrappedQueryRef({ current: wrapQueryRef(queryRef, queryRef.promise) }); + setWrappedQueryRef(wrapQueryRef(queryRef, queryRef.promise)); return promise; }, @@ -235,7 +235,7 @@ export function useBackgroundQuery< (variables) => { const promise = queryRef.refetch(variables); - setWrappedQueryRef({ current: wrapQueryRef(queryRef, queryRef.promise) }); + setWrappedQueryRef(wrapQueryRef(queryRef, queryRef.promise)); return promise; }, From 3da56f006642e67df946149240055b461f4f4191 Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Thu, 30 Nov 2023 16:42:48 +0100 Subject: [PATCH 10/23] more PR feedback --- src/react/hooks/useBackgroundQuery.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/react/hooks/useBackgroundQuery.ts b/src/react/hooks/useBackgroundQuery.ts index 37207130111..6bb353556d3 100644 --- a/src/react/hooks/useBackgroundQuery.ts +++ b/src/react/hooks/useBackgroundQuery.ts @@ -206,13 +206,12 @@ export function useBackgroundQuery< client.watchQuery(watchQueryOptions as WatchQueryOptions) ); - const [wrapped, setWrappedQueryRef] = React.useState( + const [wrappedQueryRef, setWrappedQueryRef] = React.useState( wrapQueryRef(queryRef, queryRef.promise) ); - if (unwrapQueryRef(wrapped)[0] !== queryRef) { + if (unwrapQueryRef(wrappedQueryRef)[0] !== queryRef) { setWrappedQueryRef(wrapQueryRef(queryRef, queryRef.promise)); } - let wrappedQueryRef = wrapped; if (queryRef.didChangeOptions(watchQueryOptions)) { const promise = queryRef.applyOptions(watchQueryOptions); updateWrappedQueryRef(wrappedQueryRef, promise); From 2cad58097b5c6f76f5ce824e9055036f35eb2584 Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Thu, 30 Nov 2023 16:47:27 +0100 Subject: [PATCH 11/23] review feedback --- src/react/hooks/useReadQuery.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/react/hooks/useReadQuery.ts b/src/react/hooks/useReadQuery.ts index 70a2f3fc3fb..f2320aa58ea 100644 --- a/src/react/hooks/useReadQuery.ts +++ b/src/react/hooks/useReadQuery.ts @@ -43,7 +43,7 @@ export function useReadQuery( [queryRef] ); - let promise = useSyncExternalStore( + const promise = useSyncExternalStore( React.useCallback( (forceUpdate) => { return internalQueryRef.listen((promise) => { From 346e5e8e97b53a824a4b404c190687336aa4b39e Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Thu, 30 Nov 2023 17:13:15 +0100 Subject: [PATCH 12/23] re-add delay to test --- .../__tests__/useBackgroundQuery.test.tsx | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/react/hooks/__tests__/useBackgroundQuery.test.tsx b/src/react/hooks/__tests__/useBackgroundQuery.test.tsx index 9570e16e1f9..500fb21273c 100644 --- a/src/react/hooks/__tests__/useBackgroundQuery.test.tsx +++ b/src/react/hooks/__tests__/useBackgroundQuery.test.tsx @@ -216,6 +216,7 @@ function renderVariablesIntegrationTest({ }, }, }, + delay: 200, }; } ); @@ -3184,9 +3185,13 @@ describe("useBackgroundQuery", () => { const user = userEvent.setup(); await act(() => user.click(button)); + { + // parent component re-suspends + const { snapshot } = await ProfiledApp.takeRender(); + expect(snapshot.suspenseCount).toBe(2); + } { const { snapshot, withinDOM } = await ProfiledApp.takeRender(); - expect(snapshot.suspenseCount).toBe(1); // @jerelmiller can you please verify that this is still in the spirit of the test? // This seems to have moved onto the next render - or before the test skipped one. expect(snapshot.count).toBe(2); @@ -3291,9 +3296,9 @@ describe("useBackgroundQuery", () => { const user = userEvent.setup(); await act(() => user.click(button)); - // parent component didn't re-suspend - expect(renders.suspenseCount).toBe(1); - expect(renders.count).toBe(2); + // parent component re-suspends + expect(renders.suspenseCount).toBe(2); + expect(renders.count).toBe(1); expect( await screen.findByText("1 - Spider-Man (updated)") @@ -3302,12 +3307,14 @@ describe("useBackgroundQuery", () => { await act(() => user.click(button)); // parent component re-suspends - expect(renders.suspenseCount).toBe(1); - expect(renders.count).toBe(3); + expect(renders.suspenseCount).toBe(3); + expect(renders.count).toBe(2); expect( await screen.findByText("1 - Spider-Man (updated again)") ).toBeInTheDocument(); + + expect(renders.count).toBe(3); }); it("throws errors when errors are returned after calling `refetch`", async () => { using _consoleSpy = spyOnConsole("error"); From fc66f1274843e66b883b35857b51ae56c86fbf5a Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Thu, 30 Nov 2023 18:31:54 +0100 Subject: [PATCH 13/23] add more mock delays --- src/react/hooks/__tests__/useBackgroundQuery.test.tsx | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/react/hooks/__tests__/useBackgroundQuery.test.tsx b/src/react/hooks/__tests__/useBackgroundQuery.test.tsx index 500fb21273c..8e4e356338c 100644 --- a/src/react/hooks/__tests__/useBackgroundQuery.test.tsx +++ b/src/react/hooks/__tests__/useBackgroundQuery.test.tsx @@ -3228,12 +3228,14 @@ describe("useBackgroundQuery", () => { result: { data: { character: { id: "1", name: "Captain Marvel" } }, }, + delay: 200, }, { request: { query, variables: { id: "2" } }, result: { data: { character: { id: "2", name: "Captain America" } }, }, + delay: 200, }, ]; @@ -3259,8 +3261,8 @@ describe("useBackgroundQuery", () => { await screen.findByText("2 - Captain America") ).toBeInTheDocument(); - // parent component didn't re-suspend - expect(renders.suspenseCount).toBe(1); + // parent component re-suspends + expect(renders.suspenseCount).toBe(2); expect(renders.count).toBe(3); // extra render puts an additional frame into the array From 1e3297e6952d7c9903c1bc289763db5ad50bc462 Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Thu, 30 Nov 2023 11:58:12 -0700 Subject: [PATCH 14/23] Separate withSequence from createFulfilledPromise and createRejectedPromise --- src/react/cache/QueryReference.ts | 18 ++++++++++++++---- src/utilities/promises/decoration.ts | 10 ++++------ 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/src/react/cache/QueryReference.ts b/src/react/cache/QueryReference.ts index c9ec24dac89..687e649cf0d 100644 --- a/src/react/cache/QueryReference.ts +++ b/src/react/cache/QueryReference.ts @@ -32,6 +32,15 @@ type FetchMoreOptions = Parameters< const QUERY_REFERENCE_SYMBOL: unique symbol = Symbol(); const PROMISE_SYMBOL: unique symbol = Symbol(); + +function createSequencedFulfilledPromise(value: TValue) { + return withSequence(createFulfilledPromise(value)); +} + +function createSequencedRejectedPromise(reason: unknown) { + return withSequence(createRejectedPromise(reason)); +} + /** * A `QueryReference` is an opaque object returned by {@link useBackgroundQuery}. * A child component reading the `QueryReference` via {@link useReadQuery} will @@ -131,7 +140,7 @@ export class InternalQueryReference { (this.result.data && (!this.result.partial || this.watchQueryOptions.returnPartialData)) ) { - this.promise = createFulfilledPromise(this.result); + this.promise = createSequencedFulfilledPromise(this.result); this.status = "idle"; } else { this.promise = withSequence( @@ -221,7 +230,7 @@ export class InternalQueryReference { if (currentCanonizeResults !== watchQueryOptions.canonizeResults) { this.result = { ...this.result, ...this.observable.getCurrentResult() }; - this.promise = createFulfilledPromise(this.result); + this.promise = createSequencedFulfilledPromise(this.result); } } @@ -281,7 +290,7 @@ export class InternalQueryReference { } this.result = result; - this.promise = createFulfilledPromise(result); + this.promise = createSequencedFulfilledPromise(result); this.deliver(this.promise); break; } @@ -302,7 +311,8 @@ export class InternalQueryReference { break; } case "idle": { - this.promise = createRejectedPromise>(error); + this.promise = + createSequencedRejectedPromise>(error); this.deliver(this.promise); } } diff --git a/src/utilities/promises/decoration.ts b/src/utilities/promises/decoration.ts index a19125185cc..424122ae0ab 100644 --- a/src/utilities/promises/decoration.ts +++ b/src/utilities/promises/decoration.ts @@ -40,18 +40,16 @@ export function withSequence>( } export function createFulfilledPromise(value: TValue) { - const promise = Promise.resolve(value) as FulfilledPromise & - WithSequence; + const promise = Promise.resolve(value) as FulfilledPromise; promise.status = "fulfilled"; promise.value = value; - return withSequence(promise); + return promise; } export function createRejectedPromise(reason: unknown) { - const promise = Promise.reject(reason) as RejectedPromise & - WithSequence; + const promise = Promise.reject(reason) as RejectedPromise; // prevent potential edge cases leaking unhandled error rejections promise.catch(() => {}); @@ -59,7 +57,7 @@ export function createRejectedPromise(reason: unknown) { promise.status = "rejected"; promise.reason = reason; - return withSequence(promise); + return promise; } export function isStatefulPromise( From a6e421ab47146c3e0d8f838214b10ca0c0e4982c Mon Sep 17 00:00:00 2001 From: Jerel Miller Date: Thu, 30 Nov 2023 17:12:43 -0700 Subject: [PATCH 15/23] Remove need for sequencing on promises by returning newer promise when its fulfilled --- src/react/cache/QueryReference.ts | 63 +++++++++++----------------- src/utilities/index.ts | 3 +- src/utilities/promises/decoration.ts | 22 ---------- 3 files changed, 26 insertions(+), 62 deletions(-) diff --git a/src/react/cache/QueryReference.ts b/src/react/cache/QueryReference.ts index 687e649cf0d..967b99062f5 100644 --- a/src/react/cache/QueryReference.ts +++ b/src/react/cache/QueryReference.ts @@ -10,7 +10,6 @@ import { isNetworkRequestSettled } from "../../core/index.js"; import type { ObservableSubscription, PromiseWithState, - WithSequence, } from "../../utilities/index.js"; import { createFulfilledPromise, @@ -18,11 +17,9 @@ import { } from "../../utilities/index.js"; import type { CacheKey, QueryKey } from "./types.js"; import type { useBackgroundQuery, useReadQuery } from "../hooks/index.js"; -import { withSequence, wrapPromiseWithState } from "../../utilities/index.js"; -import { secondIfNewerFulfilledOrFirst } from "../../utilities/promises/decoration.js"; +import { wrapPromiseWithState } from "../../utilities/index.js"; -type QueryRefPromise = PromiseWithState> & - WithSequence; +type QueryRefPromise = PromiseWithState>; type Listener = (promise: QueryRefPromise) => void; @@ -33,14 +30,6 @@ type FetchMoreOptions = Parameters< const QUERY_REFERENCE_SYMBOL: unique symbol = Symbol(); const PROMISE_SYMBOL: unique symbol = Symbol(); -function createSequencedFulfilledPromise(value: TValue) { - return withSequence(createFulfilledPromise(value)); -} - -function createSequencedRejectedPromise(reason: unknown) { - return withSequence(createRejectedPromise(reason)); -} - /** * A `QueryReference` is an opaque object returned by {@link useBackgroundQuery}. * A child component reading the `QueryReference` via {@link useReadQuery} will @@ -70,14 +59,17 @@ export function wrapQueryRef( export function unwrapQueryRef( queryRef: QueryReference ): [InternalQueryReference, () => QueryRefPromise] { - const reference = queryRef[QUERY_REFERENCE_SYMBOL]; + const internalQueryRef = queryRef[QUERY_REFERENCE_SYMBOL]; + return [ - reference, + internalQueryRef, () => - secondIfNewerFulfilledOrFirst( - queryRef[PROMISE_SYMBOL], - reference.promise - ), + // There is a chance the query ref's promise has been updated in the time + // the original promise had been suspended. In that case, we want to use + // it instead of the older promise which may contain outdated data. + internalQueryRef.promise.status === "fulfilled" + ? internalQueryRef.promise + : queryRef[PROMISE_SYMBOL], ]; } @@ -140,16 +132,14 @@ export class InternalQueryReference { (this.result.data && (!this.result.partial || this.watchQueryOptions.returnPartialData)) ) { - this.promise = createSequencedFulfilledPromise(this.result); + this.promise = createFulfilledPromise(this.result); this.status = "idle"; } else { - this.promise = withSequence( - wrapPromiseWithState( - new Promise((resolve, reject) => { - this.resolve = resolve; - this.reject = reject; - }) - ) + this.promise = wrapPromiseWithState( + new Promise((resolve, reject) => { + this.resolve = resolve; + this.reject = reject; + }) ); } @@ -230,7 +220,7 @@ export class InternalQueryReference { if (currentCanonizeResults !== watchQueryOptions.canonizeResults) { this.result = { ...this.result, ...this.observable.getCurrentResult() }; - this.promise = createSequencedFulfilledPromise(this.result); + this.promise = createFulfilledPromise(this.result); } } @@ -290,7 +280,7 @@ export class InternalQueryReference { } this.result = result; - this.promise = createSequencedFulfilledPromise(result); + this.promise = createFulfilledPromise(result); this.deliver(this.promise); break; } @@ -311,8 +301,7 @@ export class InternalQueryReference { break; } case "idle": { - this.promise = - createSequencedRejectedPromise>(error); + this.promise = createRejectedPromise>(error); this.deliver(this.promise); } } @@ -325,13 +314,11 @@ export class InternalQueryReference { private initiateFetch(returnedPromise: Promise>) { this.status = "loading"; - this.promise = withSequence( - wrapPromiseWithState( - new Promise((resolve, reject) => { - this.resolve = resolve; - this.reject = reject; - }) - ) + this.promise = wrapPromiseWithState( + new Promise((resolve, reject) => { + this.resolve = resolve; + this.reject = reject; + }) ); this.promise.catch(() => {}); diff --git a/src/utilities/index.ts b/src/utilities/index.ts index 0e70e465fdb..0b7083fddd5 100644 --- a/src/utilities/index.ts +++ b/src/utilities/index.ts @@ -98,13 +98,12 @@ export type { } from "./observables/Observable.js"; export { Observable } from "./observables/Observable.js"; -export type { PromiseWithState, WithSequence } from "./promises/decoration.js"; +export type { PromiseWithState } from "./promises/decoration.js"; export { isStatefulPromise, createFulfilledPromise, createRejectedPromise, wrapPromiseWithState, - withSequence, } from "./promises/decoration.js"; export * from "./common/mergeDeep.js"; diff --git a/src/utilities/promises/decoration.ts b/src/utilities/promises/decoration.ts index 424122ae0ab..9022aeed23c 100644 --- a/src/utilities/promises/decoration.ts +++ b/src/utilities/promises/decoration.ts @@ -12,33 +12,11 @@ export interface RejectedPromise extends Promise { reason: unknown; } -export interface WithSequence { - [sequence]: number; -} - -export function secondIfNewerFulfilledOrFirst( - first: PromiseWithState & WithSequence, - second: PromiseWithState & WithSequence -): PromiseWithState & WithSequence { - return second.status === "fulfilled" && second[sequence] > first[sequence] - ? second - : first; -} - -let current = 0; -const sequence = Symbol("sequence"); - export type PromiseWithState = | PendingPromise | FulfilledPromise | RejectedPromise; -export function withSequence>( - promise: T -): T & WithSequence { - return Object.assign(promise, { [sequence]: current++ }); -} - export function createFulfilledPromise(value: TValue) { const promise = Promise.resolve(value) as FulfilledPromise; From 82bd4ae04fb9f77f367f30aa6f595d17a9b2b6fa Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Fri, 1 Dec 2023 10:57:50 +0100 Subject: [PATCH 16/23] `useLoadableQuery`: remove `promiseCache` --- src/react/hooks/useLoadableQuery.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/react/hooks/useLoadableQuery.ts b/src/react/hooks/useLoadableQuery.ts index 2ec551bf26e..b6c3f5795c2 100644 --- a/src/react/hooks/useLoadableQuery.ts +++ b/src/react/hooks/useLoadableQuery.ts @@ -133,10 +133,6 @@ export function useLoadableQuery< promiseCache.set(queryRef.key, promise); } - if (queryRef) { - queryRef.promiseCache = promiseCache; - } - const calledDuringRender = useRenderGuard(); React.useEffect(() => queryRef?.retain(), [queryRef]); From f8e0e05c965154feeea26e9e92d87fe8cec3549d Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Fri, 1 Dec 2023 10:58:06 +0100 Subject: [PATCH 17/23] remove second argument to `wrapQueryRef` --- src/react/cache/QueryReference.ts | 5 ++--- src/react/hooks/useBackgroundQuery.ts | 8 ++++---- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/react/cache/QueryReference.ts b/src/react/cache/QueryReference.ts index 967b99062f5..ac78d49d78e 100644 --- a/src/react/cache/QueryReference.ts +++ b/src/react/cache/QueryReference.ts @@ -47,12 +47,11 @@ interface InternalQueryReferenceOptions { } export function wrapQueryRef( - internalQueryRef: InternalQueryReference, - promise: QueryRefPromise + internalQueryRef: InternalQueryReference ): QueryReference { return { [QUERY_REFERENCE_SYMBOL]: internalQueryRef, - [PROMISE_SYMBOL]: promise, + [PROMISE_SYMBOL]: internalQueryRef.promise, }; } diff --git a/src/react/hooks/useBackgroundQuery.ts b/src/react/hooks/useBackgroundQuery.ts index 6bb353556d3..cd0317989a1 100644 --- a/src/react/hooks/useBackgroundQuery.ts +++ b/src/react/hooks/useBackgroundQuery.ts @@ -207,10 +207,10 @@ export function useBackgroundQuery< ); const [wrappedQueryRef, setWrappedQueryRef] = React.useState( - wrapQueryRef(queryRef, queryRef.promise) + wrapQueryRef(queryRef) ); if (unwrapQueryRef(wrappedQueryRef)[0] !== queryRef) { - setWrappedQueryRef(wrapQueryRef(queryRef, queryRef.promise)); + setWrappedQueryRef(wrapQueryRef(queryRef)); } if (queryRef.didChangeOptions(watchQueryOptions)) { const promise = queryRef.applyOptions(watchQueryOptions); @@ -223,7 +223,7 @@ export function useBackgroundQuery< (options) => { const promise = queryRef.fetchMore(options as FetchMoreQueryOptions); - setWrappedQueryRef(wrapQueryRef(queryRef, queryRef.promise)); + setWrappedQueryRef(wrapQueryRef(queryRef)); return promise; }, @@ -234,7 +234,7 @@ export function useBackgroundQuery< (variables) => { const promise = queryRef.refetch(variables); - setWrappedQueryRef(wrapQueryRef(queryRef, queryRef.promise)); + setWrappedQueryRef(wrapQueryRef(queryRef)); return promise; }, From 10175b5802e0d51c2cb364a5996f7b97683d0cfe Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Fri, 1 Dec 2023 10:59:57 +0100 Subject: [PATCH 18/23] chores --- .api-reports/api-report-react.md | 67 ++++++++++++++++++++++---- .api-reports/api-report-react_hooks.md | 67 ++++++++++++++++++++++---- .api-reports/api-report-utilities.md | 4 +- .api-reports/api-report.md | 67 ++++++++++++++++++++++---- .size-limits.json | 4 +- 5 files changed, 174 insertions(+), 35 deletions(-) diff --git a/.api-reports/api-report-react.md b/.api-reports/api-report-react.md index 65a4bbe7d1f..bf3074e82bf 100644 --- a/.api-reports/api-report-react.md +++ b/.api-reports/api-report-react.md @@ -855,6 +855,14 @@ interface FragmentMap { // @public (undocumented) type FragmentMatcher = (rootValue: any, typeCondition: string, context: any) => boolean; +// @public (undocumented) +interface FulfilledPromise extends Promise { + // (undocumented) + status: "fulfilled"; + // (undocumented) + value: TValue; +} + // @public (undocumented) export function getApolloContext(): ReactTypes.Context; @@ -906,7 +914,7 @@ class InternalQueryReference { // Warning: (ae-forgotten-export) The symbol "InternalQueryReferenceOptions" needs to be exported by the entry point index.d.ts constructor(observable: ObservableQuery, options: InternalQueryReferenceOptions); // (undocumented) - applyOptions(watchQueryOptions: ObservedOptions): Promise>; + applyOptions(watchQueryOptions: ObservedOptions): QueryRefPromise; // Warning: (ae-forgotten-export) The symbol "ObservedOptions" needs to be exported by the entry point index.d.ts // // (undocumented) @@ -915,20 +923,20 @@ class InternalQueryReference { // // (undocumented) fetchMore(options: FetchMoreOptions): Promise>; - // 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) listen(listener: Listener): () => void; // (undocumented) readonly observable: ObservableQuery; + // Warning: (ae-forgotten-export) The symbol "QueryRefPromise" needs to be exported by the entry point index.d.ts + // // (undocumented) - promise: Promise>; - // (undocumented) - promiseCache?: Map>>; + promise: QueryRefPromise; // (undocumented) refetch(variables: OperationVariables | undefined): Promise>; // (undocumented) @@ -943,6 +951,8 @@ class InternalQueryReference { interface InternalQueryReferenceOptions { // (undocumented) autoDisposeTimeoutMs?: number; + // Warning: (ae-forgotten-export) The symbol "CacheKey" needs to be exported by the entry point index.d.ts + // // (undocumented) key: CacheKey; // (undocumented) @@ -1016,7 +1026,7 @@ export type LazyQueryResult = Quer export type LazyQueryResultTuple = [LazyQueryExecFunction, QueryResult]; // @public (undocumented) -type Listener = (promise: Promise>) => void; +type Listener = (promise: QueryRefPromise) => void; // @public (undocumented) export type LoadableQueryHookFetchPolicy = Extract; @@ -1441,9 +1451,25 @@ export namespace parser { // @public (undocumented) type Path = ReadonlyArray; +// @public (undocumented) +interface PendingPromise extends Promise { + // (undocumented) + status: "pending"; +} + // @public (undocumented) type Primitive = null | undefined | string | number | boolean | symbol | bigint; +// @public (undocumented) +const PROMISE_SYMBOL: unique symbol; + +// Warning: (ae-forgotten-export) The symbol "PendingPromise" needs to be exported by the entry point index.d.ts +// Warning: (ae-forgotten-export) The symbol "FulfilledPromise" needs to be exported by the entry point index.d.ts +// Warning: (ae-forgotten-export) The symbol "RejectedPromise" needs to be exported by the entry point index.d.ts +// +// @public (undocumented) +type PromiseWithState = PendingPromise | FulfilledPromise | RejectedPromise; + // @public (undocumented) const QUERY_REFERENCE_SYMBOL: unique symbol; @@ -1536,6 +1562,12 @@ class QueryInfo { variables?: Record; } +// @public (undocumented) +interface QueryKey { + // (undocumented) + __queryKey?: string; +} + // @public (undocumented) export interface QueryLazyOptions { // (undocumented) @@ -1696,12 +1728,19 @@ interface QueryOptions { // @public (undocumented) export interface QueryReference { + // (undocumented) + [PROMISE_SYMBOL]: QueryRefPromise; // Warning: (ae-forgotten-export) The symbol "InternalQueryReference" needs to be exported by the entry point index.d.ts // // (undocumented) - [QUERY_REFERENCE_SYMBOL]: InternalQueryReference; + readonly [QUERY_REFERENCE_SYMBOL]: InternalQueryReference; } +// Warning: (ae-forgotten-export) The symbol "PromiseWithState" needs to be exported by the entry point index.d.ts +// +// @public (undocumented) +type QueryRefPromise = PromiseWithState>; + // @public (undocumented) export interface QueryResult extends ObservableQueryFields { // (undocumented) @@ -1815,6 +1854,14 @@ type RefetchQueryDescriptor = string | DocumentNode; // @public (undocumented) type RefetchWritePolicy = "merge" | "overwrite"; +// @public (undocumented) +interface RejectedPromise extends Promise { + // (undocumented) + reason: unknown; + // (undocumented) + status: "rejected"; +} + // @public (undocumented) class RenderPromises { // (undocumented) @@ -2294,8 +2341,8 @@ interface WatchQueryOptions boolean; +// @public (undocumented) +interface FulfilledPromise extends Promise { + // (undocumented) + status: "fulfilled"; + // (undocumented) + value: TValue; +} + // @public (undocumented) type GraphQLErrors = ReadonlyArray; @@ -853,7 +861,7 @@ class InternalQueryReference { // Warning: (ae-forgotten-export) The symbol "InternalQueryReferenceOptions" needs to be exported by the entry point index.d.ts constructor(observable: ObservableQuery, options: InternalQueryReferenceOptions); // (undocumented) - applyOptions(watchQueryOptions: ObservedOptions): Promise>; + applyOptions(watchQueryOptions: ObservedOptions): QueryRefPromise; // Warning: (ae-forgotten-export) The symbol "ObservedOptions" needs to be exported by the entry point index.d.ts // // (undocumented) @@ -862,20 +870,20 @@ class InternalQueryReference { // // (undocumented) fetchMore(options: FetchMoreOptions): Promise>; - // 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) listen(listener: Listener): () => void; // (undocumented) readonly observable: ObservableQuery; + // Warning: (ae-forgotten-export) The symbol "QueryRefPromise" needs to be exported by the entry point index.d.ts + // // (undocumented) - promise: Promise>; - // (undocumented) - promiseCache?: Map>>; + promise: QueryRefPromise; // (undocumented) refetch(variables: OperationVariables | undefined): Promise>; // (undocumented) @@ -890,6 +898,8 @@ class InternalQueryReference { interface InternalQueryReferenceOptions { // (undocumented) autoDisposeTimeoutMs?: number; + // Warning: (ae-forgotten-export) The symbol "CacheKey" needs to be exported by the entry point index.d.ts + // // (undocumented) key: CacheKey; // (undocumented) @@ -967,7 +977,7 @@ interface LazyQueryHookOptions = [LazyQueryExecFunction, QueryResult]; // @public (undocumented) -type Listener = (promise: Promise>) => void; +type Listener = (promise: QueryRefPromise) => void; // @public (undocumented) type LoadableQueryHookFetchPolicy = Extract; @@ -1380,9 +1390,25 @@ type OperationVariables = Record; // @public (undocumented) type Path = ReadonlyArray; +// @public (undocumented) +interface PendingPromise extends Promise { + // (undocumented) + status: "pending"; +} + // @public (undocumented) type Primitive = null | undefined | string | number | boolean | symbol | bigint; +// @public (undocumented) +const PROMISE_SYMBOL: unique symbol; + +// Warning: (ae-forgotten-export) The symbol "PendingPromise" needs to be exported by the entry point index.d.ts +// Warning: (ae-forgotten-export) The symbol "FulfilledPromise" needs to be exported by the entry point index.d.ts +// Warning: (ae-forgotten-export) The symbol "RejectedPromise" needs to be exported by the entry point index.d.ts +// +// @public (undocumented) +type PromiseWithState = PendingPromise | FulfilledPromise | RejectedPromise; + // @public (undocumented) const QUERY_REFERENCE_SYMBOL: unique symbol; @@ -1463,6 +1489,12 @@ class QueryInfo { variables?: Record; } +// @public (undocumented) +interface QueryKey { + // (undocumented) + __queryKey?: string; +} + // @public (undocumented) type QueryListener = (queryInfo: QueryInfo) => void; @@ -1615,12 +1647,19 @@ interface QueryOptions { // @public (undocumented) interface QueryReference { + // (undocumented) + [PROMISE_SYMBOL]: QueryRefPromise; // Warning: (ae-forgotten-export) The symbol "InternalQueryReference" needs to be exported by the entry point index.d.ts // // (undocumented) - [QUERY_REFERENCE_SYMBOL]: InternalQueryReference; + readonly [QUERY_REFERENCE_SYMBOL]: InternalQueryReference; } +// Warning: (ae-forgotten-export) The symbol "PromiseWithState" needs to be exported by the entry point index.d.ts +// +// @public (undocumented) +type QueryRefPromise = PromiseWithState>; + // Warning: (ae-forgotten-export) The symbol "ObservableQueryFields" needs to be exported by the entry point index.d.ts // // @public (undocumented) @@ -1730,6 +1769,14 @@ type RefetchQueryDescriptor = string | DocumentNode; // @public (undocumented) type RefetchWritePolicy = "merge" | "overwrite"; +// @public (undocumented) +interface RejectedPromise extends Promise { + // (undocumented) + reason: unknown; + // (undocumented) + status: "rejected"; +} + // @public (undocumented) type RequestHandler = (operation: Operation, forward: NextLink) => Observable | null; @@ -2186,8 +2233,8 @@ interface WatchQueryOptions(promise: Promise): promise is PromiseWithState; @@ -1902,7 +1900,7 @@ export { print_2 as print } // Warning: (ae-forgotten-export) The symbol "PendingPromise" needs to be exported by the entry point index.d.ts // // @public (undocumented) -type PromiseWithState = PendingPromise | FulfilledPromise | RejectedPromise; +export type PromiseWithState = PendingPromise | FulfilledPromise | RejectedPromise; // @public (undocumented) class QueryInfo { diff --git a/.api-reports/api-report.md b/.api-reports/api-report.md index 9e336a0c50a..d2de3fd0423 100644 --- a/.api-reports/api-report.md +++ b/.api-reports/api-report.md @@ -1077,6 +1077,14 @@ export function fromError(errorValue: any): Observable; // @public (undocumented) export function fromPromise(promise: Promise): Observable; +// @public (undocumented) +interface FulfilledPromise extends Promise { + // (undocumented) + status: "fulfilled"; + // (undocumented) + value: TValue; +} + // @public (undocumented) export function getApolloContext(): ReactTypes.Context; @@ -1266,7 +1274,7 @@ class InternalQueryReference { // Warning: (ae-forgotten-export) The symbol "InternalQueryReferenceOptions" needs to be exported by the entry point index.d.ts constructor(observable: ObservableQuery, options: InternalQueryReferenceOptions); // (undocumented) - applyOptions(watchQueryOptions: ObservedOptions): Promise>; + applyOptions(watchQueryOptions: ObservedOptions): QueryRefPromise; // Warning: (ae-forgotten-export) The symbol "ObservedOptions" needs to be exported by the entry point index.d.ts // // (undocumented) @@ -1275,20 +1283,20 @@ class InternalQueryReference { // // (undocumented) fetchMore(options: FetchMoreOptions_2): Promise>; - // 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) listen(listener: Listener): () => void; // (undocumented) readonly observable: ObservableQuery; + // Warning: (ae-forgotten-export) The symbol "QueryRefPromise" needs to be exported by the entry point index.d.ts + // // (undocumented) - promise: Promise>; - // (undocumented) - promiseCache?: Map>>; + promise: QueryRefPromise; // (undocumented) refetch(variables: OperationVariables | undefined): Promise>; // (undocumented) @@ -1303,6 +1311,8 @@ class InternalQueryReference { interface InternalQueryReferenceOptions { // (undocumented) autoDisposeTimeoutMs?: number; + // Warning: (ae-forgotten-export) The symbol "CacheKey" needs to be exported by the entry point index.d.ts + // // (undocumented) key: CacheKey; // (undocumented) @@ -1422,7 +1432,7 @@ export type LazyQueryResult = Quer export type LazyQueryResultTuple = [LazyQueryExecFunction, QueryResult]; // @public (undocumented) -type Listener = (promise: Promise>) => void; +type Listener = (promise: QueryRefPromise) => void; // @public (undocumented) export type LoadableQueryHookFetchPolicy = Extract; @@ -1940,6 +1950,12 @@ export namespace parser { // @public (undocumented) export type Path = ReadonlyArray; +// @public (undocumented) +interface PendingPromise extends Promise { + // (undocumented) + status: "pending"; +} + // @public (undocumented) class Policies { constructor(config: { @@ -2003,6 +2019,16 @@ interface Printer { (node: ASTNode, originalPrint: typeof print_2): string; } +// @public (undocumented) +const PROMISE_SYMBOL: unique symbol; + +// Warning: (ae-forgotten-export) The symbol "PendingPromise" needs to be exported by the entry point index.d.ts +// Warning: (ae-forgotten-export) The symbol "FulfilledPromise" needs to be exported by the entry point index.d.ts +// Warning: (ae-forgotten-export) The symbol "RejectedPromise" needs to be exported by the entry point index.d.ts +// +// @public (undocumented) +type PromiseWithState = PendingPromise | FulfilledPromise | RejectedPromise; + // @public (undocumented) const QUERY_REFERENCE_SYMBOL: unique symbol; @@ -2093,6 +2119,12 @@ class QueryInfo { variables?: Record; } +// @public (undocumented) +interface QueryKey { + // (undocumented) + __queryKey?: string; +} + // @public (undocumented) export interface QueryLazyOptions { // (undocumented) @@ -2250,12 +2282,19 @@ export { QueryOptions } // @public (undocumented) export interface QueryReference { + // (undocumented) + [PROMISE_SYMBOL]: QueryRefPromise; // Warning: (ae-forgotten-export) The symbol "InternalQueryReference" needs to be exported by the entry point index.d.ts // // (undocumented) - [QUERY_REFERENCE_SYMBOL]: InternalQueryReference; + readonly [QUERY_REFERENCE_SYMBOL]: InternalQueryReference; } +// Warning: (ae-forgotten-export) The symbol "PromiseWithState" needs to be exported by the entry point index.d.ts +// +// @public (undocumented) +type QueryRefPromise = PromiseWithState>; + // @public (undocumented) export interface QueryResult extends ObservableQueryFields { // (undocumented) @@ -2383,6 +2422,14 @@ export type RefetchQueryDescriptor = string | DocumentNode; // @public (undocumented) export type RefetchWritePolicy = "merge" | "overwrite"; +// @public (undocumented) +interface RejectedPromise extends Promise { + // (undocumented) + reason: unknown; + // (undocumented) + status: "rejected"; +} + // @public (undocumented) class RenderPromises { // (undocumented) @@ -2963,8 +3010,8 @@ interface WriteContext extends ReadMergeModifyContext { // src/core/QueryManager.ts:384:7 - (ae-forgotten-export) The symbol "UpdateQueries" needs to be exported by the entry point index.d.ts // src/core/watchQueryOptions.ts:191:3 - (ae-forgotten-export) The symbol "UpdateQueryFn" needs to be exported by the entry point index.d.ts // src/link/http/selectHttpOptionsAndBody.ts:128:32 - (ae-forgotten-export) The symbol "HttpQueryOptions" needs to be exported by the entry point index.d.ts -// src/react/hooks/useBackgroundQuery.ts:26:3 - (ae-forgotten-export) The symbol "FetchMoreFunction" needs to be exported by the entry point index.d.ts -// src/react/hooks/useBackgroundQuery.ts:27:3 - (ae-forgotten-export) The symbol "RefetchFunction" needs to be exported by the entry point index.d.ts +// src/react/hooks/useBackgroundQuery.ts:30:3 - (ae-forgotten-export) The symbol "FetchMoreFunction" needs to be exported by the entry point index.d.ts +// src/react/hooks/useBackgroundQuery.ts:31:3 - (ae-forgotten-export) The symbol "RefetchFunction" needs to be exported by the entry point index.d.ts // src/react/hooks/useLoadableQuery.ts:51:5 - (ae-forgotten-export) The symbol "ResetFunction" needs to be exported by the entry point index.d.ts // (No @packageDocumentation comment for this package) diff --git a/.size-limits.json b/.size-limits.json index e196670f752..a75e45c32dc 100644 --- a/.size-limits.json +++ b/.size-limits.json @@ -1,4 +1,4 @@ { - "dist/apollo-client.min.cjs": 38632, - "import { ApolloClient, InMemoryCache, HttpLink } from \"dist/index.js\" (production)": 32215 + "dist/apollo-client.min.cjs": 38607, + "import { ApolloClient, InMemoryCache, HttpLink } from \"dist/index.js\" (production)": 32214 } From 4a413c32b80ee86d4c60a47923522fba13e03b9b Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Fri, 1 Dec 2023 13:14:14 +0100 Subject: [PATCH 19/23] fix most tests by adding delays --- .../hooks/__tests__/useLoadableQuery.test.tsx | 43 +++++++++++++++---- 1 file changed, 35 insertions(+), 8 deletions(-) diff --git a/src/react/hooks/__tests__/useLoadableQuery.test.tsx b/src/react/hooks/__tests__/useLoadableQuery.test.tsx index e82dc181f66..b6f79c2d034 100644 --- a/src/react/hooks/__tests__/useLoadableQuery.test.tsx +++ b/src/react/hooks/__tests__/useLoadableQuery.test.tsx @@ -497,16 +497,24 @@ it("allows the client to be overridden", async () => { const { query } = useSimpleQueryCase(); const globalClient = new ApolloClient({ - link: new ApolloLink(() => - Observable.of({ data: { greeting: "global hello" } }) - ), + link: new MockLink([ + { + request: { query }, + result: { data: { greeting: "global hello" } }, + delay: 10, + }, + ]), cache: new InMemoryCache(), }); const localClient = new ApolloClient({ - link: new ApolloLink(() => - Observable.of({ data: { greeting: "local hello" } }) - ), + link: new MockLink([ + { + request: { query }, + result: { data: { greeting: "local hello" } }, + delay: 10, + }, + ]), cache: new InMemoryCache(), }); @@ -566,9 +574,10 @@ it("passes context to the link", async () => { link: new ApolloLink((operation) => { return new Observable((observer) => { const { valueA, valueB } = operation.getContext(); - + setTimeout(() => { observer.next({ data: { context: { valueA, valueB } } }); observer.complete(); + }, 10); }); }), }); @@ -1460,12 +1469,14 @@ it("applies `errorPolicy` on next fetch when it changes between renders", async { request: { query }, result: { data: { greeting: "Hello" } }, + delay: 10, }, { request: { query }, result: { errors: [new GraphQLError("oops")], }, + delay: 10, }, ]; @@ -1545,10 +1556,15 @@ it("applies `context` on next fetch when it changes between renders", async () = `; const link = new ApolloLink((operation) => { - return Observable.of({ + return new Observable((subscriber) => { + setTimeout(() => { + subscriber.next({ data: { phase: operation.getContext().phase, }, + }); + subscriber.complete(); + }, 10); }); }); @@ -1728,6 +1744,7 @@ it("applies changed `refetchWritePolicy` to next fetch when changing between ren { request: { query, variables: { min: 0, max: 12 } }, result: { data: { primes: [2, 3, 5, 7, 11] } }, + delay: 10, }, { request: { query, variables: { min: 12, max: 30 } }, @@ -1899,6 +1916,7 @@ it("applies `returnPartialData` on next fetch when it changes between renders", }, }, }, + delay: 10, }, { request: { query: fullQuery }, @@ -2241,12 +2259,14 @@ it("re-suspends when calling `refetch` with new variables", async () => { result: { data: { character: { id: "1", name: "Captain Marvel" } }, }, + delay: 10, }, { request: { query, variables: { id: "2" } }, result: { data: { character: { id: "2", name: "Captain America" } }, }, + delay: 10, }, ]; @@ -2323,6 +2343,7 @@ it("re-suspends multiple times when calling `refetch` multiple times", async () data: { character: { id: "1", name: "Spider-Man" } }, }, maxUsageCount: 3, + delay: 10, }, ]; @@ -2495,12 +2516,14 @@ it('ignores errors returned after calling `refetch` when errorPolicy is set to " result: { data: { character: { id: "1", name: "Captain Marvel" } }, }, + delay: 10, }, { request: { query, variables: { id: "1" } }, result: { errors: [new GraphQLError("Something went wrong")], }, + delay: 10, }, ]; @@ -3273,6 +3296,7 @@ it('honors refetchWritePolicy set to "merge"', async () => { { request: { query, variables: { min: 0, max: 12 } }, result: { data: { primes: [2, 3, 5, 7, 11] } }, + delay: 10, }, { request: { query, variables: { min: 12, max: 30 } }, @@ -3385,6 +3409,7 @@ it('defaults refetchWritePolicy to "overwrite"', async () => { { request: { query, variables: { min: 0, max: 12 } }, result: { data: { primes: [2, 3, 5, 7, 11] } }, + delay: 10, }, { request: { query, variables: { min: 12, max: 30 } }, @@ -3695,6 +3720,7 @@ it('suspends when partial data is in the cache and using a "network-only" fetch { request: { query: fullQuery }, result: { data: { character: { id: "1", name: "Doctor Strange" } } }, + delay: 10, }, ]; @@ -3786,6 +3812,7 @@ it('suspends when partial data is in the cache and using a "no-cache" fetch poli { request: { query: fullQuery }, result: { data: { character: { id: "1", name: "Doctor Strange" } } }, + delay: 10, }, ]; From 8010b0c1b5d8e99123efca141cd385913de919a8 Mon Sep 17 00:00:00 2001 From: phryneas Date: Fri, 1 Dec 2023 12:16:37 +0000 Subject: [PATCH 20/23] Clean up Prettier, Size-limit, and Api-Extractor --- src/react/hooks/__tests__/useLoadableQuery.test.tsx | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/react/hooks/__tests__/useLoadableQuery.test.tsx b/src/react/hooks/__tests__/useLoadableQuery.test.tsx index b6f79c2d034..e0eb3fc2350 100644 --- a/src/react/hooks/__tests__/useLoadableQuery.test.tsx +++ b/src/react/hooks/__tests__/useLoadableQuery.test.tsx @@ -575,8 +575,8 @@ it("passes context to the link", async () => { return new Observable((observer) => { const { valueA, valueB } = operation.getContext(); setTimeout(() => { - observer.next({ data: { context: { valueA, valueB } } }); - observer.complete(); + observer.next({ data: { context: { valueA, valueB } } }); + observer.complete(); }, 10); }); }), @@ -1559,9 +1559,9 @@ it("applies `context` on next fetch when it changes between renders", async () = return new Observable((subscriber) => { setTimeout(() => { subscriber.next({ - data: { - phase: operation.getContext().phase, - }, + data: { + phase: operation.getContext().phase, + }, }); subscriber.complete(); }, 10); From c1503986ee0e5e716ddce3465c01df127a61f5f9 Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Fri, 1 Dec 2023 14:27:24 +0100 Subject: [PATCH 21/23] add `skipNonTrackingRenders` to profiler to account for "empty" renders that don't actually render any components we're interested in testing --- .../hooks/__tests__/useLoadableQuery.test.tsx | 18 +++++++++++++++ src/testing/internal/profile/profile.tsx | 23 +++++++++++-------- 2 files changed, 31 insertions(+), 10 deletions(-) diff --git a/src/react/hooks/__tests__/useLoadableQuery.test.tsx b/src/react/hooks/__tests__/useLoadableQuery.test.tsx index e0eb3fc2350..2c46087be1a 100644 --- a/src/react/hooks/__tests__/useLoadableQuery.test.tsx +++ b/src/react/hooks/__tests__/useLoadableQuery.test.tsx @@ -162,6 +162,7 @@ function createDefaultProfiler() { error: null as Error | null, result: null as UseReadQueryResult | null, }, + skipNonTrackingRenders: true, }); } @@ -524,6 +525,7 @@ it("allows the client to be overridden", async () => { createDefaultProfiledComponents(Profiler); function App() { + useTrackRenders(); const [loadQuery, queryRef] = useLoadableQuery(query, { client: localClient, }); @@ -588,6 +590,7 @@ it("passes context to the link", async () => { createDefaultProfiledComponents(Profiler); function App() { + useTrackRenders(); const [loadQuery, queryRef] = useLoadableQuery(query, { context: { valueA: "A", valueB: "B" }, }); @@ -670,6 +673,7 @@ it('enables canonical results when canonizeResults is "true"', async () => { createDefaultProfiledComponents(Profiler); function App() { + useTrackRenders(); const [loadQuery, queryRef] = useLoadableQuery(query, { canonizeResults: true, }); @@ -751,6 +755,7 @@ it("can disable canonical results when the cache's canonizeResults setting is tr createDefaultProfiledComponents(Profiler); function App() { + useTrackRenders(); const [loadQuery, queryRef] = useLoadableQuery(query, { canonizeResults: false, }); @@ -1485,6 +1490,7 @@ it("applies `errorPolicy` on next fetch when it changes between renders", async createDefaultProfiledComponents(Profiler); function App() { + useTrackRenders(); const [errorPolicy, setErrorPolicy] = useState("none"); const [loadQuery, queryRef, { refetch }] = useLoadableQuery(query, { errorPolicy, @@ -1578,6 +1584,7 @@ it("applies `context` on next fetch when it changes between renders", async () = createDefaultProfiledComponents(Profiler); function App() { + useTrackRenders(); const [phase, setPhase] = React.useState("initial"); const [loadQuery, queryRef, { refetch }] = useLoadableQuery(query, { context: { phase }, @@ -1679,6 +1686,7 @@ it("returns canonical results immediately when `canonizeResults` changes from `f createDefaultProfiledComponents(Profiler); function App() { + useTrackRenders(); const [canonizeResults, setCanonizeResults] = React.useState(false); const [loadQuery, queryRef] = useLoadableQuery(query, { canonizeResults, @@ -1786,6 +1794,7 @@ it("applies changed `refetchWritePolicy` to next fetch when changing between ren createDefaultProfiledComponents(Profiler); function App() { + useTrackRenders(); const [refetchWritePolicy, setRefetchWritePolicy] = React.useState("merge"); @@ -2089,6 +2098,7 @@ it("applies updated `fetchPolicy` on next fetch when it changes between renders" createDefaultProfiledComponents(Profiler); function App() { + useTrackRenders(); const [fetchPolicy, setFetchPolicy] = React.useState("cache-first"); @@ -2459,6 +2469,7 @@ it("throws errors when errors are returned after calling `refetch`", async () => createDefaultProfiledComponents(Profiler); function App() { + useTrackRenders(); const [loadQuery, queryRef, { refetch }] = useLoadableQuery(query); return ( @@ -2533,6 +2544,7 @@ it('ignores errors returned after calling `refetch` when errorPolicy is set to " createDefaultProfiledComponents(Profiler); function App() { + useTrackRenders(); const [loadQuery, queryRef, { refetch }] = useLoadableQuery(query, { errorPolicy: "ignore", }); @@ -2613,6 +2625,7 @@ it('returns errors after calling `refetch` when errorPolicy is set to "all"', as createDefaultProfiledComponents(Profiler); function App() { + useTrackRenders(); const [loadQuery, queryRef, { refetch }] = useLoadableQuery(query, { errorPolicy: "all", }); @@ -2695,6 +2708,7 @@ it('handles partial data results after calling `refetch` when errorPolicy is set createDefaultProfiledComponents(Profiler); function App() { + useTrackRenders(); const [loadQuery, queryRef, { refetch }] = useLoadableQuery(query, { errorPolicy: "all", }); @@ -2960,6 +2974,7 @@ it("properly uses `updateQuery` when calling `fetchMore`", async () => { createDefaultProfiledComponents(Profiler); function App() { + useTrackRenders(); const [loadQuery, queryRef, { fetchMore }] = useLoadableQuery(query); return ( @@ -3050,6 +3065,7 @@ it("properly uses cache field policies when calling `fetchMore` without `updateQ }); function App() { + useTrackRenders(); const [loadQuery, queryRef, { fetchMore }] = useLoadableQuery(query); return ( @@ -3332,6 +3348,7 @@ it('honors refetchWritePolicy set to "merge"', async () => { }); function App() { + useTrackRenders(); const [loadQuery, queryRef, { refetch }] = useLoadableQuery(query, { refetchWritePolicy: "merge", }); @@ -3445,6 +3462,7 @@ it('defaults refetchWritePolicy to "overwrite"', async () => { }); function App() { + useTrackRenders(); const [loadQuery, queryRef, { refetch }] = useLoadableQuery(query); return ( diff --git a/src/testing/internal/profile/profile.tsx b/src/testing/internal/profile/profile.tsx index 4b2717dc21d..63f69c78866 100644 --- a/src/testing/internal/profile/profile.tsx +++ b/src/testing/internal/profile/profile.tsx @@ -98,17 +98,8 @@ export interface ProfiledComponent export function profile({ Component, ...options -}: { - onRender?: ( - info: BaseRender & { - snapshot: Snapshot; - replaceSnapshot: ReplaceSnapshot; - mergeSnapshot: MergeSnapshot; - } - ) => void; +}: Parameters>[0] & { Component: React.ComponentType; - snapshotDOM?: boolean; - initialSnapshot?: Snapshot; }): ProfiledComponent { const Profiler = createProfiler(options); @@ -140,6 +131,7 @@ export function createProfiler({ onRender, snapshotDOM = false, initialSnapshot, + skipNonTrackingRenders, }: { onRender?: ( info: BaseRender & { @@ -150,6 +142,11 @@ export function createProfiler({ ) => void; snapshotDOM?: boolean; initialSnapshot?: Snapshot; + /** + * This will skip renders during which no renders tracked by + * `useTrackRenders` occured. + */ + skipNonTrackingRenders?: boolean; } = {}) { let nextRender: Promise> | undefined; let resolveNextRender: ((render: Render) => void) | undefined; @@ -194,6 +191,12 @@ export function createProfiler({ startTime, commitTime ) => { + if ( + skipNonTrackingRenders && + profilerContext.renderedComponents.length === 0 + ) { + return; + } const baseRender = { id, phase, From d07aa6add6b7e85f8832ba5cebefacc7adf17e0e Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Fri, 1 Dec 2023 18:57:08 +0100 Subject: [PATCH 22/23] add comments to "phantom render" tests --- src/react/hooks/__tests__/useLoadableQuery.test.tsx | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/react/hooks/__tests__/useLoadableQuery.test.tsx b/src/react/hooks/__tests__/useLoadableQuery.test.tsx index 2c46087be1a..cc773401510 100644 --- a/src/react/hooks/__tests__/useLoadableQuery.test.tsx +++ b/src/react/hooks/__tests__/useLoadableQuery.test.tsx @@ -3042,6 +3042,9 @@ it("properly uses `updateQuery` when calling `fetchMore`", async () => { }); } + // TODO investigate: this test highlights a React render + // that actually doesn't rerender any user-provided components + // so we need to use `skipNonTrackingRenders` await expect(Profiler).not.toRerender(); }); @@ -3126,6 +3129,9 @@ it("properly uses cache field policies when calling `fetchMore` without `updateQ }); } + // TODO investigate: this test highlights a React render + // that actually doesn't rerender any user-provided components + // so we need to use `skipNonTrackingRenders` await expect(Profiler).not.toRerender(); }); From 4bde5a0df61cf87d0d3a6da5ed35c754ac5042fd Mon Sep 17 00:00:00 2001 From: phryneas Date: Fri, 1 Dec 2023 18:01:23 +0000 Subject: [PATCH 23/23] Clean up Prettier, Size-limit, and Api-Extractor --- .api-reports/api-report-react.md | 4 ++-- .api-reports/api-report-react_hooks.md | 4 ++-- .api-reports/api-report.md | 4 ++-- .size-limits.json | 4 ++-- src/react/cache/QueryReference.ts | 6 +++--- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/.api-reports/api-report-react.md b/.api-reports/api-report-react.md index 1083b6b48ff..99bf23e5739 100644 --- a/.api-reports/api-report-react.md +++ b/.api-reports/api-report-react.md @@ -2257,8 +2257,8 @@ interface WatchQueryOptions( // There is a chance the query ref's promise has been updated in the time // the original promise had been suspended. In that case, we want to use // it instead of the older promise which may contain outdated data. - internalQueryRef.promise.status === "fulfilled" - ? internalQueryRef.promise - : queryRef[PROMISE_SYMBOL], + internalQueryRef.promise.status === "fulfilled" ? + internalQueryRef.promise + : queryRef[PROMISE_SYMBOL], ]; }