Skip to content

Commit

Permalink
Fixes #11849, BatchHttpLink not monitoring friendly (#11860)
Browse files Browse the repository at this point in the history
* fix: reevaluate window.fetch each time BatchHttpLink is used, if not configured using options.fetch

* chore: add changeset
  • Loading branch information
alessbell authored May 20, 2024
1 parent 1ac0178 commit 8740f19
Show file tree
Hide file tree
Showing 3 changed files with 87 additions and 10 deletions.
5 changes: 5 additions & 0 deletions .changeset/late-days-give.md
Original file line number Diff line number Diff line change
@@ -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`.
64 changes: 64 additions & 0 deletions src/link/batch-http/__tests__/batchHttpLink.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -271,6 +272,8 @@ const createHttpLink = (httpArgs?: any) => {
return new BatchHttpLink(args);
};

const subscriptions = new Set<ObservableSubscription>();

describe("SharedHttpTest", () => {
const data = { data: { hello: "world" } };
const data2 = { data: { hello: "everyone" } };
Expand Down Expand Up @@ -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", () => {
Expand Down Expand Up @@ -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<Response>({
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) => {
Expand Down
28 changes: 18 additions & 10 deletions src/link/batch-http/batchHttpLink.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -27,6 +28,8 @@ export namespace BatchHttpLink {
Omit<HttpOptions, "useGETForQueries">;
}

const backupFetch = maybe(() => fetch);

/**
* Transforms Operation for into HTTP results.
* context can include the headers property, which will be passed to the fetch function
Expand All @@ -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,
Expand All @@ -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 = {
Expand Down Expand Up @@ -163,7 +162,16 @@ export class BatchHttpLink extends ApolloLink {
}

return new Observable<FetchResult[]>((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) =>
Expand Down

0 comments on commit 8740f19

Please sign in to comment.