Directives support with @Directive decorator

[j]

I was toying with federation support and going about it the manual way.

Here's the issue I also posted on apollo-server apollographql/apollo-server#2965

This doesn't work but if you look at the test, it shows the federated schema.

I sort of am clueless on how to support directives so it'd be cool if someone could chime in on this.

I was mocking the test case from the federation-demo's reviews service. Ideally we can have a fully functional replica.

/cc @JacksonKearl

[JacksonKearl]

Hey I added some stuff to hook together our buildFederatedSchema and your directives and it seems to be coming together fairly well: I can view the service in GraphQL Playground and it contains all the required directives/entities/etc.

The next blocker is that defining

  @Query(returns => Recipe)
  async __resolveReference(reference: Recipe) {
    return this.items.find(item => item.title === reference.title);
  }

causes an error here https://github.com/JacksonKearl/type-graphql/blob/federation/src/schema/schema-generator.ts#L102

My local branch: j/type-graphql@federation...JacksonKearl:federation

[j]

@JacksonKearl awesome! I’m on vacation so I’ll look into it later. As for the resolver function, I might make a decorator for that. I also think there’s a way to have the function be whatever name you want and define the graphql name in the options, but I’m not too sure.

[j]

Also I believe you need to use ‘@FieldResolver()’ instead of ‘@query()’. This would go in the resolver class and have to set up the Recipe as the ‘@ObjectType(() => Recipe)’ (I’m typing on a phone and phone coding is hard)

[MichalLytek]

@j looks great!
But can we make it more elastic and support other directives too? Then the federation directives would be just build on top of them, instead of hardcoded.

Also, the decorators should be prefixed with Federation, e.g.: @FederationExtends().

[j]

@19majkel94 yup! I actually wanted to do that but decided to just do direct to prototype. I can clean things up when I get back and also do a fully replicated demo hopefully. If I do the directives route, I could change this to @directive() instead and people can implement federation stuff that way and I’ll just remove the @federation prefix?

Also, notice how @JacksonKearl did the implementation. Is that okay? What’s the best way to get typedefs and resolvers instead of an executable schema? Also, to remove federation as a dependency, should I make a pluggable printSchema option so people can bring their own? In this case, use the printSchema from the federation library?

I can get these working examples next week. Let me know how you’d like them to be implemented :)

[MichalLytek]

If I do the directives route, I could change this to @directive() instead and people can implement federation stuff that way and I’ll just remove the @federation prefix?

No, we can have a generic @Directive() mechanism and then export also a set of custom apollo federation decorators that will call @Directive under the hood.

What’s the best way to get typedefs and resolvers instead of an executable schema?

Current pipeline is metadata -> GraphQLSchema -> derive typeDefs and resolvers. This way we can quite easily support both worlds. We can wrap the require in try-catch, so all that users would have to do is to install the @apollo/federation dependency by themselves.

In the future there will be a plugin system that would solves this kind of tricks and complicated setup (own printSchema impl).

[j]

@19majkel94 great! I’ll work on this. I need to research more on what’s best to use, ‘extensionASTNodes’ or ‘astNode’. ‘extensionASTNodes’ seems like the one that should be used, but would require a chance to federations printSchema function. I haven’t really researched the proper way, since I’m not sure there really is considering there’s no direct support for directives in type constructors.

[j]

@19majkel94 It seems that a lot of directive implementations rely on typeDefs and graphql-tools to create executable schemas with defined directives, however, the route in this PR, federation will only output federation specific stuff, but if someone wants to use other directives, they'll be stripped out because the graphql printSchema strips them away from fields and object types, so unless I'm mistaken, I'm not sure what benefits will come from allowing directives on the schema.

[MichalLytek]

they'll be stripped out because the graphql printSchema strips them away from fields and object types, so unless I'm mistaken, I'm not sure what benefits will come from allowing directives on the schema

As discussed in #77, we have to fork the printSchema helper from graphql-js to print the directives too.

[j]

@19majkel94 Gotcha, in #77, in order to create directives like,

@Directive('@cache(age: "7 days")')

You'll need to parse that to create the proper AST, but those aren't exposed I don't think in the parser. I could parse them manually or just create a "fake" type with directives and just pull the parsed directives out, but it's just being hacky.

The other option is to just do something more like,

@Directive('cache', { age: '7 days' })

In the end of the day, I think the most flexible way is if type-graphql build out schema as a string typeDef like your other repo and people could bring their own buildExecutableSchema from typeDefs and a resolver map. Do you have ideas on the best way to parse the first directive option, or is the second one okay?

[MichalLytek]

The other option is to just do something more like

Yes, if we need to construct AST from the decorator metadata, it should be the API we introduce.

I think the most flexible way is if type-graphql build out schema as a string typeDef

This will break all the features that operates on GraphQLObjectType and others, like query complexity.

We can't do that until the specification will allow for reading directives metadata and exposing them in extensions property of graphql-js runtime objects:
graphql/graphql-js#1343

[j]

@19majkel94 I pushed a more generic @directive using the second option above. I'm doing this before my flight back home, so there's probably things I missed. Let me know what you think.

Notes: I moved the auto-magic "directives" initialization of the schema so that users have to pass that part in themselves (to not be hard coded for federation support), this way you can bring in custom directives too. I think all that's needed then is to build a printer that keeps directives, which I've seen around in various issues. My previous federation tests are passing.

[ldiego08]

@j thank you so much for taking care of this feature! I'm on the edge of my seat seeing how this PR unfolds. Just one thing, I know @19majkel94 requested these decorators to be prefixed with Federation but given how Apollo Federation is the collective name of something that's becoming part of the Apollo core, it seems a bit unnecessary and might clutter the decorated classes. In SDL, the Federation directives are named key, extends, etc., and for me, it makes sense we name them in code-first approach accordingly unless there are naming collision concerns. Any particular reasons why the decorators need to be prefixed?

Here's how this would look like to me in code without the prefix and seems good enough:

import { ObjectType Key, Extends } from 'typegraphql';

@Extends()
@ObjectType()
class User {
   @Key()
   id: number; 
}

Federation directives could go into its own typegraphql/federation folder if there's a definitive need to have them apart. I know there are plans for adding support for TypeORM, Prisma, and other packages, so it might be a good way forward or else decorators for all those will need prefixing.

[MichalLytek]

Any particular reasons why the decorators need to be prefixed?

Because to support #228 we might introduce the @Extends(base => User) decorator that will conflict with the apollo federation one.

or else decorators for all those will need prefixing

That's correct, there will be @PrimsaModel(), etc.

[JacksonKearl]

In terms of conflicts, it would be possible to import as:

import { ObjectType, Key, Extends } from 'typegraphql';
import { Key as FederationKey, Extends as FederationExtends } from 'typegrapql/federation'
import { Model as PrismaModel } from 'typegraphql/prisma'

@Extends()
@ObjectType()
@FederationExtends()
@FederationKey({fields: "id"})
class User {
   @Key()
   id: number; 
}

But up to you if that's something you'd want.

[j]

@JacksonKearl @ldiego08 You can also do the inverse of the "as" . It's sort of why I namespaced it as

// decorators/index.ts
export { Federation } from './federation';

// my resolver
import { Federation } from './federation';

const { Key, Extends, ... } = Federation;

But I'm not opposed to prefixing with Federation*. Another note, since they are simply directives, the Federation* exports aren't really even needed since you can just do:

@ObjectType()
@Directive('key', { fields: "id" })
class UserResolver {
  // ...
}

Also, this isn't supported in this PR:

class User {
   @FederationKey()
   id: number; 
}

I can probably add it for easy use-cases, but if you need nested fields, I'd have to do more work to resolve child types.

Also, if this repo was a mono-repo, I'd say +1 for the non-prefixed version and use @typegraphql/federation. That'd be swanky. /cc @19majkel94

I'll try to touch more things up today.

[j]

I'm still working on this, but decided to see if the apollo team would take a PR to support passing in already constructed schemas to make this implementation a lot cleaner.

apollographql/apollo-server#3013

This would make federation support really simple and not have to even be a dependency. I believe for other directives, you can use attachDirectiveResolvers from graphql-tools and don't really need to add a custom printer. I'll prototype this too.

/cc @JacksonKearl @19majkel94 @ldiego08

[j]

Update: I’m able to replicate the entire federation demo using this PR with my PR for @apollo/federation.

I’ll push the examples after cleaning things up. One thing that feels awkward is defining externals on fields in a federated schema. I’ve been so used to those properties in the class to need to be filled, but noting them as readonly with undefined types makes me feel better. Another weird thing was the reference resolvers, the Apollo code samples on their demo use the world “object” for the root on all field resolvers which makes me feel that they are the same thing, but root is actually the type name with federation key. I’ll make it clear in the examples.

[j]

I pushed examples using the PR to @apollo/federation (apollographql/apollo-server#3013). Check it out. CI failed due to the upstream @apollo/federation change. If that can get merged, then federation support is ready to go, otherwise we'll have to go the route of converting to SDL -> schema -> SDL -> schema (which feels nasty). I feel like this is as elegant as it can get until directives are baked in which should be in the next major graphql-js version. They may add a GraphQLSDLDirective type for printSchema.

9bbed68

Next on the list is that I'm going to play around with 3rd party directives and see if I can get those to work natively with this change.

/cc @JacksonKearl @19majkel94 @ldiego08

[j]

PS, not gonna lie, this feels reaaaaaal good. 🔥

The only "gotcha" I found is that __resolveReference isn't a true GraphQL field resolver. So when you use @Root(), it's not the actual object type, but instead is representation reference sent by the gateway (@apollo/federation/src/types.ts#L98). You're still able to hydrate it into the class using @Root(), but it will only have the fields from the gateway. I was using @Root() reference: Partial<MyType> instead of actually mapping it to the class since it's not really the class. I'm not sure how I feel about this, but the examples work.

@apollo/federation internally takes this:

const resolvers = {
  Product: {
    __resolveReference: (object, { fetchProduct }) => {
      return fetchProduct(object.id);
    }
  }
};

and sets it as a method on the "Product" object type:

console.log(ProductType.resolveReference === resolvers.Product.__resolveReference); // true

I always thought that __resolveReference just passes the keys for resolving that item, but if you have another fieldResolver that requires other external fields, those external fields will be passed to __resolveReference as well. So the object argument above can change depending on the field being requested.

[j]

I came into an issue regarding my previous comment. You can't get @Ctx() into __resolveReference. It looks like @Ctx() is the info object.

Otherwise, @19majkel94 what are your thoughts on decorating that "resolver" with a custom signature. i.e.

__resolveReference(reference, context, info)

apollo-tools takes anything prefixed with __ and sets it to the object type. Should we do the same or introduce another decorator to set object type properties/methods and let the functions be called as is without any middleware / etc? The "apollo way" may not be a bad move, but users will need to know they can't use argument decorators.

@ObjectType(() => User)
@Directive('key', { fields: "id" })
class UserResolver {
  @ObjectTypeMethod() // adds`User.resolveReference()` to `User` type
  async resolveReference(reference: UserReference, { fetchUser }, info): Promise<User> {
    // ...
  }
}

Then we can create a shortcut decorator for @ResolveReference if we wanted to.

[christopher-avila]

Great news!!! Congrats!

[MichalLytek]

Thank you very much @j for the work and @bbenoist for the help! ❤️

[pierreis]

Looks awesome, thanks a lot for the outstanding work! So if I understand correctly, I can only use this for federation with the buildFederatedSchema branch of apollo-server?

[MichalLytek]

Published as v0.18.0-beta.3 - npm i type-graphql@beta
https://typegraphql.ml/docs/next/directives.html

[MichalLytek]

I can only use this for federation with the buildFederatedSchema branch

@pierreis
Technically you can use createResolversMap from type-graphql and printSchema from @apollo/federation to use buildFederatedSchema with { typeDefs, resolvers }.

I've recreated the apollo federation example from this PR using this trick but I get this error:

GraphQLSchemaValidationError: [reviews] User.id -> is 
marked as @external but is not used by a @requires, @key, or @provides directive.

[reviews] User.username -> is marked as @external but 
is not used by a @requires, @key, or @provides directive.

[reviews] Product.upc -> is marked as @external but is not used by a @requires, @key, or @provides directive.

[inventory] Product.upc -> is marked as @external but 
is not used by a @requires, @key, or @provides directive.

[inventory] Product.weight -> is marked as @external but is not used by a @requires, @key, or @provides directive.

[inventory] Product.price -> is marked as @external but is not used by a @requires, @key, or @provides directive.

@j can you look at the branch apollo-federation?
https://github.com/MichalLytek/type-graphql/tree/apollo-federation

[MichalLytek]

Looks like the implementation is buggy:

@Directive(`@key(fields: "upc")`)

works but

@Directive("key", { fields: "upc" })

not - parseValue doesn't handle properly different types of args like boolean, string, null, etc. Also we have no support for directives on field resolvers, so the example doesn't want to work.

That's why I don't like rush and "push push push to production we need that feature". Thanks god I've published it as a beta, not as stable release...

[voslartomas]

At least right now it will be tested by others and will be much faster to fix these errors, don't worry everyone will be happy to test and give feedback and help with fixes. It's been 4 months, this PR was opened, I would not call it rush...

[MichalLytek]

It's been 4 months, this PR was opened, I would not call it rush...

This PR initially was about apollo federation and we still doesn't have a proper support from apollo-server. So we were iterating to remove federation hacks and introduce only directives support that could be use for custom federation integration.

[MichalLytek]

@pierreis @voslartomas @christopher-avila
I've fixed the field resolvers and added an example of apollo-federation (a modified federation demo from @j PR):
https://github.com/MichalLytek/type-graphql/tree/20c81f4f7ee82779595002d25784fce3a8ff8b9b/examples/apollo-federation

[christopher-avila]

As far as I'm testing it, it works like a charm! I'm migrating our microservices on the typescript stack to Type-graphql, but the first integration is working perfectly. Thank you so much @MichalLytek @j and other people who worked on this.

[mkuczera]

Hi, i´m right now confused. Does the apollo-federation implementation also enable the usage of custom directives like @intl?
https://www.apollographql.com/docs/graphql-tools/schema-directives/

Sorry for disturbing, just subscribed to the PR because of the Directive support and ran into confusion regarding apollo-federation

[MichalLytek]

Not with current workaround - we use their printSchema util that strips off other, non-federation directives.

Please ping on apollographql/apollo-server#3013 - we need this to have a real federation support without workarounds that breaks features like directives or query-complexity.

[fullofcaffeine]

Hi!

Just found out about GraphQL directives and a Google search led me to this issue. Great work on implementing it! I'm a bit confused on the benefits of using it for, say, user authentication + authorization vs the regular way through middlewares.

Also, the docs here: https://typegraphql.ml/docs/next/directives.html are helpful but the last part showing the runtime implementation seems incomplete. It'd be great to have an example of authorization at least, on how to check the actual data for the directive and doing something based off it.

Thanks!

[MichalLytek]

@fullofcaffeine

I'm a bit confused on the benefits of using it for, say, user authentication + authorization vs the regular way through middlewares.

If you implement the logic, you should just use middlewares 😉

Directives are for 3rd party integration, like Apollo Federation or Apollo Cache Control. So if you used e.g. some auth directives and while migrating to TypeGraphQL you don't want to reimplement that logic, you can just place the directive decorator and use the same approach as earlier.

[fmquaglia]

Hello, thanks a lot for implementing this. <3
Is there an estimated date to release this?

[MichalLytek]

@fmquaglia
#369 (comment)

[fmquaglia]

@MichalLytek Thanks. Sorry I wasn't clear.
I meant "release out of beta"

[MichalLytek]

@fmquaglia Soon