Make it possible to pass custom fetch to rest data source

[jbachhardie]

This addresses #1729

We were unsure whether it would be better for the RESTDataSource constructor to take a fetch implementation or an options object with a fetch property. We stuck to the former because it is simpler, not requiring the creation of a new type, and because the HTTPCache constructor seems to be a precedent for this pattern.

I know the feature hasn't been given the go-ahead but we were making these changes for our own use anyway and they're non-breaking so figured we'd put a PR in.

TODO:

• Update CHANGELOG.md with your change (include reference to issue & this PR)
• Make sure all of the significant new logic is covered by tests
• Rebase your changes on master so that they can be merged easily
• Make sure all tests and linter rules pass

[apollo-cla]

@jbachhardie: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Meteor Contributor Agreement here: https://contribute.meteor.com/

[codecov-io]

Codecov Report

Merging #1807 into master will decrease coverage by 3.91%.
The diff coverage is 71.42%.

@@            Coverage Diff             @@
##           master    #1807      +/-   ##
==========================================
- Coverage    77.8%   73.89%   -3.92%     
==========================================
  Files          30       29       -1     
  Lines        1149     1153       +4     
  Branches      268      297      +29     
==========================================
- Hits          894      852      -42     
- Misses        245      290      +45     
- Partials       10       11       +1

Impacted Files	Coverage Δ
...kages/apollo-datasource-rest/src/RESTDataSource.ts	86.74% <66.66%> (+0.32%)	⬆️
packages/apollo-datasource-rest/src/HTTPCache.ts	97.91% <75%> (+0.04%)	⬆️
packages/apollo-server-core/src/runHttpQuery.ts	30.2% <0%> (-17.03%)	⬇️
packages/apollo-server/src/index.ts	96.77% <0%> (-0.11%)	⬇️
...ackages/apollo-server-express/src/expressApollo.ts	86.36% <0%> (ø)	⬆️
packages/apollo-server-core/src/graphqlOptions.ts	75% <0%> (ø)	⬆️
...ackages/apollo-server-core/src/utils/dispatcher.ts
...kages/apollo-server-core/src/requestPipelineAPI.ts
packages/apollo-server-core/src/requestPipeline.ts
packages/apollo-server-core/src/runQuery.ts	80.51% <0%> (ø)
... and 1 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 94fc935...b01e10c. Read the comment docs.

[hollsteinm]

Is there an ETA when the conflicts will be resolved and merged? I am finding this to be a useful feature as well.

[alexanderkhivrych]

@jbachhardie plz merge it ASAP

[jbachhardie]

I've resolved the conflicts (which were just changelog stuff) if that's what you mean. I can't merge the PR since I'm not a maintainer.

[alexanderkhivrych]

@renovate-bot

[alexanderkhivrych]

@jbachhardie plz pull last master

[alexanderkhivrych]

@abernix can you help with this pr, i want to use custom fetch in the data source, because i have a good sdk client, so i want to use this "custom fetch" with data source, i don`t want to replace cache logic, plz ASAP

[alexanderkhivrych]

https://github.com/apollographql/apollo-server/issues/1729

[alexanderkhivrych]

@martijnwalraven

[intpp]

@martijnwalraven @abernix @evans merge it please

[tlgimenes]

Hey, how is this feature going guys ? I also need it and it looks it's ready for merging ... Thanks !

[matthewfitch23]

@jbachhardie do you have an example of how a custom fetch could be used with this?

[bulka777]

@jbachhardie do you have an example of how a custom fetch could be used with this?

export class SomeAPI extends RESTDataSource {
  constructor(customFetch) {
    super(customFetch);
  }
}
// somewhere else
new SomeAPI(customFetch);

Would be really awesome to have this so that fetch could be easily instrumented.

[btaylor-ihr]

Hey, @jbachhardie, @martijnwalraven, and @abernix :)

Do you know if there is any chance of this getting merged? I'm looking to use https://www.npmjs.com/package/fetch-retry for retry logic whilst avoiding creating my own RESTDataSource if I can help it. Let me know if you recommend something else 👍

[hwillson]

Thanks very much for working on this @jbachhardie. We'll get this merged shortly.

[hwillson]

This looks great @jbachhardie - thanks very much!

[bryancuster]

I am trying to add in zipkin-instrumentation-fetch to our apollo-server-express so I needed a custom fetch instance. While passing in a custom instance of fetch seems to be working, adding headers seems to have broken in willSendRequest.

willSendRequest(request) {
    request.headers.set('Authorization', `Bearer ${this.context.user.bearerToken}`)
  }