From 4334d30cc3fbedb4f736eff196c49a9f20a46704 Mon Sep 17 00:00:00 2001 From: Nicolas Charpentier Date: Thu, 19 Dec 2024 11:42:35 -0500 Subject: [PATCH] Compare `DocumentNode` used in `refetchQueries` as strings (#12236) Co-authored-by: Jerel Miller --- .changeset/gorgeous-sheep-knock.md | 5 + .size-limits.json | 4 +- src/core/QueryManager.ts | 40 +-- src/core/__tests__/QueryManager/index.ts | 298 ++++++++++++++++++++++- 4 files changed, 326 insertions(+), 21 deletions(-) create mode 100644 .changeset/gorgeous-sheep-knock.md diff --git a/.changeset/gorgeous-sheep-knock.md b/.changeset/gorgeous-sheep-knock.md new file mode 100644 index 00000000000..7d62428c804 --- /dev/null +++ b/.changeset/gorgeous-sheep-knock.md @@ -0,0 +1,5 @@ +--- +"@apollo/client": patch +--- + +Fix an issue with `refetchQueries` where comparing `DocumentNode`s internally by references could lead to an unknown query, even though the `DocumentNode` was indeed an active query—with a different reference. diff --git a/.size-limits.json b/.size-limits.json index c7b4947027f..54621796c0c 100644 --- a/.size-limits.json +++ b/.size-limits.json @@ -1,4 +1,4 @@ { - "dist/apollo-client.min.cjs": 41615, - "import { ApolloClient, InMemoryCache, HttpLink } from \"dist/index.js\" (production)": 34349 + "dist/apollo-client.min.cjs": 41639, + "import { ApolloClient, InMemoryCache, HttpLink } from \"dist/index.js\" (production)": 34381 } diff --git a/src/core/QueryManager.ts b/src/core/QueryManager.ts index e61e123c5f2..066dc137de9 100644 --- a/src/core/QueryManager.ts +++ b/src/core/QueryManager.ts @@ -899,15 +899,19 @@ export class QueryManager { include: InternalRefetchQueriesInclude = "active" ) { const queries = new Map>(); - const queryNamesAndDocs = new Map(); + const queryNames = new Map(); + const queryNamesAndQueryStrings = new Map(); const legacyQueryOptions = new Set(); if (Array.isArray(include)) { include.forEach((desc) => { if (typeof desc === "string") { - queryNamesAndDocs.set(desc, false); + queryNames.set(desc, desc); + queryNamesAndQueryStrings.set(desc, false); } else if (isDocumentNode(desc)) { - queryNamesAndDocs.set(this.transform(desc), false); + const queryString = print(this.transform(desc)); + queryNames.set(queryString, getOperationName(desc)); + queryNamesAndQueryStrings.set(queryString, false); } else if (isNonNullObject(desc) && desc.query) { legacyQueryOptions.add(desc); } @@ -935,12 +939,12 @@ export class QueryManager { if ( include === "active" || - (queryName && queryNamesAndDocs.has(queryName)) || - (document && queryNamesAndDocs.has(document)) + (queryName && queryNamesAndQueryStrings.has(queryName)) || + (document && queryNamesAndQueryStrings.has(print(document))) ) { queries.set(queryId, oq); - if (queryName) queryNamesAndDocs.set(queryName, true); - if (document) queryNamesAndDocs.set(document, true); + if (queryName) queryNamesAndQueryStrings.set(queryName, true); + if (document) queryNamesAndQueryStrings.set(print(document), true); } } }); @@ -969,15 +973,21 @@ export class QueryManager { }); } - if (__DEV__ && queryNamesAndDocs.size) { - queryNamesAndDocs.forEach((included, nameOrDoc) => { + if (__DEV__ && queryNamesAndQueryStrings.size) { + queryNamesAndQueryStrings.forEach((included, nameOrQueryString) => { if (!included) { - invariant.warn( - typeof nameOrDoc === "string" ? - `Unknown query named "%s" requested in refetchQueries options.include array` - : `Unknown query %o requested in refetchQueries options.include array`, - nameOrDoc - ); + const queryName = queryNames.get(nameOrQueryString); + + if (queryName) { + invariant.warn( + `Unknown query named "%s" requested in refetchQueries options.include array`, + queryName + ); + } else { + invariant.warn( + `Unknown anonymous query requested in refetchQueries options.include array` + ); + } } }); } diff --git a/src/core/__tests__/QueryManager/index.ts b/src/core/__tests__/QueryManager/index.ts index 5d6d9592bcc..1edd4e2c2f1 100644 --- a/src/core/__tests__/QueryManager/index.ts +++ b/src/core/__tests__/QueryManager/index.ts @@ -46,7 +46,7 @@ import wrap from "../../../testing/core/wrap"; import observableToPromise, { observableToPromiseAndSubscription, } from "../../../testing/core/observableToPromise"; -import { itAsync, wait } from "../../../testing/core"; +import { itAsync } from "../../../testing/core"; import { ApolloClient } from "../../../core"; import { mockFetchQuery } from "../ObservableQuery"; import { Concast, print } from "../../../utilities"; @@ -5156,6 +5156,151 @@ describe("QueryManager", () => { } ); + itAsync( + "should ignore (with warning) a document node in refetchQueries that has no active subscriptions", + (resolve, reject) => { + const mutation = gql` + mutation changeAuthorName { + changeAuthorName(newName: "Jack Smith") { + firstName + lastName + } + } + `; + const mutationData = { + changeAuthorName: { + firstName: "Jack", + lastName: "Smith", + }, + }; + const query = gql` + query getAuthors { + author { + firstName + lastName + } + } + `; + const data = { + author: { + firstName: "John", + lastName: "Smith", + }, + }; + const secondReqData = { + author: { + firstName: "Jane", + lastName: "Johnson", + }, + }; + const queryManager = mockQueryManager( + { + request: { query }, + result: { data }, + }, + { + request: { query }, + result: { data: secondReqData }, + }, + { + request: { query: mutation }, + result: { data: mutationData }, + } + ); + + const observable = queryManager.watchQuery({ query }); + return observableToPromise({ observable }, (result) => { + expect(result.data).toEqual(data); + }) + .then(() => { + // The subscription has been stopped already + return queryManager.mutate({ + mutation, + refetchQueries: [query], + }); + }) + .then(() => { + expect(consoleWarnSpy).toHaveBeenLastCalledWith( + 'Unknown query named "%s" requested in refetchQueries options.include array', + "getAuthors" + ); + }) + .then(resolve, reject); + } + ); + + itAsync( + "should ignore (with warning) a document node containing an anonymous query in refetchQueries that has no active subscriptions", + (resolve, reject) => { + const mutation = gql` + mutation changeAuthorName { + changeAuthorName(newName: "Jack Smith") { + firstName + lastName + } + } + `; + const mutationData = { + changeAuthorName: { + firstName: "Jack", + lastName: "Smith", + }, + }; + const query = gql` + query { + author { + firstName + lastName + } + } + `; + const data = { + author: { + firstName: "John", + lastName: "Smith", + }, + }; + const secondReqData = { + author: { + firstName: "Jane", + lastName: "Johnson", + }, + }; + const queryManager = mockQueryManager( + { + request: { query }, + result: { data }, + }, + { + request: { query }, + result: { data: secondReqData }, + }, + { + request: { query: mutation }, + result: { data: mutationData }, + } + ); + + const observable = queryManager.watchQuery({ query }); + return observableToPromise({ observable }, (result) => { + expect(result.data).toEqual(data); + }) + .then(() => { + // The subscription has been stopped already + return queryManager.mutate({ + mutation, + refetchQueries: [query], + }); + }) + .then(() => { + expect(consoleWarnSpy).toHaveBeenLastCalledWith( + "Unknown anonymous query requested in refetchQueries options.include array" + ); + }) + .then(resolve, reject); + } + ); + it("also works with a query document and variables", async () => { const mutation = gql` mutation changeAuthorName($id: ID!) { @@ -5228,12 +5373,157 @@ describe("QueryManager", () => { ); expect(observable.getCurrentResult().data).toEqual(secondReqData); - await wait(10); + await expect(stream).not.toEmitAnything(); + }); - queryManager["queries"].forEach((_, queryId) => { - expect(queryId).not.toContain("legacyOneTimeQuery"); + it("also works with a query document node", async () => { + const mutation = gql` + mutation changeAuthorName($id: ID!) { + changeAuthorName(newName: "Jack Smith", id: $id) { + firstName + lastName + } + } + `; + const mutationData = { + changeAuthorName: { + firstName: "Jack", + lastName: "Smith", + }, + }; + const query = gql` + query getAuthors($id: ID!) { + author(id: $id) { + firstName + lastName + } + } + `; + const data = { + author: { + firstName: "John", + lastName: "Smith", + }, + }; + const secondReqData = { + author: { + firstName: "Jane", + lastName: "Johnson", + }, + }; + + const variables = { id: "1234" }; + const mutationVariables = { id: "2345" }; + const queryManager = mockQueryManager( + { + request: { query, variables }, + result: { data }, + delay: 10, + }, + { + request: { query, variables }, + result: { data: secondReqData }, + delay: 100, + }, + { + request: { query: mutation, variables: mutationVariables }, + result: { data: mutationData }, + delay: 10, + } + ); + const observable = queryManager.watchQuery({ query, variables }); + const stream = new ObservableStream(observable); + + await expect(stream).toEmitMatchedValue({ data }); + + await queryManager.mutate({ + mutation, + variables: mutationVariables, + refetchQueries: [query], }); + await expect(stream).toEmitMatchedValue( + { data: secondReqData }, + { timeout: 150 } + ); + expect(observable.getCurrentResult().data).toEqual(secondReqData); + + await expect(stream).not.toEmitAnything(); + }); + + it("also works with different references of a same query document node", async () => { + const mutation = gql` + mutation changeAuthorName($id: ID!) { + changeAuthorName(newName: "Jack Smith", id: $id) { + firstName + lastName + } + } + `; + const mutationData = { + changeAuthorName: { + firstName: "Jack", + lastName: "Smith", + }, + }; + const query = gql` + query getAuthors($id: ID!) { + author(id: $id) { + firstName + lastName + } + } + `; + const data = { + author: { + firstName: "John", + lastName: "Smith", + }, + }; + const secondReqData = { + author: { + firstName: "Jane", + lastName: "Johnson", + }, + }; + + const variables = { id: "1234" }; + const mutationVariables = { id: "2345" }; + const queryManager = mockQueryManager( + { + request: { query, variables }, + result: { data }, + delay: 10, + }, + { + request: { query, variables }, + result: { data: secondReqData }, + delay: 100, + }, + { + request: { query: mutation, variables: mutationVariables }, + result: { data: mutationData }, + delay: 10, + } + ); + const observable = queryManager.watchQuery({ query, variables }); + const stream = new ObservableStream(observable); + + await expect(stream).toEmitMatchedValue({ data }); + + await queryManager.mutate({ + mutation, + variables: mutationVariables, + // spread the query into a new object to simulate multiple instances + refetchQueries: [{ ...query }], + }); + + await expect(stream).toEmitMatchedValue( + { data: secondReqData }, + { timeout: 150 } + ); + expect(observable.getCurrentResult().data).toEqual(secondReqData); + await expect(stream).not.toEmitAnything(); });