webonyx / graphql-php

PHP implementation of the GraphQL specification based on the reference implementation in JavaScript
https://webonyx.github.io/graphql-php
MIT License
4.64k stars 566 forks source link

Add directives to a Type or Field #588

Closed davidbarratt closed 2 years ago

davidbarratt commented 4 years ago

It seems like directives can be added to a Type or a Field if using the Type Language. I need to be able to add directives via the $config options as part of the constructor of a type or a field, but it doesn't look like this is possible?

davidbarratt commented 4 years ago

It looks like if you add a deprecationReason to a field, then you get the @deprecated Directive. But it doesn't look like you can arbitrarily set directives.

I think it might be more useful if deprecaitonReason was removed and instead you could set any directive (that can be set on a schema?).

shmax commented 4 years ago

Aren't directives a construct in the query itself, and not the schema definition?

It seems like directives can be added to a Type or a Field if using the Type Language.

Can you provide a sample of what that looks like?

davidbarratt commented 4 years ago

I looked into graphql/graphql-js and this seems to be a problem there too... so probably not an issue for this repo.

The GraphQL spec doesn't make a distiction between "Query" directives (i.e. directives you use when making a query) and "Schema" directives (i.e. directives you use when constructing a schema).

For instance @include is a Query directive but @deprecated is a Schema directive.

Custom directives are supported in this library, but only for use as Query directives, not as Schema directives (although, they are supported as part of the Type Language). So if I create a Schema directive like @key that can't be "used" on any of the types because there is no way to apply it.

The only schema directive you can apply is @deprecated by supplying a deprecatedReason in the config of a type.

vladar commented 4 years ago

So if I create a Schema directive like @key that can't be "used" on any of the types because there is no way to apply it.

If you create your schema via SDL then directives should be accessible on the type via related AST node:

https://github.com/webonyx/graphql-php/blob/ea011cfa0e05fcd14228036fd575adff229646e5/src/Type/Definition/ObjectType.php#L57-L63

davidbarratt commented 4 years ago

@vladar right, I understand, I'm saying that's the only way to do it, you can't add them as part of the config when constructing the object.

vladar commented 4 years ago

Can you please share what is your use case? What are you trying to achieve with directives in the config?

davidbarratt commented 4 years ago

I'm writting a GraphQL extension for MediaWiki and this uses PHP objects so that the schema can be passed to other extensions for modification.

I'm also looking to implement Apollo Federation which relies on schema directives.

The work-around I came up with is extend ObjecType with custom config and ignore the directives. It's a non-standard approach, but I think it should do the trick.

vladar commented 4 years ago

Reference implementation has a concept of extensions (via https://github.com/graphql/graphql-js/issues/1527). Feel free to open a PR with a port of this graphql-js PR: https://github.com/graphql/graphql-js/pull/2097

terion-name commented 4 years ago

@vladar primary usecases are server-side directives, such as for lighthouse or apollo federation and others. E.g. schema can be generated programatically, printed and then passed to lighthouse, apollo, etc. It is really a problem and design flaw that directives can't be added programmatically

dolfje commented 3 years ago

I don't know if it is exactly the same, But the following schema

directive @field on FIELD  
type Query  {
    data: String @field
}

With the following parsing of that schema (Now the code is linearly, but normally the $ast is cached, so the parsing should not be done multiple times)

$document = Parser::parse($contents);
$ast = AST::toArray($document), true);
$document = AST::fromArray($ast);
$schema = BuildSchema::build($document);

Then I have no knowledge of the directives with a custom resolver. ResolveInfo constains the ObjectType. But it doesn't have the ast and it nowhere contains the directive information.

So I cannot use the directive information inside the resolver.

spawnia commented 2 years ago

Directives are a vehicle of the schema definition language. It is possible to use them as metadata to enhance how parsed types behave, but do not provide any functionality by themselves. Even though I use them heavily in Lighthouse, I do not believe it is useful to add them programmatically, instead some sort of structured metadata as proposed in https://github.com/webonyx/graphql-php/issues/588#issuecomment-559224026 could be implemented.

simPod commented 2 years ago

https://www.apollographql.com/docs/apollo-server/schema/directives/ specifies there can be directives in the schema. Seems valid I'd say.

spawnia commented 2 years ago

@simPod to clarify, it is possible to add directives through the schema definition language, both built-in and custom ones:

type Foo @customObjectDirective {
  bar: ID @deprecated
}

This is parsed just fine, and the resulting AST nodes allow programmatic access to the defined directives. This is used heavily in some implementations that base their schema on SDL, such as https://github.com/nuwave/lighthouse.

What also works is exposing certain metadata (currently only deprecation) through directives in the printed schema output.

What does not work, and as I argued above is not the right approach, is attaching directives to types defined without the usage of the schema definition language. I have not found any other GraphQL implementation that does it, probably because it does not make much sense.

simPod commented 2 years ago

I use builders that produce array definitions (https://webonyx.github.io/graphql-php/schema-definition/). I'd very much like to attach custom directives to the schema and migrating to SDL is not an option, it cannot be used in my case.

I think it should be possible to define any schema that complies with spec. Whether it makes sense or not is the question for userland. IMO this library should not be opinionated in a way to tell users what gql features they should or should not use.

spawnia commented 2 years ago

I'd very much like to attach custom directives to the schema

Why? Schema directives do not have inherent functionality or externally observable behaviour. A schema defined in array form can have all the functionality of a schema defined in SDL.

simPod commented 2 years ago

Why

Because it's better to add certain directives to schema object rather than text descriptions. I need to communicate certain intentions to schema consumers.

The same way we detect the field is deprecated not by parsing some arbitrary description but reading standard @deprecated directive.

A schema defined in array form can have all the functionality of a schema defined in SDL.

I have no tool to generate SDL from array definitions. Or am I missing something?

spawnia commented 2 years ago

I need to communicate certain intentions to schema consumers.

As of now, simply having directives attached to types does not communicate anything. They are neither available in introspection, nor are they printed in the schema (see https://github.com/webonyx/graphql-php/pull/996).

There are related initiatives to expose metadata through introspection, but it is a long way from being standardized: https://github.com/graphql/graphql-spec/issues/300

I have no tool to generate SDL from array definitions. Or am I missing something?

That is correct, and I don't think such a tool would be useful or necessary.

This issue describes part of a potential solution to an underspecified problem. Implementing it as described and by itself will not achieve anything. We need to think about the larger problem at hand, which seems to be attaching metadata to the type system, without shoeboxing our thinking into using directives for it. Directives are a tool to attach metadata to SDL and may be interpreted by the schema, but they are not part of the schema itself.

LastDragon-ru commented 2 years ago

As of now, simply having directives attached to types does not communicate anything.

Actually, it will allow significanlty simplify SchemaPrinter and AST transformations for @searchBy/@sortBy directives (right now it should support both AST Nodes and Types, I would preffer use only one of them).

Moreover, custom directives looks totally useless - I do not see any way how they can be used.

simPod commented 2 years ago

They are neither available in introspection, nor are the printed in the schema

This is a good point. Without that it's useless for me.

smyrick commented 1 year ago

Hey @spawnia, I am a Solutions Architect at @apollographql and we are running into issues with some users of this library who are trying to use a code-first approach and publish their Federated schemas to the GraphOS Schema Registry.

Apollo Federation operates by using schema directives (not query directives) purely as metadata to help us merge multiple subgraph definitions together and know which subgraphs can resolve which fields. These directives have no runtime code. A core use case is applying the @key directive to an object type.

type User @key(fields: "id") {
  id: String
  email: String
}

Unfortunately, as already stated, the schema printer does not support printing directives, which does seem to make custom schema directives definitions not important as clients won't see them in the schema anyway. However, Federation needs these definitions. The apollo-federation-php gets around this by creating custom class extensions, like EntityObjectType, that you define instead of simply adding them to the existing ObjectType.

Once we have a Federated schema, the library is able to get the full schema with directives because Federation also adds a special Query._service field which returns the full SDL as a string. In a SDL-first schema, you wouldn't need this but with code-first you do in order to publish the full schema. This is why I think we separately should update the schema printer to allow a toggle to opt-in to include directives, but that is not the main concern.


Our main concern, and why I think this issue should be reopened, is that you can not add other metadata directives to your schema using only this library, like @tag, if you are using the code-first approach but you can if you are using the SDL-first approach. In my mind code-first and SDL-first should have the same capabilities, even if the building pattern is different.

A proposed solution is adding a new directives option to all type definitions which would actually match the reference graphql-js implementation

note I have no php experience so apologies if this code is not correct

$tagDirective = new Directive([
    'name' => 'tag',
    'locations' => [
        DirectiveLocation::FIELD, DirectiveLocation::OBJECT,
    ],
    'args' => [
        'name' => Type::string()
    ]
]);

$userType = new ObjectType([
    'name' => 'User',
    'directives' => [ $tagDirective('name': "external") ]
    'fields' => [
        'id' => Type::string(),
        'email' => [
          'type' => Type::string(),
          'directives' => [ $tagDirective('name': "external") ]
        ]
    ]
]);
ruudk commented 1 year ago

Can this be re-opened? I also would like to attach directives to my types, and bake them into the schema.