From efe7e49e88bf6944b3672ee59da00e7cec33ba46 Mon Sep 17 00:00:00 2001 From: Alessia Bellisario Date: Mon, 20 May 2024 13:23:33 -0400 Subject: [PATCH 1/2] fix: reevaluate window.fetch each time BatchHttpLink is used, if not configured using options.fetch --- .../batch-http/__tests__/batchHttpLink.ts | 64 +++++++++++++++++++ src/link/batch-http/batchHttpLink.ts | 28 +++++--- 2 files changed, 82 insertions(+), 10 deletions(-) diff --git a/src/link/batch-http/__tests__/batchHttpLink.ts b/src/link/batch-http/__tests__/batchHttpLink.ts index 6dea1805b89..1209b0414c1 100644 --- a/src/link/batch-http/__tests__/batchHttpLink.ts +++ b/src/link/batch-http/__tests__/batchHttpLink.ts @@ -6,6 +6,7 @@ import { ApolloLink } from "../../core/ApolloLink"; import { execute } from "../../core/execute"; import { Observable, + ObservableSubscription, Observer, } from "../../../utilities/observables/Observable"; import { BatchHttpLink } from "../batchHttpLink"; @@ -271,6 +272,8 @@ const createHttpLink = (httpArgs?: any) => { return new BatchHttpLink(args); }; +const subscriptions = new Set(); + describe("SharedHttpTest", () => { const data = { data: { hello: "world" } }; const data2 = { data: { hello: "everyone" } }; @@ -300,10 +303,16 @@ describe("SharedHttpTest", () => { error, complete, }; + subscriptions.clear(); }); afterEach(() => { fetchMock.restore(); + if (subscriptions.size) { + // Tests within this describe block can add subscriptions to this Set + // that they want to be canceled/unsubscribed after the test finishes. + subscriptions.forEach((sub) => sub.unsubscribe()); + } }); it("raises warning if called with concat", () => { @@ -624,6 +633,61 @@ describe("SharedHttpTest", () => { ); }); + it("uses the latest window.fetch function if options.fetch not configured", (done) => { + const httpLink = createHttpLink({ uri: "data" }); + + const fetch = window.fetch; + expect(typeof fetch).toBe("function"); + + const fetchSpy = jest.spyOn(window, "fetch"); + fetchSpy.mockImplementation(() => + Promise.resolve({ + text() { + return Promise.resolve( + JSON.stringify({ + data: { hello: "from spy" }, + }) + ); + }, + } as Response) + ); + + const spyFn = window.fetch; + expect(spyFn).not.toBe(fetch); + + subscriptions.add( + execute(httpLink, { + query: sampleQuery, + }).subscribe({ + error: done.fail, + + next(result) { + expect(fetchSpy).toHaveBeenCalledTimes(1); + expect(result).toEqual({ + data: { hello: "from spy" }, + }); + + fetchSpy.mockRestore(); + expect(window.fetch).toBe(fetch); + + subscriptions.add( + execute(httpLink, { + query: sampleQuery, + }).subscribe({ + error: done.fail, + next(result) { + expect(result).toEqual({ + data: { hello: "world" }, + }); + done(); + }, + }) + ); + }, + }) + ); + }); + itAsync( "prioritizes context headers over setup headers", (resolve, reject) => { diff --git a/src/link/batch-http/batchHttpLink.ts b/src/link/batch-http/batchHttpLink.ts index cb80e8d3809..d0412f68cf2 100644 --- a/src/link/batch-http/batchHttpLink.ts +++ b/src/link/batch-http/batchHttpLink.ts @@ -3,6 +3,7 @@ import { ApolloLink } from "../core/index.js"; import { Observable, hasDirectives, + maybe, removeClientSetsFromDocument, } from "../../utilities/index.js"; import { fromError } from "../utils/index.js"; @@ -27,6 +28,8 @@ export namespace BatchHttpLink { Omit; } +const backupFetch = maybe(() => fetch); + /** * Transforms Operation for into HTTP results. * context can include the headers property, which will be passed to the fetch function @@ -43,7 +46,7 @@ export class BatchHttpLink extends ApolloLink { let { uri = "/graphql", // use default global fetch if nothing is passed in - fetch: fetcher, + fetch: preferredFetch, print = defaultPrinter, includeExtensions, preserveHeaderCase, @@ -55,14 +58,10 @@ export class BatchHttpLink extends ApolloLink { ...requestOptions } = fetchParams || ({} as BatchHttpLink.Options); - // dev warnings to ensure fetch is present - checkFetcher(fetcher); - - //fetcher is set here rather than the destructuring to ensure fetch is - //declared before referencing it. Reference in the destructuring would cause - //a ReferenceError - if (!fetcher) { - fetcher = fetch; + if (__DEV__) { + // Make sure at least one of preferredFetch, window.fetch, or backupFetch + // is defined, so requests won't fail at runtime. + checkFetcher(preferredFetch || backupFetch); } const linkConfig = { @@ -163,7 +162,16 @@ export class BatchHttpLink extends ApolloLink { } return new Observable((observer) => { - fetcher!(chosenURI, options) + // Prefer BatchHttpLink.Options.fetch (preferredFetch) if provided, and + // otherwise fall back to the *current* global window.fetch function + // (see issue #7832), or (if all else fails) the backupFetch function we + // saved when this module was first evaluated. This last option protects + // against the removal of window.fetch, which is unlikely but not + // impossible. + const currentFetch = + preferredFetch || maybe(() => fetch) || backupFetch; + + currentFetch!(chosenURI, options) .then((response) => { // Make the raw response available in the context. operations.forEach((operation) => From 3f5f00f0dd67e7f38c979aadbb2cfa65a50a94c5 Mon Sep 17 00:00:00 2001 From: Alessia Bellisario Date: Mon, 20 May 2024 13:26:05 -0400 Subject: [PATCH 2/2] chore: add changeset --- .changeset/late-days-give.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/late-days-give.md diff --git a/.changeset/late-days-give.md b/.changeset/late-days-give.md new file mode 100644 index 00000000000..4fe36c2d495 --- /dev/null +++ b/.changeset/late-days-give.md @@ -0,0 +1,5 @@ +--- +"@apollo/client": patch +--- + +Fixes [#11849](https://github.com/apollographql/apollo-client/issues/11849) by reevaluating `window.fetch` each time `BatchHttpLink` uses it, if not configured via `options.fetch`. Takes the same approach as PR [#8603](https://github.com/apollographql/apollo-client/pull/8603) which fixed the same issue in `HttpLink`.