Skip to content
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

Does not work in tandem with other directives #98

Open
robross0606 opened this issue Sep 17, 2019 · 2 comments
Open

Does not work in tandem with other directives #98

robross0606 opened this issue Sep 17, 2019 · 2 comments

Comments

@robross0606
Copy link

Cruddl throws up if you also reference other non-cruddl directives in your schema. By graphql standard, @Directives are supposed to be chainable from left-to-right. Instead, cruddl throws errors if it sees a directive it doesn't understand. Is there a way to pass other directive handlers into cruddl so it knows about the use of other non-cruddl directives such as graphql-constraint-directive?

@Yogu
Copy link
Member

Yogu commented Sep 17, 2019

If we manage to sketch an elegant solution to really support custom directives, this can make its way to cruddl. But it first needs to be designed properly. For example, we could allow users to specify custom directive definitions, which then are made available in the model classes. Then, you could read these directives within a custom database adapter. However, if you want to dive deep into the core cruddl concepts of root entities, child entities, value objects, entity extensions, references, relations, collect fields etc. and modify their behavior, this would be very complicated to design properly I think.

By graphql standard, @Directives are supposed to be cumulative and chainable from left-to-right. There is no reason cruddl should be throwing an error if it sees a directive it doesn't understand.

Not sure what part of the GraphQL spec you are referring to. We use the validator KnownDirectives from graphql-js with the following documentation:

A GraphQL document is only valid if all @directives are known by the schema and legally positioned.

Also, the sources cruddl accepts use GraphQL syntax and adhere to its semantics, but they are by design not exactly equivalent to arbitrary type system definitions. We're just using GraphQL to configure cruddl. A very important goal of cruddl is that schema errors are caught with meaningful messages, assisting the user in writing functional schemas. That's why everything that's not explicitly implemented is forbidden. Just allowing to specify directive definitions would not help at all - the directives would just get lost in cruddl.

Is there a way to pass other directive handlers into cruddl so it knows about the use of other non-cruddl directives such as graphql-constraint-directive?

Thanks for providing an example (I guess you mean https://github.com/confuser/graphql-constraint-directive). You won't be able to just "pass directive handlers" and graphql-constraint-directive would magically work. If I understand that directive correctly, you need to set that on input type definitions. You can't pass input type definitions to cruddl. Instead, input types are generated automatically - for creating, updating, and filtering objects. Validation on input data is on the roadmap, but it will be a specific cruddl feature.

@robross0606
Copy link
Author

A very good point about the input type directive. That just happened to be one thing I pulled up as a potential example. The other one is audit logging. I could see that one may have to be implemented as a DbAdapter wrapper.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants