search

matthewmueller / graph.ql

Faster and simpler way to create GraphQL servers
633 stars 34 forks source link

Change graphql into a peer dependency #28

Open a-s-o opened 7 years ago

a-s-o commented 7 years ago

Hi,

Great project; I find writing graphql with this is great. However, I have ran into issues where the internal graphql version used by this package is hard coded at 0.7.1 while the latest is 0.8.2. graphql.GraphQLSchema must come from the same version as internally there are numerous instanceof checks on the objects.

This pull request changes graphql into a peer dependecy so any version can be used and the package will still work. So far it seems okay on my system. There are no huge changes in v0.7.1 to v0.8.2 from looking at their change long. Couple of spec changes that don't seem to have any impact so far from my testing.

This will change the install procedure from npm install graph.ql into npm install graphql graph.ql which is a bit confusing however, will reduce the burden on this package from keeping up with their version changes.

Hope that is clear, as I am typing this out a bit in a hurry.

Thanks,

Amandeep

matthewmueller commented 7 years ago

I don't think it needs to be a peer dependency, since this module will work fine if you have multiple different version of graphql installed on your system. How about relaxing the dependency a bit to major changes?

hasnat commented 7 years ago

@matthewmueller graphql complains Ensure that there are not multiple versions of GraphQL installed in your node_modules directory Am not saying its due to graph.ql but other libraries mostly use peer dependency for graphql I had issue when working with express-graphql and graph.ql

Altho your point is still also valid to keep it in dependencies.

As I saw your comments in other pull request I resolved my issue by adding rm in post install in npm scripts "postinstall": "rm -rf node_modules/graph.ql/node_modules/graphql",

a-s-o commented 7 years ago

Yes, I think this package works fine with all versions of grapqhql but the issue occurs when we want to use the resulting schema with another package.

My use case is to use this package to write my schema and implementation (nicer syntax) and then give it over to apollo-server where it fails with the error that hasnat described