Skip to content

Commit 8740f19

Browse files
authored
Fixes #11849, BatchHttpLink not monitoring friendly (#11860)
* fix: reevaluate window.fetch each time BatchHttpLink is used, if not configured using options.fetch * chore: add changeset
1 parent 1ac0178 commit 8740f19

File tree

3 files changed

+87
-10
lines changed

3 files changed

+87
-10
lines changed

.changeset/late-days-give.md

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@apollo/client": patch
3+
---
4+
5+
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`.

src/link/batch-http/__tests__/batchHttpLink.ts

+64
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { ApolloLink } from "../../core/ApolloLink";
66
import { execute } from "../../core/execute";
77
import {
88
Observable,
9+
ObservableSubscription,
910
Observer,
1011
} from "../../../utilities/observables/Observable";
1112
import { BatchHttpLink } from "../batchHttpLink";
@@ -271,6 +272,8 @@ const createHttpLink = (httpArgs?: any) => {
271272
return new BatchHttpLink(args);
272273
};
273274

275+
const subscriptions = new Set<ObservableSubscription>();
276+
274277
describe("SharedHttpTest", () => {
275278
const data = { data: { hello: "world" } };
276279
const data2 = { data: { hello: "everyone" } };
@@ -300,10 +303,16 @@ describe("SharedHttpTest", () => {
300303
error,
301304
complete,
302305
};
306+
subscriptions.clear();
303307
});
304308

305309
afterEach(() => {
306310
fetchMock.restore();
311+
if (subscriptions.size) {
312+
// Tests within this describe block can add subscriptions to this Set
313+
// that they want to be canceled/unsubscribed after the test finishes.
314+
subscriptions.forEach((sub) => sub.unsubscribe());
315+
}
307316
});
308317

309318
it("raises warning if called with concat", () => {
@@ -624,6 +633,61 @@ describe("SharedHttpTest", () => {
624633
);
625634
});
626635

636+
it("uses the latest window.fetch function if options.fetch not configured", (done) => {
637+
const httpLink = createHttpLink({ uri: "data" });
638+
639+
const fetch = window.fetch;
640+
expect(typeof fetch).toBe("function");
641+
642+
const fetchSpy = jest.spyOn(window, "fetch");
643+
fetchSpy.mockImplementation(() =>
644+
Promise.resolve<Response>({
645+
text() {
646+
return Promise.resolve(
647+
JSON.stringify({
648+
data: { hello: "from spy" },
649+
})
650+
);
651+
},
652+
} as Response)
653+
);
654+
655+
const spyFn = window.fetch;
656+
expect(spyFn).not.toBe(fetch);
657+
658+
subscriptions.add(
659+
execute(httpLink, {
660+
query: sampleQuery,
661+
}).subscribe({
662+
error: done.fail,
663+
664+
next(result) {
665+
expect(fetchSpy).toHaveBeenCalledTimes(1);
666+
expect(result).toEqual({
667+
data: { hello: "from spy" },
668+
});
669+
670+
fetchSpy.mockRestore();
671+
expect(window.fetch).toBe(fetch);
672+
673+
subscriptions.add(
674+
execute(httpLink, {
675+
query: sampleQuery,
676+
}).subscribe({
677+
error: done.fail,
678+
next(result) {
679+
expect(result).toEqual({
680+
data: { hello: "world" },
681+
});
682+
done();
683+
},
684+
})
685+
);
686+
},
687+
})
688+
);
689+
});
690+
627691
itAsync(
628692
"prioritizes context headers over setup headers",
629693
(resolve, reject) => {

src/link/batch-http/batchHttpLink.ts

+18-10
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { ApolloLink } from "../core/index.js";
33
import {
44
Observable,
55
hasDirectives,
6+
maybe,
67
removeClientSetsFromDocument,
78
} from "../../utilities/index.js";
89
import { fromError } from "../utils/index.js";
@@ -27,6 +28,8 @@ export namespace BatchHttpLink {
2728
Omit<HttpOptions, "useGETForQueries">;
2829
}
2930

31+
const backupFetch = maybe(() => fetch);
32+
3033
/**
3134
* Transforms Operation for into HTTP results.
3235
* context can include the headers property, which will be passed to the fetch function
@@ -43,7 +46,7 @@ export class BatchHttpLink extends ApolloLink {
4346
let {
4447
uri = "/graphql",
4548
// use default global fetch if nothing is passed in
46-
fetch: fetcher,
49+
fetch: preferredFetch,
4750
print = defaultPrinter,
4851
includeExtensions,
4952
preserveHeaderCase,
@@ -55,14 +58,10 @@ export class BatchHttpLink extends ApolloLink {
5558
...requestOptions
5659
} = fetchParams || ({} as BatchHttpLink.Options);
5760

58-
// dev warnings to ensure fetch is present
59-
checkFetcher(fetcher);
60-
61-
//fetcher is set here rather than the destructuring to ensure fetch is
62-
//declared before referencing it. Reference in the destructuring would cause
63-
//a ReferenceError
64-
if (!fetcher) {
65-
fetcher = fetch;
61+
if (__DEV__) {
62+
// Make sure at least one of preferredFetch, window.fetch, or backupFetch
63+
// is defined, so requests won't fail at runtime.
64+
checkFetcher(preferredFetch || backupFetch);
6665
}
6766

6867
const linkConfig = {
@@ -163,7 +162,16 @@ export class BatchHttpLink extends ApolloLink {
163162
}
164163

165164
return new Observable<FetchResult[]>((observer) => {
166-
fetcher!(chosenURI, options)
165+
// Prefer BatchHttpLink.Options.fetch (preferredFetch) if provided, and
166+
// otherwise fall back to the *current* global window.fetch function
167+
// (see issue #7832), or (if all else fails) the backupFetch function we
168+
// saved when this module was first evaluated. This last option protects
169+
// against the removal of window.fetch, which is unlikely but not
170+
// impossible.
171+
const currentFetch =
172+
preferredFetch || maybe(() => fetch) || backupFetch;
173+
174+
currentFetch!(chosenURI, options)
167175
.then((response) => {
168176
// Make the raw response available in the context.
169177
operations.forEach((operation) =>

0 commit comments

Comments
 (0)