type-graphql icon indicating copy to clipboard operation
type-graphql copied to clipboard

Schema directives support

Open MichalLytek opened this issue 7 years ago β€’ 32 comments

Partially solved by https://github.com/MichalLytek/type-graphql/pull/369

Currently blocked by https://github.com/graphql/graphql-js/issues/1343


Ability to define any arbitrary graphql directive to be passed to the compiled schema:

@ObjectType({ directives: ['@cacheControl(maxAge: 240)'] })
class SampleObject {
  // ...
}

Which should be equivalent to:

type SampleObject @cacheControl(maxAge: 240) {
  # ...
}

It might be also implemented as a separate decorator that would make possible to create custom decorators like @CacheControl({ maxAge: 240 }):

@Directive('@cacheControl(maxAge: 240)')
@ObjectType()
class SampleObject {
  // ...
}

MichalLytek avatar May 12 '18 13:05 MichalLytek

@19majkel94 Why this is blocked? at face seems less complicated.

JasCodes avatar Jun 29 '18 18:06 JasCodes

Go to the link in first post and read the discussion :wink: If you can implement this - feel free to make a PR πŸ˜ƒ

MichalLytek avatar Jun 29 '18 20:06 MichalLytek

This sadly prevents anybody from being able to use type-graphql that is using graphcool/prisma as a backend. We would love to use it but sadly without being able to generate the schema necessary by the downstream tooling its a non starter :(

ntziolis avatar Dec 23 '18 10:12 ntziolis

@ntziolis AFAIK, Prisma is the glue between your GraphQL server and your database. You define your model using SDL and you receive a generated client that acts as an ORM in your server app.

So there should be no problem to use Prisma along with your GraphQL API build with TypeGraphQL. Of course, the tighter integration (defining prisma model and your API GraphQL Object Type in one class with decorators) would help in reducing boilerplate, but I think it's not a deal breaker πŸ˜‰

MichalLytek avatar Dec 23 '18 11:12 MichalLytek

And to report an update about the status of the issue - now I think I understand where is the problem with directives in graphql-js. They are purely the SDL thing, so tools written in different languages can parse the schema definition and apply certain behaviors in runtime.

So as with graphql-js we define a runtime JS object, other tools (like Prisma or Apollo Engine) don't have an access to the directives. That's why we have to wait for a new GraphQL specification that will expose the directives info via an introspection query. For now the only workaround is to generate typeDefs and resolversMap instead of executable GraphQLSchema and paste the "directives" from metadata storage into proper lines in SDL string πŸ˜•

MichalLytek avatar Dec 23 '18 11:12 MichalLytek

@19majkel94 I understand the first instinct, but there are various reasons why we feel when solving this issue a lot of other goodness would come from it:

  • prisma has a VERY poor data model migration story, in particular:
    • no versioning of schema during dev time
    • deployment of the same schema causes deployment errors in many cases breaking CI pipelines
    • no support for up/down migrations
  • using partial or changed types in public schema only possible via copy&paste
    • using the same types in your public schema minus a few fields requires copy&pasting the type (and related types) and manually removing the fields in question
    • causes errors when our devs update prisma data model but forget todo the same on the public side (lets be honest this happens a lot)

In order to clean this up and follow DRY here is how we wanted to use type-graphql

  • type-graphql becomes our way of defining both public & prisma schema (including generating the actual SDL files)
  • at the same time be able to generate type safe resolvers for the public schema
  • and use the additional features to standardize on DI etc most of which is handled manually / project by project today with different libs

I understand this might have been the initial intention of type-graphql but I can tell you after the 10th+ graphql project: The need for a framework that is the beginning and end where types originate is VERY high especially when using a graphql datasource underneath (we have tried various beyond prisma).

ntziolis avatar Dec 23 '18 11:12 ntziolis

Hi @19majkel94

I was trying to undestand the problem for long hours πŸ˜„ As far as I found apollo graphql-tools is doing simple job for schema directives. It just finds fields with directives and overrides their resolvers with enhanced version that applies that specific directive.

So as TypeGraphQL stores metadata for first part, the second part can be done the same way.

I don't know what you want to provide for prisma. The datamodel of the prisma is not a graphql thing. it's a customized version so I think it has nothing to do with TypeGraphQL that cares about server - client relation.

Hossein-s avatar May 22 '19 19:05 Hossein-s

@Hossein-s So show me a proof of concept how to apply directive to a field using graphql-js, not GraphQL SDL syntax.

MichalLytek avatar May 22 '19 19:05 MichalLytek

@19majkel94 Great. I will work on it.

Hossein-s avatar May 22 '19 19:05 Hossein-s

@Hossein-s I also took quite a long time to find a possible solution for programatically declare directives for a node, but no luck so far. So I am also keen to see your finding.

If I understand correctly, your statement here is not quite right:

As far as I found apollo graphql-tools is doing simple job for schema directives. It just finds fields with directives and overrides their resolvers with enhanced version that applies that specific directive.

A directive is not a function, it is simply a piece of information that can be attached to a node (field, type, etc), which only contains name and optional args, and the directive itself has no logic at all, that said there is no way (and doesn't make sense) to "apply the directive".

In the case of apollo, you also need to implement SchemaDirectiveVisitor for specific directives.

zhenwenc avatar May 22 '19 21:05 zhenwenc

@zhenwenc When you have SDL it's just piece of information. But when you want an executable schema, then executor applies the logic implemented for that piece of information to the resolver. As you can see in the SchemaDirectiveVisitor samples, every custom directive (and built in directives like deprecated directive) have an implementation, then you decorate fields and objects to call that implementation. (But I couldn't find cacheControl source to find how it works (even couldn't set it up to work with apollo 🚢 )).

Do you know any case that astNode definitions needed by server? I'm new on this topic.

Hossein-s avatar May 23 '19 07:05 Hossein-s

After investigating more, I found implementation of schema directives in apollo and graphql-yoga very similar to TypeGraphQL middlewares that are already available (But you can pass arguments to them).

@19majkel94 I don't know where schema directives sit in TypeGraphQL. 😞

Hossein-s avatar May 23 '19 07:05 Hossein-s

@Hossein-s

When you have SDL it's just piece of information.

The problem (or difficulty) here is not about executing the directive, since apollo already provided a way to handle it (via SchemaDirectiveVisitor), but how to add the directive into the GraphQLSchema without using DSL.

If I understand it correctly, the reason why TypeGraphQL can use Middleware to implement cache control is because you can easily manipulate the cache control store since apollo injected it into the resolve info, which doesn't actually needs to involve directive at all.

zhenwenc avatar May 24 '19 01:05 zhenwenc

@19majkel94 @zhenwenc I've created a PoC to show what I'm talking about. you can find it here.

It does something like SchemaDirectiveVisitor.visitSchemaDirectives described here.

Hossein-s avatar May 24 '19 11:05 Hossein-s

If I understand it correctly, the reason why TypeGraphQL can use Middleware to implement cache control is because you can easily manipulate the cache control store since apollo injected it into the resolve info, which doesn't actually needs to involve directive at all.

@zhenwenc Thanks! Now I undestand where the astNode is used πŸ˜„.

So I think it's possible to patch the resulting schema and add the astNode if there is need πŸ€” I will work on it...

Hossein-s avatar May 24 '19 11:05 Hossein-s

I have updated PoC repository so that it adds astNode to GraphQLField. you can see it here.

So apollo cache control can read directives in this function. But other directives work without astNode.

It's very minimal just to show the posibility of feature.

Hossein-s avatar May 24 '19 12:05 Hossein-s

@Hossein-s Can you check in your PoC if you add directive to astNodes, will it show up in printSchema?

MichalLytek avatar Jun 01 '19 18:06 MichalLytek

@19majkel94 Unfortunately no. printSchema is not considering directives at all :-/ https://github.com/graphql/graphql-js/issues/869

Hossein-s avatar Jun 02 '19 11:06 Hossein-s

@Hossein-s Yeah but we can use the custom one that prints astNodes πŸ˜‰ https://github.com/graphql/graphql-js/issues/869#issuecomment-374351118

MichalLytek avatar Jun 02 '19 11:06 MichalLytek

@19majkel94 Yes, always there's a solutionπŸ˜ƒ I tested that snippet but it didn't work (because of lacking astNodes for Query and other types). So modified the original printSchema to print them. https://github.com/Hossein-s/type-graphql-schema-directives/commit/fb236bee9afcf6aafca446d75ce0a437a35c75ca

Hossein-s avatar Jun 02 '19 15:06 Hossein-s

Hey @Hossein-s @19majkel94 , What's the currnet status of this? I was investigating this some time ago for https://github.com/prisma/nexus/issues/53#issuecomment-467169476 I have some time now and would like to help you solve and test this.

ngmariusz avatar Jun 21 '19 18:06 ngmariusz

Yes please I would love to add field meta data from directives as well for use in the UI. My intent was to convert the schema in full server side and hang somewhere so the client can pull in all the meta data for UI auto generation.

jjwtay avatar Jun 22 '19 19:06 jjwtay

Hey @Hossein-s, is your solution working ? I'm really interested to add directive to field

EdouardBougon avatar Jul 26 '19 15:07 EdouardBougon

@Hossein-s Thanks for your idea, and I finally finished my workaroud repo.

@MichalLytek This is an in-doing, buggy work for schema directive. I'm wondering if this is the right approach?

snys98 avatar Oct 12 '19 23:10 snys98

@snys98 We already have a working implementation of directives support in a pending PR: https://github.com/MichalLytek/type-graphql/pull/369

MichalLytek avatar Oct 13 '19 18:10 MichalLytek

This issue is partially solved by #369 but it doesn't support all the directives locations like scalars or arguments. So I will leave this issue open to track the work needs to be done in the future.

MichalLytek avatar Nov 02 '19 14:11 MichalLytek

Are there plans to support Directives on Subscriptions?

glentakahashi avatar Nov 21 '19 22:11 glentakahashi

@glentakahashi

This issue is partially solved by #369 but it doesn't support all the directives locations

Feel free to open a PR with added support for subscriptions.

MichalLytek avatar Nov 22 '19 09:11 MichalLytek

Because of #546 and other issues, I've decided to rollback the support for JS-like syntax - now only the SDL string way is supported. Sorry for that, better now than after stable release 😝

@glentakahashi I've added a test case and subscriptions should also be supported from the beginning πŸ˜‰

MichalLytek avatar Feb 26 '20 18:02 MichalLytek

@MichalLytek thanks for the update! And to clarify, do you mean that directives on subscriptions should now work with the SDL method? Or that they will work in a future method.

glentakahashi avatar Feb 26 '20 19:02 glentakahashi