Skip to content

Standard directives question #632

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

Closed
sungam3r opened this issue Oct 24, 2019 · 8 comments · Fixed by #633
Closed

Standard directives question #632

sungam3r opened this issue Oct 24, 2019 · 8 comments · Fixed by #633
Labels
✏️ Editorial PR is non-normative or does not influence implementation

Comments

@sungam3r
Copy link
Contributor

I did not find in the specification the answer to the question whether standard directives include, deprecated and skip should be returned from introspection request. And should these directives be presented in SDL? Or can they be omitted in both cases?

@IvanGoncharov IvanGoncharov added 🤷‍♀️ Ambiguity An issue/PR which identifies or fixes spec ambiguity ✏️ Editorial PR is non-normative or does not influence implementation and removed 🤷‍♀️ Ambiguity An issue/PR which identifies or fixes spec ambiguity labels Oct 24, 2019
@IvanGoncharov
Copy link
Member

IvanGoncharov commented Oct 24, 2019

@sungam3r De facto you don't need to specify standard directive or standard types in SDL.
Also, de facto all types and directives include standard ones always present inside introspection.
But I agree it needs to clarify it somewhere similar to how it's done for standard scalars in #597.

@sungam3r Do you want to work on such PR?

@sungam3r
Copy link
Contributor Author

Ok. I will try. I just want to clarify the answer - no standard directives in SDL and all of them in introspection. Right?

@sungam3r
Copy link
Contributor Author

If a built-in scalar type is not referenced anywhere in a schema (there is no field, argument, or input field of that type) then it must not be included in the schema

Will this rule also apply to the @deprecated directive if it is not used? And it turns out that the @skip and @include directives should never come with introspection since they cannot be used in the schema, they are intended for requests (ExecutableDirectiveLocation).

@IvanGoncharov
Copy link
Member

I just want to clarify the answer - no standard directives in SDL and all of them in introspection. Right?

I would say they can be omitted in SDL and then the implementation should add default implementation. But SDL can contain standard directive, for example:
https://github.com/graphql/graphql-js/blob/c0cf320d3a569c83166ac93d36b2dfb81ebb5b75/src/utilities/__tests__/schemaPrinter-test.js#L593-L599

And it turns out that the @Skip and @include directives should never come with introspection since they cannot be used in the schema, they are intended for requests (ExecutableDirectiveLocation).

Reference criteria make sense for types but not for query directives if the directive location allows directives to be used in query document it should 100% be present in introspection.

Will this rule also apply to the @deprecated directive if it is not used?

This is an interesting question on its own. At the moment we put all directives into introspection even if they SDL ones. I would keep it as is until we figure out the solution for #300.

@sungam3r
Copy link
Contributor Author

By the way, all my questions are caused by a #300. I carefully read this issue and all the issues in one way or another related to it. Now I have a working solution in my fork of graphql-dotnet for getting directives by introspection, I also have a converter from json result to SDL format. It remains only to resolve this question with the standard directives.

@sungam3r
Copy link
Contributor Author

Since we are talking about #597:

If a built-in scalar type is not
referenced anywhere in a schema (there is no field, argument, or input field of
that type) then it must not be included in the schema.
When representing a GraphQL schema using the type system definition language, all built-in scalars must be omitted for brevity.

But what about not built-in not referenced anywhere scalars? Should they be present in SDL?

@IvanGoncharov
Copy link
Member

But what about not built-in not referenced anywhere scalars? Should they be present in SDL?

At the moment yes and I think SDL should be a valid option for representing partial schemas in general.

@IvanGoncharov
Copy link
Member

Closing this issue since progress now tracked in #633

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✏️ Editorial PR is non-normative or does not influence implementation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants