Ensure transporters are using isomorphic-fetch instead of browser only whatwg-fetch

[guilhermeblanco]

Introduction

I have 3 distinct projects:
A - Client UI. Executes GraphQL queries against GraphQL server B
B - GraphQL server, responsible to fetch information from third-party vendor API and upon response trigger subsequent queries to GraphQL server C
C - GraphQL server, wrapping another third-party vendor API

Problem resides on B. Whenever A issues a query that contains the field that requires B to contact C, it crashes.

This is understandable, since apollo-client transports use whatwg-fetch, which is meant to be used by browsers, and not by a node server. Switching whatwg-fetch to a universally compatible package isomorphic-fetch (which uses the whatwg-fetch when on browser and node-fetch when on server), problem disappears.

Situation

On a query that requires performing a query to server C, I have the following excerpt:

import { GraphQLNonNull, GraphQLObjectType, GraphQLScalarType, GraphQLString } from 'graphql'
import gql from 'graphql-tag'
import ModuleInterfaceType, { interfaceFields } from '../ModuleInterfaceType'

const ProductOverviewModuleType = new GraphQLObjectType({
    name: 'ProductOverviewModuleType',
    description: '...',
    interfaces: [ ModuleInterfaceType ],
    fields: () => ({
        ...interfaceFields,
        product: {
            type: new GraphQLNonNull(new GraphQLScalarType({
                name: 'Product',
                serialize: value => value,
                parseValue: value => value,
                parseLiteral: ast => ast.value
            })),
            description: 'product',
            resolve: ({ fields }, args, context) => {
                const query = gql`query ProductQuery($product: String!) {
                    ProductQuery(name: $product) {
                        id,
                        name
                        label,
                        description
                    }
                }`;

                // The following line triggers the error
                return context.serviceC
                    .query({
                        query: query,
                        variables: { product: fields.product }
                    })
                    .then(entry => entry.data.ProductQuery);
            }
        },
        label: {
            type: GraphQLString,
            description: 'product label override',
            resolve: ({ fields }) => fields.label
        },
        description: {
            type: GraphQLString,
            description: 'product description override',
            resolve: ({ fields }) => fields.description
        }
    })
});

export const CONTENT_TYPE = 'productOverviewModule';

export default ProductOverviewModuleType

Error

{ Error
    at new ApolloError (<path_to_project>/src/errors/ApolloError.ts:32:18)
    at <path_to_project>/src/core/QueryManager.ts:368:31
    at <path_to_project>/src/core/QueryManager.ts:1051:13
    at Array.forEach (native)
    at <path_to_project>/src/core/QueryManager.ts:1046:19
    at <path_to_project>/node_modules/lodash/_createBaseFor.js:17:11
    at baseForOwn (<path_to_project>/node_modules/lodash/_baseForOwn.js:13:20)
    at forOwn (<path_to_project>/node_modules/lodash/forOwn.js:33:20)
    at QueryManager.broadcastQueries (<path_to_project>/src/core/QueryManager.ts:1041:5)
    at Array.<anonymous> (<path_to_project>/src/core/QueryManager.ts:224:14)
  graphQLErrors: null,
  networkError:
   ReferenceError: fetch is not defined
       at HTTPFetchNetworkInterface.fetchFromRemoteEndpoint (<path_to_project>/src/transport/networkInterface.ts:162:12)
       at process._tickDomainCallback (internal/process/next_tick.js:129:7),
  stack: 'Error\n    at new ApolloError (<path_to_project>/src/errors/ApolloError.ts:32:18)\n    at <path_to_project>/src/core/QueryManager.ts:368:31\n    at <path_to_project>/src/core/QueryManager.ts:1051:13\n    at Array.forEach (native)\n    at <path_to_project>/src/core/QueryManager.ts:1046:19\n    at <path_to_project>/node_modules/lodash/_createBaseFor.js:17:11\n    at baseForOwn (<path_to_project>/node_modules/lodash/_baseForOwn.js:13:20)\n    at forOwn (<path_to_project>/node_modules/lodash/forOwn.js:33:20)\n    at QueryManager.broadcastQueries (<path_to_project>/src/core/QueryManager.ts:1041:5)\n    at Array.<anonymous> (<path_to_project>/src/core/QueryManager.ts:224:14)',
  message: 'Network error: fetch is not defined',
  extraInfo: undefined }

Workaround

I'm not being blocked by this issue, as I defined isomorphic-fetch import to global in my main JS file, like this:

import fetch from 'isomorphic-fetch'

global.fetch = fetch;

[apollo-cla]

@guilhermeblanco: 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/

[guilhermeblanco]

CLA signed.

[stubailo]

You can also just install isomorphic-fetch yourself, right? Usually libraries don't ship these kinds of polyfills at all but we decided to ship one for the primary browser use case. Using Apollo on the server seems more advanced so perhaps it's fine to ask people to install isomorphic-fetch themselves?

[guilhermeblanco]

@stubailo That's what I did, but it doesn't solve the problem.
I have to rely on the workaround I also described to get it to work.

[stubailo]

That's surprising, since it works well here in GitHunt's server side rendering code: https://github.com/apollostack/GitHunt-React/blob/b91466f625ba5b43bed3b7957a406eccb837de0f/ui/server.js#L9

Are you sure something else isn't going on?

Also, I would rather accept a PR that checks for global.fetch and prints a nice warning for people to install a polyfill I think.

[guilhermeblanco]

@stubailo GitHunt's makes a global import, then defining global.fetch. That's why it "happens" to work in that case. Remove that line and you'll see apollo-client crash.

I assume this library should operate on both ends (server and client), so it makes sense it use a tool that enables that. isomorphic-fetch is not a complex tool, and will address this problem of this library forever.

However, if you still think apollo-client should focus on frontend only and consider backend as "extra", that's also fine. But I'd have to reconsider using this library as potential unexpected behavior that future releases may bring to my code.

[kinncj]

if you still think apollo-client should focus on frontend

apollo-client should be a client no matter where it will be run... I have several graphql clients in node, and that would be super useful not to mention cleaner... I'd rather have the client to behave as a client that's not environment dependent (browser, v8 on the backend, etc) than a browser only.

[helfer]

Thanks @guilhermeblanco. As far as I can tell there's no downside to using isomorphic-fetch as opposed to whatwg-fetch, so I'll go ahead and merge this.

[sntran]

My apology if this is not a correct place to report the issue, but I just want to check if this is actually an issue and not just me.

I have upgrade apollo-client today to 0.5.22 for my react-native app. 0.5.21 works fine, but on 0.5.22, I got the exception:

Can't find variable: self

It's thrown from node_modules/isomorphic-fetch/fetch-npm-browserify.js. As I'm not familiar enough with this, I assume this is from the changes in this PR, as I see there were a few similar issues reported in isomorphic-fetch repo, such as this PR.

If this is an actual issue on isomorphic-fetch side, maybe we need to revert this?

[helfer]

@sntran Hm, that's unfortunate indeed. We definitely can't use isomorphic-fetch if it breaks react-native.

@guilhermeblanco maybe you can convince the author of isomorphic-fetch to take care of the issue on their side or find another isomorphic fetch package?

[kinncj]

@sntran matthew-andrews/isomorphic-fetch#23