From aa1ee9f3b99878960d2108fea2d805d54c5d42a7 Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Wed, 18 Dec 2024 13:26:06 +0100 Subject: [PATCH 1/3] add GH action to run tests against react@canary on a schedule (#12232) --- .github/workflows/scheduled-test-canary.yml | 25 +++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 .github/workflows/scheduled-test-canary.yml diff --git a/.github/workflows/scheduled-test-canary.yml b/.github/workflows/scheduled-test-canary.yml new file mode 100644 index 00000000000..639878253ba --- /dev/null +++ b/.github/workflows/scheduled-test-canary.yml @@ -0,0 +1,25 @@ +# a GitHub Action that once a day runs all tests from `main` and `release-*` branches +# with the latest `canary` and `experimental` release of `react` and `react-dom` + +on: + schedule: + - cron: "0 0 * * *" + workflow_dispatch: +jobs: + test: + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + tag: + - canary + - experimental + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-node@v4 + with: + node-version: 22.x + - uses: bahmutov/npm-install@v1 + - run: npm install react@${{ matrix.tag }} react-dom@${{ matrix.tag }} + # tests can be flaky, this runs only once a day and we want to minimize false negatives - retry up to three times + - run: "parallel --line-buffer -j 1 --retries 3 'npm run test:ci -- --selectProjects ' ::: 'ReactDOM 19'" From 2fb7d7a3362b7d7296a5ebac8081cf0bfc1865eb Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Wed, 18 Dec 2024 17:03:21 +0100 Subject: [PATCH 2/3] canary test adjustments (#12233) * run on multiple branches, log installed versions, adjust test command * add `|| true` * different react version logging * allow inputs on dispatch * wording --- .github/workflows/scheduled-test-canary.yml | 25 ++++++++++++++++----- 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/.github/workflows/scheduled-test-canary.yml b/.github/workflows/scheduled-test-canary.yml index 639878253ba..42d5442d948 100644 --- a/.github/workflows/scheduled-test-canary.yml +++ b/.github/workflows/scheduled-test-canary.yml @@ -1,25 +1,38 @@ # a GitHub Action that once a day runs all tests from `main` and `release-*` branches # with the latest `canary` and `experimental` release of `react` and `react-dom` - +name: Scheduled React Canary Test on: schedule: - cron: "0 0 * * *" workflow_dispatch: + inputs: + branches: + description: "Branches to test" + required: true + default: '["main", "release-3.13", "release-4.0"]' + tags: + description: "React and React-DOM versions" + required: true + default: '["canary", "experimental"]' jobs: test: runs-on: ubuntu-latest strategy: fail-fast: false matrix: - tag: - - canary - - experimental + tag: ${{ fromJson(github.event_name == 'workflow_dispatch' && inputs.tags || '["canary", "experimental"]') }} + branch: ${{ fromJson(github.event_name == 'workflow_dispatch' && inputs.branches || '["main", "release-3.13", "release-4.0"]') }} steps: - uses: actions/checkout@v4 + with: + ref: ${{ matrix.branch }} - uses: actions/setup-node@v4 with: node-version: 22.x - uses: bahmutov/npm-install@v1 - - run: npm install react@${{ matrix.tag }} react-dom@${{ matrix.tag }} + - run: | + npm install react@${{ matrix.tag }} react-dom@${{ matrix.tag }} # tests can be flaky, this runs only once a day and we want to minimize false negatives - retry up to three times - - run: "parallel --line-buffer -j 1 --retries 3 'npm run test:ci -- --selectProjects ' ::: 'ReactDOM 19'" + - run: | + node -e 'console.log("\n\nReact %s, React-DOM %s\n\n", require("react").version, require("react-dom").version)' + parallel --line-buffer -j 1 --retries 3 'npm test -- --logHeapUsage --selectProjects ' ::: 'ReactDOM 19' From 4334d30cc3fbedb4f736eff196c49a9f20a46704 Mon Sep 17 00:00:00 2001 From: Nicolas Charpentier Date: Thu, 19 Dec 2024 11:42:35 -0500 Subject: [PATCH 3/3] 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(); });