Standardized way to organize custom field resolver

[pyrou]

What problem does this feature proposal attempt to solve?

Propose, and document a standardized way to organize custom field resolver in a Laravel project.

Which possible solutions should be considered?

Idea is about making the resolver attribute optional in the @field directive.



When resolver attribute not provided on @field directive, resolver classname/method would be computed from following :


- The configured namespace in config(‘lighthouse.namespaces.types’)


 - The parent type name


- The field name




Example



type User {
  gravatar: String! @field
}


becomes equivalent as 
:



type User {
  gravatar: String! @field(resolver: "App\\GraphQL\\Types\\UserType@gravatar")
}



by convention.

Implementation detail







Possibly some user would prefer to have something even more fine-grained like, why not a dedicated class per resolver, let's say App\\GraphQL\\Resolvers\\User\\GravatarResolver@__invoke.









To still permit this, the « default resolver name resolver » (yeah.. a proper name has to be found) would be written in a way it could be customized/overridden in project-land (like an app()->bind in a service provider). Then up to the developer to write his own logic of default resolver name resolution.










Backward compatibility / Breaking changes









Currently, resolver attribute is required on @field directive. So proposed file organization adoption is optional, and 100% backward compatible with existing lighthouse integration.










I would be happy to contrib to this feature.

[spawnia]

I am a big fan of having one class per resolver, it makes dependency injection very elegant.

[pyrou]

I am big fan too, that's why I already proposed a way to do it so.

But as reading documentation I thought you (as nuwave company) would be more happy with one class per Type as default. Because of this documented example :

type User {
  created_at: String!
    @field(resolver: "App\\GraphQL\\Types\\UserType@created_at")
}

[k0ka]

I guess we might just change the default resolver and omit @field at all.

Here is how I did on my project:

1. Simple fields are store in objects. However I'm using Doctrine ORM and I can't use object properties. I've changed the default resolver so it executes getProperty() functions.
2. Complicated fields are handled by one class per resolver.

[pyrou]

Love the idea about default resolver.

But, I also had in my drafts a proposition to automatically fallback on snake case when attribute doesn't exists in GraphQL schema case. Because in real life situation, my GraphQL is 100% camelCase, and Laravel still "forces" me to have snake_case in database :(

Then instead of doing this :

type User {
   id: ID!
   firstName: String @rename(attribute: "first_name")
   lastName: String @rename(attribute: "last_name")
   avatarUrl: String @rename(attribute: "avatar_url")
}

I would love to simply write :

type User {
   id: ID!
   firstName: String
   lastName: String
   avatarUrl: String
}

Maybe that all together might become a bit too much "magic" ?

[k0ka]

The first thing to do
We should change the way the default resolver is set. It's quite fuzzy at the moment. It would be much easier to just set it in the config.

The more complicated continuation
We might create a "middlewared" default resolver. The request goes through a chain of special middlewares until any of them returned a value. Middlewares might be like:

1. return array value
2. return object property
3. if the returned value is a function, run it rather than return.
4. convert to snakecase
5. try to run getProperty method
6. check a method in another object

These middlewares might be turned on/off in the config. So each user can easily configure the default resolver if they don't want to write their own.

The current default resolver is defined in https://github.com/webonyx/graphql-php/blob/master/src/Executor/Executor.php#L181
Middlewares 1,2 and 3 mentioned above can identically replace it.

[spawnia]

I have entertained the idea of changing the default resolver for a while, let's track that in a separate issue: #2191

I have also realized this issue is a duplicate of #1711.