From baf7c59253d5f09fe02b277758a01417c81614d6 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Fri, 6 Aug 2021 14:06:45 -0400 Subject: [PATCH 1/3] Reevaluate window.fetch each time HttpLink uses it, if unconfigured. Should help with a variety of strategies for instrumenting window.fetch, as discussed in issue #7832. --- src/__tests__/__snapshots__/exports.ts.snap | 1 + src/link/http/createHttpLink.ts | 26 +++++++++++++-------- src/utilities/index.ts | 1 + 3 files changed, 18 insertions(+), 10 deletions(-) diff --git a/src/__tests__/__snapshots__/exports.ts.snap b/src/__tests__/__snapshots__/exports.ts.snap index 9dd91569351..3835a31be88 100644 --- a/src/__tests__/__snapshots__/exports.ts.snap +++ b/src/__tests__/__snapshots__/exports.ts.snap @@ -362,6 +362,7 @@ Array [ "iterateObserversSafely", "makeReference", "makeUniqueId", + "maybe", "maybeDeepFreeze", "mergeDeep", "mergeDeepArray", diff --git a/src/link/http/createHttpLink.ts b/src/link/http/createHttpLink.ts index 80f22484a39..6282377a948 100644 --- a/src/link/http/createHttpLink.ts +++ b/src/link/http/createHttpLink.ts @@ -14,26 +14,25 @@ import { import { createSignalIfSupported } from './createSignalIfSupported'; import { rewriteURIForGET } from './rewriteURIForGET'; import { fromError } from '../utils'; +import { maybe } from '../../utilities'; + +const backupFetch = maybe(() => fetch); export const createHttpLink = (linkOptions: HttpOptions = {}) => { let { uri = '/graphql', // use default global fetch if nothing passed in - fetch: fetcher, + fetch: preferredFetch, includeExtensions, useGETForQueries, includeUnusedVariables = false, ...requestOptions } = linkOptions; - // 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 = { @@ -142,7 +141,14 @@ export const createHttpLink = (linkOptions: HttpOptions = {}) => { } return new Observable(observer => { - fetcher!(chosenURI, options) + // Prefer linkOptions.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 => { operation.setContext({ response }); return response; diff --git a/src/utilities/index.ts b/src/utilities/index.ts index ff4e765c8f1..9f151b88386 100644 --- a/src/utilities/index.ts +++ b/src/utilities/index.ts @@ -84,6 +84,7 @@ export { export * from './common/mergeDeep'; export * from './common/cloneDeep'; export * from './common/maybeDeepFreeze'; +export * from './common/maybe'; export * from './observables/iteration'; export * from './observables/asyncMap'; export * from './observables/Concast'; From 3ac4b8d1f173e3c762cac0926eff810a4d5bbeb5 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Fri, 6 Aug 2021 14:49:48 -0400 Subject: [PATCH 2/3] Verify HttpLink uses latest window.fetch unless configured otherwise. --- src/link/http/__tests__/HttpLink.ts | 55 +++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/src/link/http/__tests__/HttpLink.ts b/src/link/http/__tests__/HttpLink.ts index 3d17685832b..d848021a36a 100644 --- a/src/link/http/__tests__/HttpLink.ts +++ b/src/link/http/__tests__/HttpLink.ts @@ -54,6 +54,7 @@ describe('HttpLink', () => { const data2 = { data: { hello: 'everyone' } }; const mockError = { throws: new TypeError('mock me') }; let subscriber: ZenObservable.Observer; + const subscriptions = new Set(); beforeEach(() => { fetchMock.restore(); @@ -74,10 +75,17 @@ describe('HttpLink', () => { 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('does not need any constructor arguments', () => { @@ -830,6 +838,53 @@ describe('HttpLink', () => { ); }); + 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(); + }, + })); + }, + })); + }); + it('prioritizes context over setup', done => { const variables = { params: 'stub' }; const middleware = new ApolloLink((operation, forward) => { From 8b0c71654529dfe4ce1c449e1400e651da954235 Mon Sep 17 00:00:00 2001 From: Ben Newman Date: Fri, 6 Aug 2021 15:04:23 -0400 Subject: [PATCH 3/3] Mention PR #8603 in CHANGELOG.md. --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 57f1f6ed16c..1558595f404 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,10 @@ +## Apollo Client 3.4.6 (not yet released) + +## Improvements + +- Reevaluate `window.fetch` each time `HttpLink` uses it, if not configured using `options.fetch`. This change enables a variety of strategies for instrumenting `window.fetch`, without requiring those strategies to run before `@apollo/client/link/http` is first imported.
+ [@benjamn](https://github.com/benjamn) in [#8603](https://github.com/apollographql/apollo-client/pull/8603) + ## Apollo Client 3.4.5 ### Bug Fixes