Update graphql spec to April2016

[danez]

There is a new version of the GraphQL spec April2016.

It changed some parts in the introspection.
I might have time to open a PR for this, but probably at earliest in two weeks.

[vladar]

Hey, thanks for proposing PR!

Actually I've been waiting for new major graphql-js release to start importing latest changes. And already started working on this. Will post here when scope of changes will be more clear for me.

It is also a good moment to add changes specific to graphql-php so that we only had one breaking change migration.

So if anyone have ideas or proposals - feel free to post them while it is not too late for this %)

[danez]

What bugs me the most currently is defining types. It feels a little bit weird to define the types completely with arrays - especially interfaces - , although php can handle classes and interfaces just fine.

What I wanted to try is creating a system where one could define types with real classes and annotation, like this https://gist.github.com/danez/34e3fd37201c69395317c70b44410299 I haven't really thought about this in depth, but I think it should be possible. It should be like doctrine.

This doesn't necessarily needs to go into this library, but could also be a layer above graphql-php, because usually in production apps you want to precompile annotations anyway.

[vladar]

Yeah, I completely agree with you. I've been thinking about annotations too. Also I know that some people use custom builder classes to reduce the boilerplate.

Never found time to experiment with it, but it can be definitely implemented as a layer on top of graphql-php.

I will be happy to accept a PR for this, but it can easily be a separate project as well.

[jenschude]

Why not trying jmespath? It allows to create data extract queries in the result. The queries are compiled and cached. This approach is for example used by the AWS SDK.

[smolinari]

👍 For types defined with classes and interfaces.

Scott

[vladar]

New version of the library will also contain Schema Definition Language parser. So in theory another option for type definition could be something like this: https://gist.github.com/vladar/5def770be005cd9218560c80aa133715

@jayS-de I don't quite get what you are saying. Can you provide more context / examples on your idea?

[jenschude]

@vladar I think I just got it wrong :) I was talking about the result set.

You can find example usage here:

https://github.com/jmespath/jmespath.php#jmespathphp

This would prevent expensive mapping of the result to objects, but still it would be convenient to access the results.

[jenschude]

Btw. a collegue of mine just promoted this project here. GraphlQL compatibility acceptance tests. It may be worth to consider to be used and be more in line with the spec. https://github.com/graphql-cats/graphql-cats

[vladar]

@jayS-de That's so great. I've been thinking about such general test suite too. It makes so much sense for projects like GraphQL. I will be happy to integrate it when I have a chance. Thanks for sharing!

[mcg-web]

@vladar can we also think of creating some type of community around graphql-php? because actually it look like the project relies only on you. This could help to be more reactive on updates and bug fixes... This is just a subjection but some colleagues thinks this could be a good idea.

[vladar]

@mcg-web I am all for it. Added you as collaborator for this project as a first step %)

[mcg-web]

@vladar Thanks for adding me as collaborator 👍 ! We'll see how to create a reactive community for this project (like the js implementation)...

[smolinari]

I would imagine a "features list" of enhancements would be be a good start. Anyone willing to contribute can mention which feature they'd like to attack. 😄

Scott

[mcg-web]

I'm trying to integrate the new specs (april 2016), I think that we should switch $context and $info to limit BC. So resolver signature we look like this resolver($value, $args, $info, $context) and not resolver($value, $args, $context, $info). This avoid of rewriting every single resolver using $info.

[mcg-web]

Wrong button 😿

[danez]

Is there any news on that? I saw that there is already a branch for it and we are waiting for a fix for #38. Anything that needs to be done? Can I help somehow?

[ivome]

I am currently running the new branch in testing without any major issues (besides the ones discussed in other tickets). Anything we can help to get this released?

I would be against switching $info and $context in the resolver signature:

• We would divert from the JS library which could make other (future) ports of related libraries confusing
• I think you are more likely to need the custom data from the $context than the ResolveInfo. In those cases you can omit the $info argument in the resolver function. I can imagine that's why they changed the signature in the JS library as well.

It only took me a few minutes to refactor my codebase with a simple find and replace...

[mcg-web]

@ivome in a single project like graphql-relay it is easy to manage this BC, but for a project with a huge schema (more than 300 types) with multiple resolver (>500), this BC could push some dev to don't make the move to the next version. That's not an important change if compare to the JS version, i think that when a BC can be avoided, then it should be... this is an open source project so we not the only one to use it...

[danez]

Im also in favor of what @ivome said. Even if it is a large project it shouldn't be extremely complex to replace all resolvers. And if you make a new semver major version of graphql-php, it is clear that there are breaking changes.

[smolinari]

And this project isn't even 1.0.0 yet or beta for that matter. So, anyone using it should expect things to break or be broken.

Scott

[mcg-web]

@danez @smolinari That's the only major change between these two versions for dev. After yes we a not version 1.0 but we'll reach it by adding BC when we can easily avoid them?

[smolinari]

@mcg-web - I am generally against holding back progress in the name of BC, especially when the change is worthwhile, which I feel this one is.

Scott

[ivome]

@mcg-web I also have a project with hundreds of resolver functions. I replaced array $args, ResolveInfo $info) with array $args, $context, ResolveInfo $info) on the whole codebase and I was done. So if you have consistent naming of function arguments, this shouldn't be an issue. Also, you will see immediately which functions you missed as soon as you execute the resolvers, since PHP will throw an error.

I feel like we might end up doing a lot more typing further down the road when we switch those arguments, because we always have to implement ResolveInfo argument in each function when we want to access $context, even if we don't need it.

So I'd vote for leaving the interface as it is and releasing a new major version, so devs know that there are breaking changes.
Besides that one, we also have other breaking changes:

• Constructor of Schema
• Signature of GraphQL::execute
• All types that have interfaces need to be passed to schema, see Interface lazy loading, field definitions as closure not working #38

If we add upgrade instructions I think we should be fine IMHO.

[ivome]

I updated the documentation and added upgrade instructions. What else is missing?

[vladar]

@ivome Thanks a lot for upgrade document. I just published 0.7.0 release which corresponds to april 2016 specs. My apologies for the long delay with this. But feel free to post pull requests - things will go much faster this way %)