Skip to content

Expose FieldDefinition in ResolveInfo #522

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Sep 12, 2019
Merged

Expose FieldDefinition in ResolveInfo #522

merged 5 commits into from
Sep 12, 2019

Conversation

spawnia
Copy link
Collaborator

@spawnia spawnia commented Jul 24, 2019

I require access to FieldDefinition->args in a resolver. As i see it, there is no straightforward way to access it.

This PR simply adds the FieldDefinition object to the ResolveInfo, while keeping backwards compatibility.

@yura3d
Copy link
Contributor

yura3d commented Jul 25, 2019

Access to field types and their arguments, that use in a query, already exists through QueryPlan class, which is unfortunately not documented yet. You can see PRs #65 and #436 to get more info.

@vladar
Copy link
Member

vladar commented Aug 2, 2019

Can you explain your use case a bit?

P.S. Technically it is a breaking change because of ResolveInfo constructor signature change.

@spawnia
Copy link
Collaborator Author

spawnia commented Aug 6, 2019

We are using complex nested mutations in Lighthouse, see https://lighthouse-php.com/master/eloquent/nested-mutations.html for some examples of how they are used.

I am looking to implement a generalized solution for composing nested mutations.

Take for example a query like this:

mutation {
  createPost(input: {
    title: "My new Post"
    author: {
      connect: 123
    }
  }){
    id
    author {
      name
    }
  }
}

The title argument would be handled by a different resolver then the author argument. To split up the input, i require access to the underlying schema, where i attach resolvers to the input definitions.

@spawnia
Copy link
Collaborator Author

spawnia commented Aug 6, 2019

P.S. Technically it is a breaking change because of ResolveInfo constructor signature change.

Technically true, although i found the constructor is only used twice - within the executors. It is not marked with @api

@spawnia
Copy link
Collaborator Author

spawnia commented Sep 9, 2019

Is there something i can do to move this PR forward?

@vladar
Copy link
Member

vladar commented Sep 10, 2019

Thanks for reminding. I just want to make sure we are on the same page.

Here is what confuses me a bit: InputObjectType fields do not have resolvers. Only regular ObjectType fields do have them.

So I guess you are interested in the field definition of the createPost field. But then the following could work:

$fieldDef = $info->parentType->getField($info->fieldName);

Or am I missing something?

P.S. Also curious about your opinion on #513 (comment) (unrelated to this PR)

@spawnia
Copy link
Collaborator Author

spawnia commented Sep 10, 2019

InputObjectType fields do not have resolvers.

Actually, i am working on implementing something like that. It is meant to allow composing resolve actions within complex, nested inputs.

Your suggestion might work, but i think it is unnecessarily complicated. The field definition sits there anyways and i can not think of any reason why access to it should be limited. Let's remove that limitation and enable the more straightforward:

$fieldDef = $info->fieldDefinition;

That gives users the freedom to do what they need to do and enables more possible use cases, even if they are pretty fringe ones.

@vladar
Copy link
Member

vladar commented Sep 11, 2019

Can you add an assertion in this test to make sure we don't accidentally revert this later when syncing again with graphql-js (which doesn't expose this data)?

Then I'll merge it

@vladar vladar merged commit 6554423 into webonyx:master Sep 12, 2019
@spawnia spawnia deleted the typed-arguments branch September 15, 2019 08:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants