-
-
Notifications
You must be signed in to change notification settings - Fork 427
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
Schema (type) level directive support with optional support of federation composeDirective #1308
Schema (type) level directive support with optional support of federation composeDirective #1308
Conversation
7c132f4
to
807c458
Compare
Thanks a lot for the PR 🙂I have some suggestions for changes:
Some code snippetstypes defined in
|
I was thinking about auto registration on directive apply level, the only issue I see is that if we have two directives with the same name (non pub functions or directive itself renamed) and trying to register those will cause some confusion (we probably will need completely override previous registration), maybe we need additional check that directives are "equivalent" in the sense that they have the name and param names/types identical - otherwise throw compilation error. |
Maybe we can detect conflicts at runtime like this. async-graphql/src/registry/mod.rs Line 810 in 2047740
|
3e0bc08
to
4a6345c
Compare
|
…e string instead of bool
4a6345c
to
0fc1603
Compare
0fc1603
to
e12b87a
Compare
So for the last item - location check at compile time. This is the compilation error if you try to apply directive on Object level but its declared for field only: #[TypeDirective(
location = "fielddefinition",
composable = "https://custom.spec.dev/extension/v1.0"
)]
fn testDirective(scope: String, input: u32, opt: Option<u64>) {} and then you try to apply it: #[derive(SimpleObject)]
#[graphql(
directive = testDirective::apply("simple object type".to_string(), 1, Some(3))
)]
struct SimpleValue {
#[graphql(
directive = testDirective::apply("field and param with \" symbol".to_string(), 2, Some(3))
)]
some_data: String,
} it fails with the compilation error:
|
The only thing what bothers me is specification of locations on TypeDirective itself. I took definition of TypeDirectiveLocation from original DirectiveLocation and it has darling specification to expect strings to be in |
Good work, thank you! 👏 |
Ghrr probably another PR will follow to fix the checks :) |
I'm working on this 🙂 |
Currently
async_graphql
supports only "execution" type directives (which can be specified on query fields) and provided function is called during query execution.There is another type of directives supported by grapqhl spec - type system directives, which simply acts like type level decorators http://spec.graphql.org/June2018/#TypeSystemDirectiveLocation
Although those type of directives doesn't (and shouldn't) alter the behaviour of library itself, but its a powerful tool to communicate additional metadata to schema introspectors and with @composeDirective support, it can be used in various ways to communicate meta type info to supergraph routers etc.
The alternative way would be reuse already available @tag directives, limitation being that the value of tag is just a string which lacks further syntax check etc.
This PR proposes new proc macro
TypeDirective
which is similar to currently implementedDirective
and is also is applied to rust functions. The difference is that function is used to generate arguments for directive definition, and later can be called on graphql construct to generate actual directive "invocation". Example:will result in following defintion on generated SDL schema:
and can be invoked on object or field level like this:
which will result in the following SDL fragment:
This allows to a nice way to extend grapqhl schema with additional metadata information.
Another powerful feature which is defined by apollo federation v2.1 is @composeDirective, which essentially says that any directives which need to be preserved on supergraph composition need to be declared with @composeDirective.
In order to achieve that two additional things need to be done on TypeDirective level and on SDL export options:
and on TypeDirective additional property is requred:
composable property needs to contain strict URL with at least two path segments (last is always semver style version like v1.0) or apollo federation supergraph checks will fail.
The addtions above will result in additional schema fragment, which basically exposes directive as composable and is preserved in supergraph construction:
Different type directives can contain different compose extension urls - they will be grouped accordingly.
Limitations:
Currently this PR supporst only OBJECT and FIELD_DEFINITION locations, to keep the scope a bit smaller, but support for other grapqhl constructs can be added easily in the future.
Questions:
and invocation can be
But the struct can be named as
somethingDirective
and registered asschema.type_directive(somethingDirective)
. For me this might feel a little bit more natural, but generates two objects instead of one (currently apply function is a method inside struct).