af / apollo-local-query

Simpler server rendering with apollo-client 1.x, using a local GraphQL networkInterface
MIT License
64 stars 7 forks source link

Use graphql library by default #6

Closed vladshcherbin closed 7 years ago

vladshcherbin commented 7 years ago

Currently, we need to pass graphql library to createLocalInterface, why do we need this?

Apollo is using it by default, is it possible to use it by default too so we have to provide only the schema or what is the use-case for this?

af commented 7 years ago

Hi,

The main reason for that is to decouple the graphql version from this library. If graphql is a dependency, newer versions of that lib aren't usable unless we continually bump the package.json. Also you're going to need to install graphql separately to define your schema, so it doesn't save a dependency either. I didn't realize that Apollo depended on graphql explicitly, admittedly that does make this a bit less compelling.

A side benefit is that you can also mock graphql's interface for server side testing, and simply pass your mock into this library, if you need to.

I'm closing this issue with the rationale above, but feel free to continue discussing. I'll re-open if you can change my mind :P

vladshcherbin commented 7 years ago

@af As for the main reason about the version, peerDependencies can probably solve this easily. For example, this is from graphql-tools.

af commented 7 years ago

True, a peer dep would work in this case. For your use case, do you only have the graphql package installed in order to feed it into this module?

vladshcherbin commented 7 years ago

@af I'm using a lot of apollo packages, both server and client side. It's already required by at least one of them.

From server graphql-tools, from client apollo-client, react-apollo are using it.

Although this packages are using graphql package, I never had to import it. In fact, in react-apollo you import it from it:

import { graphql } from 'react-apollo'

The whole reason of this issue is that a lot of apollo packages are using graphql under the hood, it's a little strange to directly import and pass it only in this package.

vladshcherbin commented 7 years ago

From graphql.org page, all listed js libraries are using graphql package so I think this package can import it under the hood and require only schema to be passed in.

vladshcherbin commented 7 years ago

That's probably all of my reasons, maybe it's enough for you to change your mind. I'm also okay if you decide to leave everything as it is. 😉