API Add Directive types

marwan38 commented 4 months ago

This PR is working as intended for my use case and can be merged in. However, I have a note for the maintainers here to make the decision on. I'm currently using this as a patch in our codebase on version 4.13 of this module. I'm hoping that it can be merged in by the time I'm done with our codebase migration and can then bump up to v5 of this module.

SchemaTransciber does not introspect the directives. Therefore users relying on the schema transcriber won't be able to get the full schema including the directives. I can't make the decision on behalf of the framework on how much you want added within the introspection query. I personally don't use it, we generate the schema directly using @graphql-codegen and that introspects what we need. An example of a more comprehensive introspection query can be found here and what the directives field may contain here. That introspect query also adds the complexity of configuration, so I'm not really sure what you guys would want here. As I said, this feature as-is is enough for me at the moment.

Description

Directives are part of the GraphQL specification and this PR adds the Directive type for developers to create custom ones. Ideally, this PR would be followed up (or done alongside) with more integration of directives. One use case i would like to see is adding deprecationReason to to types.

types:
  myType:
    fields: {}
    deprecationReason: Use goodType instead

or if there is any reason to add directives to a field (not sure if this is a valid use case)

types:
  myType:
    fields: {}
    directives:
      deprecated:
        deprecationReason: Use goodType instead

Manual testing steps

A new type is added directives in the graphql schema yaml file.

---
directives:
  myDirective:
    description: 'Optional description'
    args:
      if:
        type: Boolean
        defaultValue: true
    locations: [QUERY]

Viewing the the graphql schema (e.g. using /dev/graphql/ide) should now contain an entry like so:

# Optional description
directive @myDirective(if: Boolean = true) on QUERY

The configuration of the Directive type can be found here https://webonyx.github.io/graphql-php/type-definitions/directives/#custom-directives.

The directive can then be resolved within the resolver like so:

/** @var ResolveInfo $info */
$allDirectives = $info->operation?->directives?->getIterator() ?? [];
foreach ($allDirectives as $directive) {
  var_dump($directive->name->value);
}

Issues

578

Pull request checklist

[x] The target branch is correct
- See picking the right version
[x] All commits are relevant to the purpose of the PR (e.g. no debug statements, unrelated refactoring, or arbitrary linting)
- Small amounts of additional linting are usually okay, but if it makes it hard to concentrate on the relevant changes, ask for the unrelated changes to be reverted, and submitted as a separate PR.
[x] The commit messages follow our commit message guidelines
[x] The PR follows our contribution guidelines
[x] Code changes follow our coding conventions
[x] This change is covered with tests (or tests aren't necessary for this change)
[ ] Any relevant User Help/Developer documentation is updated; for impactful changes, information is added to the changelog for the intended release
[ ] CI is green

GuySartorelli commented 3 months ago

@marwan38 This PR hasn't been updated for a while. Are you still keen for this feature to be included? If so please address the comments above. If not, I will close this PR.

marwan38 commented 3 months ago

@GuySartorelli Yeah, still want it included. I've just not dedicated any time to it just yet. If you prefer to close this PR and bring up a new one when I'm ready; that's fine.

There's a few bad commits in here for various different reasons (my own formatters, debug code, etc..).

GuySartorelli commented 1 month ago

Sweet. Yes, please feel free to open a new PR when you have a chance to work on this - but I'll close it for now.

silverstripe / silverstripe-graphql