From bd3fc84b59329c67766830385e83578a26158694 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Wed, 9 Jun 2021 18:12:20 -0400 Subject: [PATCH 01/13] Implement `useFragment` hook. --- src/__tests__/__snapshots__/exports.ts.snap | 3 + src/cache/core/types/Cache.ts | 4 +- src/cache/inmemory/inMemoryCache.ts | 4 +- src/react/hooks/index.ts | 1 + src/react/hooks/useFragment.ts | 77 +++++++++++++++++++++ 5 files changed, 85 insertions(+), 4 deletions(-) create mode 100644 src/react/hooks/useFragment.ts diff --git a/src/__tests__/__snapshots__/exports.ts.snap b/src/__tests__/__snapshots__/exports.ts.snap index aa50e65fc3a..eaf287ba424 100644 --- a/src/__tests__/__snapshots__/exports.ts.snap +++ b/src/__tests__/__snapshots__/exports.ts.snap @@ -53,6 +53,7 @@ Array [ "throwServerError", "toPromise", "useApolloClient", + "useFragment", "useLazyQuery", "useMutation", "useQuery", @@ -242,6 +243,7 @@ Array [ "parser", "resetApolloContext", "useApolloClient", + "useFragment", "useLazyQuery", "useMutation", "useQuery", @@ -280,6 +282,7 @@ Array [ exports[`exports of public entry points @apollo/client/react/hooks 1`] = ` Array [ "useApolloClient", + "useFragment", "useLazyQuery", "useMutation", "useQuery", diff --git a/src/cache/core/types/Cache.ts b/src/cache/core/types/Cache.ts index 6f830f9abd9..4086d34a45e 100644 --- a/src/cache/core/types/Cache.ts +++ b/src/cache/core/types/Cache.ts @@ -28,7 +28,7 @@ export namespace Cache { export interface DiffOptions< TData = any, TVariables = any, - > extends ReadOptions { + > extends Omit, "rootId"> { // The DiffOptions interface is currently just an alias for // ReadOptions, though DiffOptions used to be responsible for // declaring the returnPartialData option. @@ -37,7 +37,7 @@ export namespace Cache { export interface WatchOptions< TData = any, TVariables = any, - > extends ReadOptions { + > extends DiffOptions { watcher?: object; immediate?: boolean; callback: WatchCallback; diff --git a/src/cache/inmemory/inMemoryCache.ts b/src/cache/inmemory/inMemoryCache.ts index 6ce2a71d677..69a497f332a 100644 --- a/src/cache/inmemory/inMemoryCache.ts +++ b/src/cache/inmemory/inMemoryCache.ts @@ -120,7 +120,7 @@ export class InMemoryCache extends ApolloCache { // currently using a data store that can track cache dependencies. const store = c.optimistic ? this.optimisticData : this.data; if (supportsResultCaching(store)) { - const { optimistic, rootId, variables } = c; + const { optimistic, id, variables } = c; return store.makeCacheKey( c.query, // Different watches can have the same query, optimistic @@ -130,7 +130,7 @@ export class InMemoryCache extends ApolloCache { // separation is to include c.callback in the cache key for // maybeBroadcastWatch calls. See issue #5733. c.callback, - canonicalStringify({ optimistic, rootId, variables }), + canonicalStringify({ optimistic, id, variables }), ); } } diff --git a/src/react/hooks/index.ts b/src/react/hooks/index.ts index 5201dc6e645..b7e45dfeda8 100644 --- a/src/react/hooks/index.ts +++ b/src/react/hooks/index.ts @@ -6,3 +6,4 @@ export * from './useMutation'; export { useQuery } from './useQuery'; export * from './useSubscription'; export * from './useReactiveVar'; +export * from './useFragment'; diff --git a/src/react/hooks/useFragment.ts b/src/react/hooks/useFragment.ts new file mode 100644 index 00000000000..66287c496be --- /dev/null +++ b/src/react/hooks/useFragment.ts @@ -0,0 +1,77 @@ +import { useEffect, useState } from "react"; +import { equal } from "@wry/equality"; + +import { Cache, MissingFieldError, Reference, StoreObject } from "../../cache"; +import { useApolloClient } from "./useApolloClient"; + +export interface UseFragmentOptions +extends Omit< + Cache.DiffOptions, + | "id" + | "query" + | "optimistic" +>, Omit< + Cache.ReadFragmentOptions, + | "id" +> { + from: StoreObject | Reference | string; + // Override this field to make it optional (default: true). + optimistic?: boolean; +} + +export interface UseFragmentResult { + data: TData | undefined; + complete: boolean; + missing?: MissingFieldError[]; + previousResult?: UseFragmentResult; + lastCompleteResult?: UseFragmentResult; +} + +export function useFragment( + options: UseFragmentOptions, +): UseFragmentResult { + const { cache } = useApolloClient(); + + const { + fragment, + fragmentName, + from, + optimistic = true, + ...rest + } = options; + + const diffOptions: Cache.DiffOptions = { + ...rest, + id: typeof from === "string" ? from : cache.identify(from), + query: cache["getFragmentDoc"](fragment, fragmentName), + optimistic, + }; + + const preDiff = cache.diff(diffOptions); + const setDiff = useState(preDiff)[1]; + + useEffect(() => { + let immediate = true; + return cache.watch({ + ...diffOptions, + immediate, + callback(newDiff) { + if (!immediate || !equal(newDiff, preDiff)) { + setDiff(newDiff); + } + immediate = false; + }, + }); + }, [preDiff]); + + const result: UseFragmentResult = { + data: preDiff.result, + complete: !!preDiff.complete, + }; + + if (preDiff.missing) { + result.missing = preDiff.missing!; + } + + return result; +} From 6dd58298f4f0a7058369c8c4b8bb641b89a2a251 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Wed, 9 Jun 2021 18:32:22 -0400 Subject: [PATCH 02/13] Basic tests of `useFragment` hook. --- .../hooks/__tests__/useFragment.test.tsx | 540 ++++++++++++++++++ 1 file changed, 540 insertions(+) create mode 100644 src/react/hooks/__tests__/useFragment.test.tsx diff --git a/src/react/hooks/__tests__/useFragment.test.tsx b/src/react/hooks/__tests__/useFragment.test.tsx new file mode 100644 index 00000000000..681a32ab43a --- /dev/null +++ b/src/react/hooks/__tests__/useFragment.test.tsx @@ -0,0 +1,540 @@ +import * as React from "react"; +import { render, waitFor } from "@testing-library/react"; +import { act } from "react-dom/test-utils"; + +import { useFragment } from "../useFragment"; +import { MockedProvider } from "../../../testing"; +import { InMemoryCache, gql, TypedDocumentNode, Reference } from "../../../core"; +import { useQuery } from "../useQuery"; + +describe("useFragment", () => { + it("is importable and callable", () => { + expect(typeof useFragment).toBe("function"); + }); + + type Item = { + __typename: string; + id: number; + text?: string; + }; + + const ListFragment: TypedDocumentNode = gql` + fragment ListFragment on Query { + list { + id + } + } + `; + + const ItemFragment: TypedDocumentNode = gql` + fragment ItemFragment on Item { + text + } + `; + + type QueryData = { + list: Item[]; + }; + + it("can rerender individual list elements", async () => { + const cache = new InMemoryCache({ + typePolicies: { + Item: { + fields: { + text(existing, { readField }) { + return existing || `Item #${readField("id")}`; + }, + }, + }, + }, + }); + + const listQuery: TypedDocumentNode = gql` + query { + list { + id + } + } + `; + + cache.writeQuery({ + query: listQuery, + data: { + list: [ + { __typename: "Item", id: 1 }, + { __typename: "Item", id: 2 }, + { __typename: "Item", id: 5 }, + ], + }, + }) + + const renders: string[] = []; + + function List() { + renders.push("list"); + const { loading, data } = useQuery(listQuery); + expect(loading).toBe(false); + return ( +
    + {data!.list.map(item => )} +
+ ); + } + + function Item(props: { id: number }) { + renders.push("item " + props.id); + const { complete, data } = useFragment({ + fragment: ItemFragment, + fragmentName: "ItemFragment", + from: { + __typename: "Item", + id: props.id, + }, + }); + return
  • {complete ? data!.text : "incomplete"}
    • ; + } + + const { getAllByText } = render( + + + + ); + + function getItemTexts() { + return getAllByText(/^Item/).map( + li => li.firstChild!.textContent + ); + } + + await waitFor(() => { + expect(getItemTexts()).toEqual([ + "Item #1", + "Item #2", + "Item #5", + ]); + }); + + expect(renders).toEqual([ + "list", + "item 1", + "item 2", + "item 5", + ]); + + act(() => { + cache.writeFragment({ + fragment: ItemFragment, + data: { + __typename: "Item", + id: 2, + text: "Item #2 updated", + }, + }); + }); + + await waitFor(() => { + expect(getItemTexts()).toEqual([ + "Item #1", + "Item #2 updated", + "Item #5", + ]); + }); + + expect(renders).toEqual([ + "list", + "item 1", + "item 2", + "item 5", + // Only the second item should have re-rendered. + "item 2", + ]); + + act(() => { + cache.modify({ + fields: { + list(list: Reference[], { readField }) { + return [ + ...list, + cache.writeFragment({ + fragment: ItemFragment, + data: { + __typename: "Item", + id: 3, + text: "Item #3 from cache.modify", + }, + })!, + cache.writeFragment({ + fragment: ItemFragment, + data: { + __typename: "Item", + id: 4, + text: "Item #4 from cache.modify", + }, + })!, + ].sort((ref1, ref2) => ( + readField("id", ref1)! - + readField("id", ref2)! + )); + }, + }, + }); + }); + + await waitFor(() => { + expect(getItemTexts()).toEqual([ + "Item #1", + "Item #2 updated", + "Item #3 from cache.modify", + "Item #4 from cache.modify", + "Item #5", + ]); + }); + + expect(renders).toEqual([ + "list", + "item 1", + "item 2", + "item 5", + "item 2", + // This is what's new: + "list", + "item 1", + "item 2", + "item 3", + "item 4", + "item 5", + ]); + + act(() => { + cache.writeFragment({ + fragment: ItemFragment, + data: { + __typename: "Item", + id: 4, + text: "Item #4 updated", + }, + }); + }); + + await waitFor(() => { + expect(getItemTexts()).toEqual([ + "Item #1", + "Item #2 updated", + "Item #3 from cache.modify", + "Item #4 updated", + "Item #5", + ]); + }); + + expect(renders).toEqual([ + "list", + "item 1", + "item 2", + "item 5", + "item 2", + "list", + "item 1", + "item 2", + "item 3", + "item 4", + "item 5", + // Only the fourth item should have re-rendered. + "item 4", + ]); + + expect(cache.extract()).toEqual({ + "Item:1": { + __typename: "Item", + id: 1, + }, + "Item:2": { + __typename: "Item", + id: 2, + text: "Item #2 updated", + }, + "Item:3": { + __typename: "Item", + id: 3, + text: "Item #3 from cache.modify", + }, + "Item:4": { + __typename: "Item", + id: 4, + text: "Item #4 updated", + }, + "Item:5": { + __typename: "Item", + id: 5, + }, + ROOT_QUERY: { + __typename: "Query", + list: [ + { __ref: "Item:1" }, + { __ref: "Item:2" }, + { __ref: "Item:3" }, + { __ref: "Item:4" }, + { __ref: "Item:5" }, + ], + }, + __META: { + extraRootIds: [ + "Item:2", + "Item:3", + "Item:4", + ], + }, + }); + }); + + it("List can use useFragment with ListFragment", async () => { + const cache = new InMemoryCache({ + typePolicies: { + Item: { + fields: { + text(existing, { readField }) { + return existing || `Item #${readField("id")}`; + }, + }, + }, + }, + }); + + const listQuery: TypedDocumentNode = gql` + query { + list { + ...ListFragment + ...ItemFragment + } + } + ${ListFragment} + ${ItemFragment} + `; + + cache.writeQuery({ + query: listQuery, + data: { + list: [ + { __typename: "Item", id: 1 }, + { __typename: "Item", id: 2 }, + { __typename: "Item", id: 5 }, + ], + }, + }) + + const renders: string[] = []; + + function List() { + renders.push("list"); + const { complete, data } = useFragment({ + fragment: ListFragment, + from: { __typename: "Query" }, + }); + expect(complete).toBe(true); + return ( +
      + {data!.list.map(item => )} +
    + ); + } + + function Item(props: { id: number }) { + renders.push("item " + props.id); + const { complete, data } = useFragment({ + fragment: ItemFragment, + from: { + __typename: "Item", + id: props.id, + }, + }); + return
  • {complete ? data!.text : "incomplete"}
    • ; + } + + const { getAllByText } = render( + + + + ); + + function getItemTexts() { + return getAllByText(/^Item/).map( + li => li.firstChild!.textContent + ); + } + + await waitFor(() => { + expect(getItemTexts()).toEqual([ + "Item #1", + "Item #2", + "Item #5", + ]); + }); + + expect(renders).toEqual([ + "list", + "item 1", + "item 2", + "item 5", + ]); + + act(() => { + cache.writeFragment({ + fragment: ItemFragment, + data: { + __typename: "Item", + id: 2, + text: "Item #2 updated", + }, + }); + }); + + await waitFor(() => { + expect(getItemTexts()).toEqual([ + "Item #1", + "Item #2 updated", + "Item #5", + ]); + }); + + expect(renders).toEqual([ + "list", + "item 1", + "item 2", + "item 5", + // Only the second item should have re-rendered. + "item 2", + ]); + + act(() => { + cache.modify({ + fields: { + list(list: Reference[], { readField }) { + return [ + ...list, + cache.writeFragment({ + fragment: ItemFragment, + data: { + __typename: "Item", + id: 3, + }, + })!, + cache.writeFragment({ + fragment: ItemFragment, + data: { + __typename: "Item", + id: 4, + }, + })!, + ].sort((ref1, ref2) => ( + readField("id", ref1)! - + readField("id", ref2)! + )); + }, + }, + }); + }); + + await waitFor(() => { + expect(getItemTexts()).toEqual([ + "Item #1", + "Item #2 updated", + "Item #3", + "Item #4", + "Item #5", + ]); + }); + + expect(renders).toEqual([ + "list", + "item 1", + "item 2", + "item 5", + "item 2", + // This is what's new: + "list", + "item 1", + "item 2", + "item 3", + "item 4", + "item 5", + ]); + + act(() => { + cache.writeFragment({ + fragment: ItemFragment, + data: { + __typename: "Item", + id: 4, + text: "Item #4 updated", + }, + }); + }); + + await waitFor(() => { + expect(getItemTexts()).toEqual([ + "Item #1", + "Item #2 updated", + "Item #3", + "Item #4 updated", + "Item #5", + ]); + }); + + expect(renders).toEqual([ + "list", + "item 1", + "item 2", + "item 5", + "item 2", + "list", + "item 1", + "item 2", + "item 3", + "item 4", + "item 5", + // Only the fourth item should have re-rendered. + "item 4", + ]); + + expect(cache.extract()).toEqual({ + "Item:1": { + __typename: "Item", + id: 1, + }, + "Item:2": { + __typename: "Item", + id: 2, + text: "Item #2 updated", + }, + "Item:3": { + __typename: "Item", + id: 3, + }, + "Item:4": { + __typename: "Item", + id: 4, + text: "Item #4 updated", + }, + "Item:5": { + __typename: "Item", + id: 5, + }, + ROOT_QUERY: { + __typename: "Query", + list: [ + { __ref: "Item:1" }, + { __ref: "Item:2" }, + { __ref: "Item:3" }, + { __ref: "Item:4" }, + { __ref: "Item:5" }, + ], + }, + __META: { + extraRootIds: [ + "Item:2", + "Item:3", + "Item:4", + ], + }, + }); + }); +}); From c60f852517ac53ac9ce8dc308471e708e0aff3c7 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Wed, 8 Dec 2021 11:49:00 -0500 Subject: [PATCH 03/13] Make useFragment(...).missing a MissingTree rather than an error array. --- src/cache/core/types/common.ts | 13 ++- src/cache/index.ts | 1 + .../hooks/__tests__/useFragment.test.tsx | 95 +++++++++++++++++++ src/react/hooks/useFragment.ts | 15 ++- 4 files changed, 120 insertions(+), 4 deletions(-) diff --git a/src/cache/core/types/common.ts b/src/cache/core/types/common.ts index 24b8ef4bb5f..142f1844d17 100644 --- a/src/cache/core/types/common.ts +++ b/src/cache/core/types/common.ts @@ -28,7 +28,18 @@ export class MissingFieldError { public readonly path: MissingTree | Array, public readonly query: DocumentNode, public readonly variables?: Record, - ) {} + ) { + if (Array.isArray(this.path)) { + this.missing = this.message; + for (let i = this.path.length - 1; i >= 0; --i) { + this.missing = { [this.path[i]]: this.missing }; + } + } else { + this.missing = this.path; + } + } + + public readonly missing: MissingTree; } export interface FieldSpecifier { diff --git a/src/cache/index.ts b/src/cache/index.ts index 5778a1d2c69..b99823f7212 100644 --- a/src/cache/index.ts +++ b/src/cache/index.ts @@ -4,6 +4,7 @@ export { Transaction, ApolloCache } from './core/cache'; export { Cache } from './core/types/Cache'; export { DataProxy } from './core/types/DataProxy'; export { + MissingTree, MissingFieldError, ReadFieldOptions } from './core/types/common'; diff --git a/src/react/hooks/__tests__/useFragment.test.tsx b/src/react/hooks/__tests__/useFragment.test.tsx index 681a32ab43a..09cf1c1ac31 100644 --- a/src/react/hooks/__tests__/useFragment.test.tsx +++ b/src/react/hooks/__tests__/useFragment.test.tsx @@ -1,5 +1,6 @@ import * as React from "react"; import { render, waitFor } from "@testing-library/react"; +import { renderHook } from '@testing-library/react-hooks'; import { act } from "react-dom/test-utils"; import { useFragment } from "../useFragment"; @@ -537,4 +538,98 @@ describe("useFragment", () => { }, }); }); + + it("useFragment(...).missing is a tree describing missing fields", async () => { + const cache = new InMemoryCache; + const wrapper = ({ children }: any) => ( + {children} + ); + + const ListAndItemFragments: TypedDocumentNode = gql` + fragment ListFragment on Query { + list { + id + ...ItemFragment + } + } + ${ItemFragment} + `; + + const ListQuery: TypedDocumentNode = gql` + query ListQuery { + list { + id + } + } + `; + + const ListQueryWithText: TypedDocumentNode = gql` + query ListQuery { + list { + id + text + } + } + `; + + const { result } = renderHook( + () => useFragment({ + fragment: ListAndItemFragments, + fragmentName: "ListFragment", + from: { __typename: "Query" }, + returnPartialData: true, + }), + { wrapper }, + ); + + expect(result.current.complete).toBe(false); + expect(result.current.data).toEqual({}); // TODO Should be undefined? + expect(result.current.missing).toEqual({ + list: "Can't find field 'list' on ROOT_QUERY object", + }); + + const data125 = { + list: [ + { __typename: "Item", id: 1 }, + { __typename: "Item", id: 2 }, + { __typename: "Item", id: 5 }, + ], + }; + + await act(async () => { + cache.writeQuery({ + query: ListQuery, + data: data125, + }); + }); + + expect(result.current.complete).toBe(false); + expect(result.current.data).toEqual(data125); + expect(result.current.missing).toEqual({ + list: { + 0: { text: "Can't find field 'text' on Item:1 object" }, + 1: { text: "Can't find field 'text' on Item:2 object" }, + 2: { text: "Can't find field 'text' on Item:5 object" }, + }, + }); + + const data182WithText = { + list: [ + { __typename: "Item", id: 1, text: "oyez1" }, + { __typename: "Item", id: 8, text: "oyez8" }, + { __typename: "Item", id: 2, text: "oyez2" }, + ], + }; + + await act(async () => { + cache.writeQuery({ + query: ListQueryWithText, + data: data182WithText, + }); + }); + + expect(result.current.complete).toBe(true); + expect(result.current.data).toEqual(data182WithText); + expect(result.current.missing).toBeUndefined(); + }); }); diff --git a/src/react/hooks/useFragment.ts b/src/react/hooks/useFragment.ts index 66287c496be..be4dafa243d 100644 --- a/src/react/hooks/useFragment.ts +++ b/src/react/hooks/useFragment.ts @@ -1,7 +1,14 @@ import { useEffect, useState } from "react"; import { equal } from "@wry/equality"; -import { Cache, MissingFieldError, Reference, StoreObject } from "../../cache"; +import { mergeDeepArray } from "../../utilities"; +import { + Cache, + Reference, + StoreObject, + MissingTree, +} from "../../cache"; + import { useApolloClient } from "./useApolloClient"; export interface UseFragmentOptions @@ -22,7 +29,7 @@ extends Omit< export interface UseFragmentResult { data: TData | undefined; complete: boolean; - missing?: MissingFieldError[]; + missing?: MissingTree; previousResult?: UseFragmentResult; lastCompleteResult?: UseFragmentResult; } @@ -70,7 +77,9 @@ export function useFragment( }; if (preDiff.missing) { - result.missing = preDiff.missing!; + result.missing = mergeDeepArray( + preDiff.missing.map(error => error.missing) + ); } return result; From 57588f4129633bde18470da6d46d5131981d1c93 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Wed, 8 Dec 2021 13:45:48 -0500 Subject: [PATCH 04/13] Comment about using objects to represent arrays in MissingTree. --- src/react/hooks/__tests__/useFragment.test.tsx | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/react/hooks/__tests__/useFragment.test.tsx b/src/react/hooks/__tests__/useFragment.test.tsx index 09cf1c1ac31..9fcccc0f12a 100644 --- a/src/react/hooks/__tests__/useFragment.test.tsx +++ b/src/react/hooks/__tests__/useFragment.test.tsx @@ -607,6 +607,14 @@ describe("useFragment", () => { expect(result.current.data).toEqual(data125); expect(result.current.missing).toEqual({ list: { + // Even though Query.list is actually an array in the data, data paths + // through this array leading to missing fields potentially involve only + // a small/sparse subset of the array's indexes, so we use objects for + // the entire MissingTree, to avoid having to worry about sparse arrays. + // This also means there's no missing.list.length property, which is + // good because "length" could be a name of an actual field that's + // missing, and it's somewhat unclear what the length of a sparse array + // should be, whereas object keys have a less ambiguous interpretation. 0: { text: "Can't find field 'text' on Item:1 object" }, 1: { text: "Can't find field 'text' on Item:2 object" }, 2: { text: "Can't find field 'text' on Item:5 object" }, From 58aa68dbe90e7674a89d19b8d3cc969c8ae742a7 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Wed, 8 Dec 2021 15:05:07 -0500 Subject: [PATCH 05/13] Provide historical useFragment(...).{previous,lastComplete}Result. --- .../hooks/__tests__/useFragment.test.tsx | 33 +++++++++++++- src/react/hooks/useFragment.ts | 45 +++++++++++++++---- 2 files changed, 68 insertions(+), 10 deletions(-) diff --git a/src/react/hooks/__tests__/useFragment.test.tsx b/src/react/hooks/__tests__/useFragment.test.tsx index 9fcccc0f12a..44f0e63923d 100644 --- a/src/react/hooks/__tests__/useFragment.test.tsx +++ b/src/react/hooks/__tests__/useFragment.test.tsx @@ -3,7 +3,7 @@ import { render, waitFor } from "@testing-library/react"; import { renderHook } from '@testing-library/react-hooks'; import { act } from "react-dom/test-utils"; -import { useFragment } from "../useFragment"; +import { useFragment, UseFragmentResult } from "../useFragment"; import { MockedProvider } from "../../../testing"; import { InMemoryCache, gql, TypedDocumentNode, Reference } from "../../../core"; import { useQuery } from "../useQuery"; @@ -582,12 +582,39 @@ describe("useFragment", () => { { wrapper }, ); + function checkHistory(expectedResultCount: number) { + function historyToArray( + result: UseFragmentResult, + ): UseFragmentResult[] { + const array = result.previousResult + ? historyToArray(result.previousResult) + : []; + array.push(result); + return array; + } + const all = historyToArray(result.current); + expect(all.length).toBe(expectedResultCount); + expect(all).toEqual(result.all); + + if (result.current.complete) { + expect(result.current).toBe( + result.current.lastCompleteResult + ); + } else { + expect(result.current).not.toBe( + result.current.lastCompleteResult + ); + } + } + expect(result.current.complete).toBe(false); expect(result.current.data).toEqual({}); // TODO Should be undefined? expect(result.current.missing).toEqual({ list: "Can't find field 'list' on ROOT_QUERY object", }); + checkHistory(1); + const data125 = { list: [ { __typename: "Item", id: 1 }, @@ -621,6 +648,8 @@ describe("useFragment", () => { }, }); + checkHistory(2); + const data182WithText = { list: [ { __typename: "Item", id: 1, text: "oyez1" }, @@ -639,5 +668,7 @@ describe("useFragment", () => { expect(result.current.complete).toBe(true); expect(result.current.data).toEqual(data182WithText); expect(result.current.missing).toBeUndefined(); + + checkHistory(3); }); }); diff --git a/src/react/hooks/useFragment.ts b/src/react/hooks/useFragment.ts index be4dafa243d..494a476d32a 100644 --- a/src/react/hooks/useFragment.ts +++ b/src/react/hooks/useFragment.ts @@ -54,8 +54,9 @@ export function useFragment( optimistic, }; - const preDiff = cache.diff(diffOptions); - const setDiff = useState(preDiff)[1]; + let latestDiff = cache.diff(diffOptions); + let [latestResult, setResult] = + useState>(() => diffToResult(latestDiff)); useEffect(() => { let immediate = true; @@ -63,24 +64,50 @@ export function useFragment( ...diffOptions, immediate, callback(newDiff) { - if (!immediate || !equal(newDiff, preDiff)) { - setDiff(newDiff); + if (!immediate || !equal(newDiff, latestDiff)) { + setResult(latestResult = diffToResult( + latestDiff = newDiff, + latestResult, + )); } immediate = false; }, }); - }, [preDiff]); + }, [latestDiff]); + return latestResult; +} + +function diffToResult( + diff: Cache.DiffResult, + previousResult?: UseFragmentResult, +): UseFragmentResult { const result: UseFragmentResult = { - data: preDiff.result, - complete: !!preDiff.complete, + data: diff.result, + complete: !!diff.complete, }; - if (preDiff.missing) { + if (diff.missing) { result.missing = mergeDeepArray( - preDiff.missing.map(error => error.missing) + diff.missing.map(error => error.missing) ); } + if (previousResult) { + result.previousResult = previousResult; + } + + const lastCompleteResult = result.complete ? result : ( + previousResult && ( + previousResult.complete + ? previousResult + : previousResult.lastCompleteResult + ) + ); + + if (lastCompleteResult) { + result.lastCompleteResult = lastCompleteResult; + } + return result; } From a0e32ce2286b2e2d4e3ee71852dde969869b7f03 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Wed, 8 Dec 2021 16:01:41 -0500 Subject: [PATCH 06/13] Rename `result` local variable to `renderResult` for clarity. --- .../hooks/__tests__/useFragment.test.tsx | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/src/react/hooks/__tests__/useFragment.test.tsx b/src/react/hooks/__tests__/useFragment.test.tsx index 44f0e63923d..4d1c7aa9ca7 100644 --- a/src/react/hooks/__tests__/useFragment.test.tsx +++ b/src/react/hooks/__tests__/useFragment.test.tsx @@ -572,7 +572,7 @@ describe("useFragment", () => { } `; - const { result } = renderHook( + const { result: renderResult } = renderHook( () => useFragment({ fragment: ListAndItemFragments, fragmentName: "ListFragment", @@ -592,24 +592,24 @@ describe("useFragment", () => { array.push(result); return array; } - const all = historyToArray(result.current); + const all = historyToArray(renderResult.current); expect(all.length).toBe(expectedResultCount); - expect(all).toEqual(result.all); + expect(all).toEqual(renderResult.all); - if (result.current.complete) { - expect(result.current).toBe( - result.current.lastCompleteResult + if (renderResult.current.complete) { + expect(renderResult.current).toBe( + renderResult.current.lastCompleteResult ); } else { - expect(result.current).not.toBe( - result.current.lastCompleteResult + expect(renderResult.current).not.toBe( + renderResult.current.lastCompleteResult ); } } - expect(result.current.complete).toBe(false); - expect(result.current.data).toEqual({}); // TODO Should be undefined? - expect(result.current.missing).toEqual({ + expect(renderResult.current.complete).toBe(false); + expect(renderResult.current.data).toEqual({}); // TODO Should be undefined? + expect(renderResult.current.missing).toEqual({ list: "Can't find field 'list' on ROOT_QUERY object", }); @@ -630,9 +630,9 @@ describe("useFragment", () => { }); }); - expect(result.current.complete).toBe(false); - expect(result.current.data).toEqual(data125); - expect(result.current.missing).toEqual({ + expect(renderResult.current.complete).toBe(false); + expect(renderResult.current.data).toEqual(data125); + expect(renderResult.current.missing).toEqual({ list: { // Even though Query.list is actually an array in the data, data paths // through this array leading to missing fields potentially involve only @@ -665,9 +665,9 @@ describe("useFragment", () => { }); }); - expect(result.current.complete).toBe(true); - expect(result.current.data).toEqual(data182WithText); - expect(result.current.missing).toBeUndefined(); + expect(renderResult.current.complete).toBe(true); + expect(renderResult.current.data).toEqual(data182WithText); + expect(renderResult.current.missing).toBeUndefined(); checkHistory(3); }); From 79a4c914d8ef569274c30912af2e087a5087d3db Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Thu, 9 Dec 2021 10:35:03 -0500 Subject: [PATCH 07/13] Test useFragment missing tree and result history after eviction. --- .../hooks/__tests__/useFragment.test.tsx | 83 ++++++++++++++++++- 1 file changed, 82 insertions(+), 1 deletion(-) diff --git a/src/react/hooks/__tests__/useFragment.test.tsx b/src/react/hooks/__tests__/useFragment.test.tsx index 4d1c7aa9ca7..c78f5860a6f 100644 --- a/src/react/hooks/__tests__/useFragment.test.tsx +++ b/src/react/hooks/__tests__/useFragment.test.tsx @@ -540,7 +540,23 @@ describe("useFragment", () => { }); it("useFragment(...).missing is a tree describing missing fields", async () => { - const cache = new InMemoryCache; + const cache = new InMemoryCache({ + typePolicies: { + Query: { + fields: { + list(items: Reference[] | undefined, { canRead }) { + // This filtering happens by default currently in the StoreReader + // execSubSelectedArrayImpl method, but I am beginning to question + // the wisdom of that automatic filtering. In case we end up + // changing the default behavior in the future, I've encoded the + // filtering explicitly here, so this test won't be broken. + return items && items.filter(canRead); + }, + } + } + } + }); + const wrapper = ({ children }: any) => ( {children} ); @@ -670,5 +686,70 @@ describe("useFragment", () => { expect(renderResult.current.missing).toBeUndefined(); checkHistory(3); + + await act(async () => cache.batch({ + update(cache) { + cache.evict({ + id: cache.identify({ + __typename: "Item", + id: 8, + }), + }); + + cache.evict({ + id: cache.identify({ + __typename: "Item", + id: 2, + }), + fieldName: "text", + }); + }, + })); + + expect(renderResult.current.complete).toBe(false); + expect(renderResult.current.data).toEqual({ + list: [ + { __typename: "Item", id: 1, text: "oyez1" }, + { __typename: "Item", id: 2 }, + ], + }); + expect(renderResult.current.missing).toEqual({ + // TODO Figure out why Item:8 is not represented here. Likely because of + // auto-filtering of dangling references from arrays, but that should + // still be reflected here, if possible. + list: { + 1: { + text: "Can't find field 'text' on Item:2 object", + }, + }, + }); + + checkHistory(4); + + expect(cache.extract()).toEqual({ + "Item:1": { + __typename: "Item", + id: 1, + text: "oyez1", + }, + "Item:2": { + __typename: "Item", + id: 2, + }, + "Item:5": { + __typename: "Item", + id: 5, + }, + ROOT_QUERY: { + __typename: "Query", + list: [ + { __ref: "Item:1" }, + { __ref: "Item:8" }, + { __ref: "Item:2" }, + ], + }, + }); + + expect(cache.gc().sort()).toEqual(["Item:5"]); }); }); From 720005521d277ebef02eaf631e8d0c72bb3cca01 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Thu, 3 Feb 2022 11:34:40 -0500 Subject: [PATCH 08/13] Fix misplaced ...ListFragment spread. https://github.com/apollographql/apollo-client/pull/8782#discussion_r797984651 Previously, the ListFragment was effectively ignored because its type condition (on Query) did not match the "Item" __typename. Although the code was definitely wrong before, this change ends up not making a visible difference because the Item.id field provided by ListFragment is the default field used for normalization, so it gets picked up and used regardless of whether it's mentioned in the query (even when ListFragment is completely ignored, as it was before). To make this change matter in a visible way, I added an extra root query field to ListFragment to strengthen the tests. --- .../hooks/__tests__/useFragment.test.tsx | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/src/react/hooks/__tests__/useFragment.test.tsx b/src/react/hooks/__tests__/useFragment.test.tsx index c78f5860a6f..f962dd48cd8 100644 --- a/src/react/hooks/__tests__/useFragment.test.tsx +++ b/src/react/hooks/__tests__/useFragment.test.tsx @@ -19,11 +19,14 @@ describe("useFragment", () => { text?: string; }; - const ListFragment: TypedDocumentNode = gql` + const ListFragment: TypedDocumentNode = gql` fragment ListFragment on Query { list { id } + # Used to make sure ListFragment got used, even if the id field of the + # nested list items is provided by other means. + extra } `; @@ -33,9 +36,13 @@ describe("useFragment", () => { } `; - type QueryData = { + interface QueryData { list: Item[]; - }; + } + + interface QueryDataWithExtra extends QueryData { + extra: string; + } it("can rerender individual list elements", async () => { const cache = new InMemoryCache({ @@ -300,10 +307,10 @@ describe("useFragment", () => { }, }); - const listQuery: TypedDocumentNode = gql` + const listQuery: TypedDocumentNode = gql` query { + ...ListFragment list { - ...ListFragment ...ItemFragment } } @@ -319,6 +326,7 @@ describe("useFragment", () => { { __typename: "Item", id: 2 }, { __typename: "Item", id: 5 }, ], + extra: "from ListFragment", }, }) @@ -528,6 +536,7 @@ describe("useFragment", () => { { __ref: "Item:4" }, { __ref: "Item:5" }, ], + extra: "from ListFragment", }, __META: { extraRootIds: [ From 811898471d9f2ac1fe6caf8742f289cc94a986b0 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Thu, 3 Feb 2022 14:27:38 -0500 Subject: [PATCH 09/13] Comment about intended UseFragmentOptions fields and their types. --- src/react/hooks/useFragment.ts | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/react/hooks/useFragment.ts b/src/react/hooks/useFragment.ts index 494a476d32a..2249e0d6b9c 100644 --- a/src/react/hooks/useFragment.ts +++ b/src/react/hooks/useFragment.ts @@ -26,6 +26,21 @@ extends Omit< optimistic?: boolean; } +// Since the above definition of UseFragmentOptions can be hard to parse without +// help from TypeScript/VSCode, here are the intended fields and their types. +// Uncomment this code to check that it's consistent with the definition above. +// +// export interface UseFragmentOptions { +// from: string | StoreObject | Reference; +// fragment: DocumentNode | TypedDocumentNode; +// fragmentName?: string; +// optimistic?: boolean; +// variables?: TVars; +// previousResult?: any; +// returnPartialData?: boolean; +// canonizeResults?: boolean; +// } + export interface UseFragmentResult { data: TData | undefined; complete: boolean; From 892af74f54a9221285d4763e84ec5e692588d518 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Thu, 17 Feb 2022 14:05:19 -0500 Subject: [PATCH 10/13] Remove support for useFragment(...).{previous,lastComplete}Result. --- .../hooks/__tests__/useFragment.test.tsx | 49 ++++++++++--------- src/react/hooks/useFragment.ts | 18 ------- 2 files changed, 26 insertions(+), 41 deletions(-) diff --git a/src/react/hooks/__tests__/useFragment.test.tsx b/src/react/hooks/__tests__/useFragment.test.tsx index f962dd48cd8..ee3abc5da7a 100644 --- a/src/react/hooks/__tests__/useFragment.test.tsx +++ b/src/react/hooks/__tests__/useFragment.test.tsx @@ -3,7 +3,7 @@ import { render, waitFor } from "@testing-library/react"; import { renderHook } from '@testing-library/react-hooks'; import { act } from "react-dom/test-utils"; -import { useFragment, UseFragmentResult } from "../useFragment"; +import { useFragment } from "../useFragment"; import { MockedProvider } from "../../../testing"; import { InMemoryCache, gql, TypedDocumentNode, Reference } from "../../../core"; import { useQuery } from "../useQuery"; @@ -608,28 +608,31 @@ describe("useFragment", () => { ); function checkHistory(expectedResultCount: number) { - function historyToArray( - result: UseFragmentResult, - ): UseFragmentResult[] { - const array = result.previousResult - ? historyToArray(result.previousResult) - : []; - array.push(result); - return array; - } - const all = historyToArray(renderResult.current); - expect(all.length).toBe(expectedResultCount); - expect(all).toEqual(renderResult.all); - - if (renderResult.current.complete) { - expect(renderResult.current).toBe( - renderResult.current.lastCompleteResult - ); - } else { - expect(renderResult.current).not.toBe( - renderResult.current.lastCompleteResult - ); - } + // Temporarily disabling this check until we can come up with a better + // (more opt-in) system than result.previousResult.previousResult... + + // function historyToArray( + // result: UseFragmentResult, + // ): UseFragmentResult[] { + // const array = result.previousResult + // ? historyToArray(result.previousResult) + // : []; + // array.push(result); + // return array; + // } + // const all = historyToArray(renderResult.current); + // expect(all.length).toBe(expectedResultCount); + // expect(all).toEqual(renderResult.all); + + // if (renderResult.current.complete) { + // expect(renderResult.current).toBe( + // renderResult.current.lastCompleteResult + // ); + // } else { + // expect(renderResult.current).not.toBe( + // renderResult.current.lastCompleteResult + // ); + // } } expect(renderResult.current.complete).toBe(false); diff --git a/src/react/hooks/useFragment.ts b/src/react/hooks/useFragment.ts index 2249e0d6b9c..d719adcc568 100644 --- a/src/react/hooks/useFragment.ts +++ b/src/react/hooks/useFragment.ts @@ -82,7 +82,6 @@ export function useFragment( if (!immediate || !equal(newDiff, latestDiff)) { setResult(latestResult = diffToResult( latestDiff = newDiff, - latestResult, )); } immediate = false; @@ -95,7 +94,6 @@ export function useFragment( function diffToResult( diff: Cache.DiffResult, - previousResult?: UseFragmentResult, ): UseFragmentResult { const result: UseFragmentResult = { data: diff.result, @@ -108,21 +106,5 @@ function diffToResult( ); } - if (previousResult) { - result.previousResult = previousResult; - } - - const lastCompleteResult = result.complete ? result : ( - previousResult && ( - previousResult.complete - ? previousResult - : previousResult.lastCompleteResult - ) - ); - - if (lastCompleteResult) { - result.lastCompleteResult = lastCompleteResult; - } - return result; } From f45fdc43b387ce80548d13e60e928018136999c8 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Wed, 8 Dec 2021 14:51:26 -0500 Subject: [PATCH 11/13] Use useSyncExternalStore to reimplement/simplify useFragment. --- src/react/hooks/useFragment.ts | 44 ++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 20 deletions(-) diff --git a/src/react/hooks/useFragment.ts b/src/react/hooks/useFragment.ts index d719adcc568..80e1b4965c3 100644 --- a/src/react/hooks/useFragment.ts +++ b/src/react/hooks/useFragment.ts @@ -1,4 +1,4 @@ -import { useEffect, useState } from "react"; +import { useRef } from "react"; import { equal } from "@wry/equality"; import { mergeDeepArray } from "../../utilities"; @@ -10,6 +10,7 @@ import { } from "../../cache"; import { useApolloClient } from "./useApolloClient"; +import { useSyncExternalStore } from "./useSyncExternalStore"; export interface UseFragmentOptions extends Omit< @@ -69,27 +70,30 @@ export function useFragment( optimistic, }; + const resultRef = useRef>(); let latestDiff = cache.diff(diffOptions); - let [latestResult, setResult] = - useState>(() => diffToResult(latestDiff)); - useEffect(() => { - let immediate = true; - return cache.watch({ - ...diffOptions, - immediate, - callback(newDiff) { - if (!immediate || !equal(newDiff, latestDiff)) { - setResult(latestResult = diffToResult( - latestDiff = newDiff, - )); - } - immediate = false; - }, - }); - }, [latestDiff]); + return useSyncExternalStore( + forceUpdate => { + let immediate = true; + return cache.watch({ + ...diffOptions, + immediate, + callback(diff) { + if (!immediate && !equal(diff, latestDiff)) { + resultRef.current = diffToResult(latestDiff = diff); + forceUpdate(); + } + immediate = false; + }, + }); + }, - return latestResult; + () => { + return resultRef.current || + (resultRef.current = diffToResult(latestDiff)); + }, + ); } function diffToResult( @@ -102,7 +106,7 @@ function diffToResult( if (diff.missing) { result.missing = mergeDeepArray( - diff.missing.map(error => error.missing) + diff.missing.map(error => error.missing), ); } From 1e3705375adef6195dcead251ceb6d0f825425ce Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Wed, 25 May 2022 13:43:54 -0400 Subject: [PATCH 12/13] Bump bundlesize limit to 29.8kB (now 29.77kB). Although useFragment increases bundle sizes by this measurement, it also provides an alternative to useQuery and useLazyQuery that should prove smaller (if used alone, or with `useBackgroundQuery`) than using the existing hooks. --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index a9c514089d7..47780dab285 100644 --- a/package.json +++ b/package.json @@ -56,7 +56,7 @@ { "name": "apollo-client", "path": "./dist/apollo-client.min.cjs", - "maxSize": "29.5kB" + "maxSize": "29.8kB" } ], "engines": { From fa9549418962350a3e595862e9df029bb6cd6c0e Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Wed, 25 May 2022 13:49:28 -0400 Subject: [PATCH 13/13] Mention `useFragment` PR #8782 in `CHANGELOG.md` --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 10a8ad06134..8cb4393ddad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ ## Apollo Client 3.7.0 (in development) +### New Features + +- Implement `useFragment` hook, which represents a lightweight live binding into the `ApolloCache`, and never triggers network requests of its own.
    + [@benjamn](https://github.com/benjamn) in [#8782](https://github.com/apollographql/apollo-client/pull/8782) + - Replace `concast.cleanup` method with simpler `concast.beforeNext` API, which promises to call the given callback function just before the next result/error is delivered. In addition, `concast.removeObserver` no longer takes a `quietly?: boolean` parameter, since that parameter was partly responsible for cleanup callbacks sometimes not getting called.
    [@benjamn](https://github.com/benjamn) in [#9718](https://github.com/apollographql/apollo-client/pull/9718)