Skip to content
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

Field resolver validation reports a false positive for array and object types #1341

Open
Kingdutch opened this issue Apr 26, 2023 · 0 comments

Comments

@Kingdutch
Copy link
Contributor

Problem

Given the following schema

schema {
  query: Query
}

type Query {
  someField: TypedField
}

type TypedField {
  one: String!
  two: String!
}

Where the schema is mapped using

    $registry->addFieldResolver('Query', 'someField'
      $builder->fromValue(['one' => 1, 'two' => 2])
    );

The TypedField will automatically be resolved by the GraphQL library's Executor::defaultFieldResolver. However, the validator is not aware of this and will report missing field resolvers for the sub-types, even though the schema will work just fine.

This can lead to bug reports which may put people on the wrong trail such as https://www.drupal.org/project/graphql_address/issues/3354352

Proposed resolution

This is a tricky problem because the schema can not be validated at inspection time, but really only by inspecting the resolved value. The current mapping validation mostly checks "if there's a field resolver" and doesn't capture any information about the resolvers, so we also can't utilise something like reflection to make this process smarter and look at the @return type of Resolver::resolve method (this would require also inspecting the output of the ResolverBuilder's chain).

This would be a good candidate for the providing module to mark the type TypedField as an ignored type. The Validator already has an $ignored_types optional argument which would need to be provided in the ValidationController.

The downside is that if someone uses a different resolver for the someField then it may no longer return a type which actually has the one and two fields. However this will not be caught by the validator because it's ignored.

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

No branches or pull requests

1 participant