diff --git a/.changeset/tame-rocks-build.md b/.changeset/tame-rocks-build.md new file mode 100644 index 00000000000..37cd3c47621 --- /dev/null +++ b/.changeset/tame-rocks-build.md @@ -0,0 +1,5 @@ +--- +"@apollo/client": patch +--- + +Fix `watchFragment` reports `complete=false` when from object is not identifiable diff --git a/src/__tests__/ApolloClient.ts b/src/__tests__/ApolloClient.ts index fce6521d2af..2e4d9e0e5e2 100644 --- a/src/__tests__/ApolloClient.ts +++ b/src/__tests__/ApolloClient.ts @@ -14,7 +14,11 @@ import { Kind } from "graphql"; import { DeepPartial, Observable } from "../utilities"; import { ApolloLink, FetchResult } from "../link/core"; import { HttpLink } from "../link/http"; -import { createFragmentRegistry, InMemoryCache } from "../cache"; +import { + createFragmentRegistry, + InMemoryCache, + MissingFieldError, +} from "../cache"; import { ObservableStream, spyOnConsole } from "../testing/internal"; import { TypedDocumentNode } from "@graphql-typed-document-node/core"; import { invariant } from "../utilities/globals"; @@ -2647,6 +2651,148 @@ describe("ApolloClient", () => { }); } }); + + it("reports complete as false when `from` is not identifiable", async () => { + const cache = new InMemoryCache(); + const client = new ApolloClient({ + cache, + link: ApolloLink.empty(), + }); + const ItemFragment = gql` + fragment ItemFragment on Item { + id + text + } + `; + + const observable = client.watchFragment({ + fragment: ItemFragment, + from: {}, + }); + + const stream = new ObservableStream(observable); + + { + const result = await stream.takeNext(); + + expect(result).toStrictEqual({ + data: {}, + complete: false, + missing: + "Can't determine completeness for fragment query on non-identifiable object", + }); + } + }); + + it("reports diffs correctly when using getFragmentDoc", async () => { + const cache = new InMemoryCache(); + + const diffWithFragment = cache.diff({ + query: cache["getFragmentDoc"](gql` + fragment FooFragment on Foo { + foo + } + `), + id: cache.identify({}), + returnPartialData: true, + optimistic: true, + }); + + expect(diffWithFragment).toStrictEqual({ + result: {}, + complete: false, + missing: [ + new MissingFieldError( + "Can't determine completeness for fragment query on non-identifiable object", + expect.anything(), // query + expect.anything() // variables + ), + ], + }); + + await new Promise((res) => { + cache.watch({ + query: cache["getFragmentDoc"](gql` + fragment FooFragment on Foo { + foo + } + `), + id: cache.identify({}), + returnPartialData: true, + immediate: true, + optimistic: true, + callback: (diff) => { + expect(diff).toStrictEqual({ + result: {}, + complete: false, + missing: [ + new MissingFieldError( + "Can't determine completeness for fragment query on non-identifiable object", + expect.anything(), // query + expect.anything() // variables + ), + ], + }); + res(void 0); + }, + }); + }); + }); + + it("reports diffs correctly when not using getFragmentDoc", async () => { + const cache = new InMemoryCache(); + + const diffWithoutFragment = cache.diff({ + query: gql` + query { + foo + } + `, + id: cache.identify({}), + returnPartialData: true, + optimistic: true, + }); + + expect(diffWithoutFragment).toStrictEqual({ + result: {}, + complete: false, + missing: [ + new MissingFieldError( + "Can't find field 'foo' on ROOT_QUERY object", + expect.anything(), // query + expect.anything() // variables + ), + ], + }); + + await new Promise((res) => { + cache.watch({ + query: gql` + query { + foo + } + `, + id: cache.identify({}), + returnPartialData: true, + immediate: true, + optimistic: true, + callback: (diff) => { + expect(diff).toStrictEqual({ + result: {}, + complete: false, + missing: [ + new MissingFieldError( + "Can't find field 'foo' on ROOT_QUERY object", + expect.anything(), // query + expect.anything() // variables + ), + ], + }); + res(void 0); + }, + }); + }); + }); }); describe("defaultOptions", () => { diff --git a/src/__tests__/dataMasking.ts b/src/__tests__/dataMasking.ts index 8c386b67e6f..4c1c150ce0c 100644 --- a/src/__tests__/dataMasking.ts +++ b/src/__tests__/dataMasking.ts @@ -1360,8 +1360,7 @@ describe("client.watchQuery", () => { const { data, complete } = await fragmentStream.takeNext(); expect(data).toEqual({}); - // TODO: Update when https://github.com/apollographql/apollo-client/issues/12003 is fixed - expect(complete).toBe(true); + expect(complete).toBe(false); } }); @@ -1437,8 +1436,7 @@ describe("client.watchQuery", () => { const { data, complete } = await fragmentStream.takeNext(); expect(data).toEqual({}); - // TODO: Update when https://github.com/apollographql/apollo-client/issues/12003 is fixed - expect(complete).toBe(true); + expect(complete).toBe(false); } }); diff --git a/src/cache/inmemory/inMemoryCache.ts b/src/cache/inmemory/inMemoryCache.ts index 7be5b921ab5..08646b57eeb 100644 --- a/src/cache/inmemory/inMemoryCache.ts +++ b/src/cache/inmemory/inMemoryCache.ts @@ -24,6 +24,7 @@ import { print, cacheSizes, defaultCacheSizes, + getMainDefinition, } from "../../utilities/index.js"; import type { InMemoryCacheConfig, NormalizedCacheObject } from "./types.js"; import { StoreReader } from "./readFromStore.js"; @@ -256,6 +257,35 @@ export class InMemoryCache extends ApolloCache { public diff( options: Cache.DiffOptions ): Cache.DiffResult { + // Detect non-identifiable objects with fragment queries + // Only apply this fix when: + // 1. options.id is explicitly undefined (not just missing) + // 2. The query is a pure fragment query (not a named query with fragments) + if (options.id === undefined && hasOwn.call(options, "id")) { + const mainDef = getMainDefinition(options.query); + // Check if this is a pure fragment query (operation without name, only fragment spreads) + if ( + mainDef.kind === "OperationDefinition" && + !mainDef.name && + mainDef.selectionSet.selections.every( + (sel) => sel.kind === "FragmentSpread" + ) + ) { + return { + result: {} as TData, + complete: false, + missing: [ + new MissingFieldError( + "Can't determine completeness for fragment query on non-identifiable object", + "Can't determine completeness for fragment query on non-identifiable object", + options.query, + options.variables + ), + ], + }; + } + } + return this.storeReader.diffQueryAgainstStore({ ...options, store: options.optimistic ? this.optimisticData : this.data, diff --git a/src/react/hooks/__tests__/useFragment.test.tsx b/src/react/hooks/__tests__/useFragment.test.tsx index 3884c22338c..a5013b2990f 100644 --- a/src/react/hooks/__tests__/useFragment.test.tsx +++ b/src/react/hooks/__tests__/useFragment.test.tsx @@ -1639,8 +1639,7 @@ describe("useFragment", () => { const { data, complete } = await takeSnapshot(); expect(data).toEqual({}); - // TODO: Update when https://github.com/apollographql/apollo-client/issues/12003 is fixed - expect(complete).toBe(true); + expect(complete).toBe(false); } expect(console.warn).toHaveBeenCalledTimes(1); diff --git a/src/react/hooks/__tests__/useSuspenseFragment.test.tsx b/src/react/hooks/__tests__/useSuspenseFragment.test.tsx index d24d1804348..6d572779afe 100644 --- a/src/react/hooks/__tests__/useSuspenseFragment.test.tsx +++ b/src/react/hooks/__tests__/useSuspenseFragment.test.tsx @@ -870,64 +870,60 @@ it("does not rerender when fields with @nonreactive on nested fragment change", await expect(takeSnapshot).not.toRerender(); }); -// TODO: Update when https://github.com/apollographql/apollo-client/issues/12003 is fixed -it.failing( - "warns and suspends when passing parent object to `from` when key fields are missing", - async () => { - using _ = spyOnConsole("warn"); - - interface Fragment { - age: number; +it("warns and suspends when passing parent object to `from` when key fields are missing", async () => { + using _ = spyOnConsole("warn"); + + interface Fragment { + age: number; + } + + const fragment: TypedDocumentNode = gql` + fragment UserFields on User { + age } + `; - const fragment: TypedDocumentNode = gql` - fragment UserFields on User { - age - } - `; + const client = new ApolloClient({ cache: new InMemoryCache() }); - const client = new ApolloClient({ cache: new InMemoryCache() }); + const { replaceSnapshot, render, takeRender } = + createDefaultRenderStream(); + const { SuspenseFallback } = createDefaultTrackedComponents(); - const { replaceSnapshot, render, takeRender } = - createDefaultRenderStream(); - const { SuspenseFallback } = createDefaultTrackedComponents(); + function App() { + const result = useSuspenseFragment({ + fragment, + from: { __typename: "User" }, + }); - function App() { - const result = useSuspenseFragment({ - fragment, - from: { __typename: "User" }, - }); + replaceSnapshot({ result }); - replaceSnapshot({ result }); + return null; + } - return null; + using _disabledAct = disableActEnvironment(); + await render( + }> + + , + { + wrapper: ({ children }) => ( + {children} + ), } + ); - using _disabledAct = disableActEnvironment(); - await render( - }> - - , - { - wrapper: ({ children }) => ( - {children} - ), - } - ); - - expect(console.warn).toHaveBeenCalledTimes(1); - expect(console.warn).toHaveBeenCalledWith( - "Could not identify object passed to `from` for '%s' fragment, either because the object is non-normalized or the key fields are missing. If you are masking this object, please ensure the key fields are requested by the parent object.", - "UserFields" - ); + expect(console.warn).toHaveBeenCalledTimes(1); + expect(console.warn).toHaveBeenCalledWith( + "Could not identify object passed to `from` for '%s' fragment, either because the object is non-normalized or the key fields are missing. If you are masking this object, please ensure the key fields are requested by the parent object.", + "UserFields" + ); - { - const { renderedComponents } = await takeRender(); + { + const { renderedComponents } = await takeRender(); - expect(renderedComponents).toStrictEqual([SuspenseFallback]); - } + expect(renderedComponents).toStrictEqual([SuspenseFallback]); } -); +}); test("returns null if `from` is `null`", async () => { interface ItemFragment {