Add option printDirective to printSchema

[spawnia]

Resolves https://github.com/graphql/graphql-js/issues/2020

I initially thought about implementing this as https://github.com/graphql/graphql-js/issues/2020#issuecomment-866798901, but really could not think of a good use case where a directive should be printed in a modified way.

Thus, I went for a simpler signature:

shouldPrintDirective?: (directiveNode: DirectiveNode) => boolean

This is only a partial implementation for now, there are more possible locations in a schema that can have a directive. Before I finish the implementation, I would like to validate the direction is right.

[IvanGoncharov]

@spawnia Thanks for opening draft PR. Any solution that we merge should work with a code-first approach and extensions. I want to push your previous proposal https://github.com/graphql/graphql-js/issues/2020#issuecomment-866798901 even further and add callback as part of GraphQLDirective that accepts GraphQLField, GraphQLType, etc., and returns an array of directive args.

Example:

export const GraphQLDeprecatedDirective: GraphQLDirective =
  new GraphQLDirective({
    name: 'deprecated',
    locations: [
      DirectiveLocation.FIELD_DEFINITION,
      // ...
    ],
    args: {
      reason: {
        type: GraphQLString,
        defaultValue: DEFAULT_DEPRECATION_REASON,
      },
    },
    notSureWhatNameToChoose(obj) {
      if (obj.deprecationReason != null) {
        return [
          obj.deprecationReason === DEFAULT_DEPRECATION_REASON
          ? {}
          : { reason: obj.deprecationReason },
        ];
      }
    }
  });

That way directives can leave in separate packages and be fully self-contained.

[spawnia]

Hey @IvanGoncharov, thanks for the feedback on the PR.

It does surface a very interesting point, that is how we can bridge the gap for features such as @deprecated, @specifiedBy and possibly future features such as @oneOf.

Representation

• SDL: ✔️ covered by the spec
• Code: ✔️ mechanism is up to the implementation
• Introspection : ✔️ covered by the spec

Conversion

• SDL to Code: ✔️ supported in buildSchema
• Introspection to Code: ✔️ supported in buildClientSchema
• Code to Introspection: ✔️supported by introspection mechanism
• SDL to Introspection: ✔️ works through SDL -> Code -> Introspection
• Code to SDL: ⛔ not supported
• Introspection to SDL: ⛔ would work through Introspection -> Code -> SDL

With those built-in features that are also available in introspection, I do not see a use case why they should ever not be present in a printed schema. They are already exposed through introspection anyways, and thus not expected to contain internal-only information. Thus, I think we could simply always print them. If there are concerns regarding backwards compatiblity, we could add a config setting such as printBuiltInDirectives: bool.

I want to push your previous proposal #2020 (comment) even further and add callback as part of GraphQLDirective that accepts GraphQLField, GraphQLType, etc., and returns an array of directive args.

Your example makes it quite clear why it would be useful for directives to influence how they are printed. Howerver, in a purely SDL driven implementation, not every used directive necessarily even has a code based definition associated with it. The main project I am building (https://github.com/nuwave/lighthouse) only defines directives through SDL, which has been sufficient for validation and all other purposes. We would require a default implementation of this method anyways - probably just returning all the arguments? I would argue that this functionality is related but not essential to the problem I am trying to solve here.

Any solution that we merge should work with a code-first approach and extensions.

If it must exclusively work with extensions, that would force directive users into having to specify the following for each directive:

1. Method to convert directive into extensions
2. Method to convert extensions back into directive

We should offer a generalized implementation of this that smooths the path from SDL -> Code -> SDL for implementations that are primarily SDL-driven.

[IvanGoncharov]

With those built-in features that are also available in introspection, I do not see a use case why they should ever not be present in a printed schema. They are already exposed through introspection anyways, and thus not expected to contain internal-only information. Thus, I think we could simply always print them. If there are concerns regarding backwards compatiblity, we could add a config setting such as printBuiltInDirectives: bool.

@spawnia Not sure I understand your arguments, yes built-in stuff should be printable. If you look into source code they are already printed with hand-written code for each directive. I just suggest a mechanism that will work for both built-in and custom directives.

The main project I am building (nuwave/lighthouse) only defines directives through SDL, which has been sufficient for validation and all other purposes. We would require a default implementation of this method anyways - probably just returning all the arguments? I would argue that this functionality is related but not essential to the problem I am trying to solve here.

astNode were added just to enhance error messages we can't use them for anything else. I totally agree with Lee on this one: https://github.com/graphql/graphql-js/issues/869#issuecomment-309899380 The only way forward that I see is @directive => change object (field, type, etc.) including extensions => @directive.

[spawnia]

If you look into source code they are already printed with hand-written code for each directive.

You are right, I completely missed that.

Since extensions is completely custom, I don't think we can build abstractions on it to make directive printing any easier. I came back around to https://github.com/graphql/graphql-js/issues/2020 and now implemented a POC for an API that does not have any assumptions about SDL usage, extensions, presence or implementation of directive classes, etc.

[IvanGoncharov]

Since extensions is completely custom, I don't think we can build abstractions on it to make directive printing any easier. I came back around to #2020 and now implemented a POC for an API that does not have any assumptions about SDL usage, extensions, presence or implementation of directive classes, etc.

@spawnia It should be tied to directive objects, otherwise it's not modular. We tie everything to a schema (resolvers, parseValue/serialize, enum values, etc.) so we shouldn't break it for directives. Otherwise schema created by one tool/library is not fully printable by another library.

[spawnia]

It should be tied to directive objects, otherwise it's not modular.

That implies we tie directive objects to definitions in a unified manner. That seems to contradict the philosophy of not letting directives bleed into the programmatic representation of a schema.

The implementation I propose is agnostic as to if or how directives are tied into the schema. It can leverage AST nodes if the schema came from an SDL source, it can use something the developer placed in extensions, or it can use any other mechanism. It merely acknowledges that it may be desired to include directives in a printed SDL document, and leaves everything else completely open.

Otherwise schema created by one tool/library is not fully printable by another library.

I am not convinced that schema-agnostic and universally uniform printing is needed. It may even be necessary to do custom printing, for instance in Apollo federation: https://www.apollographql.com/docs/federation/federation-spec/#fetch-service-capabilities

[IvanGoncharov]

I am not convinced that schema-agnostic and universally uniform printing is needed. It may even be necessary to do custom printing, for instance in Apollo federation: apollographql.com/docs/federation/federation-spec/#fetch-service-capabilities

@spawnia It's exactly a problem we have right now. Apollo has all these directives in their schema but you can't print them because:

1. You can't print directives at all and your solution solves that.
2. Code for printing is Apollo-specific, your solution doesn't fix that.

If we truly want to solve this problem, the schema should be self-printable.

That implies we tie directive objects to definitions in a unified manner. That seems to contradict the philosophy of not letting directives bleed into the programmatic representation of a schema.

Directive definitions are already part of the schema even SDL directives (you can check introspection). If you read spec:

Directives can also be used to annotate the type system definition language as well, which can be a useful tool for supplying additional metadata in order to generate GraphQL execution services, produce client generated runtime code, or many other useful extensions of the GraphQL semantics.

So "supplying additional metadata" is directly mapped to extensions as it the only "metadata-like" thing you can change in GraphQL* classes. Also, it's perfectly fine to have directive objects in the schema (and we have them today). What's not ok (and Lee was referring to it) is to have "directive values" (not definitions) being associated with schema. The directive should be used as "transformations" (the same way as decorators used in any language) and should not persist in the schema (but transformation can do anything including writing to "extensions").

Back to your proposal, let's use your framework as an example. Disclaimer: Don't know much about graphql-php but I assume it work the same way as graphql-js. So you defined your directives in SDL and process users SDL, now you get schema which you can pass to any other tool. Those 3rd-party tools see SDL directives in your schema (schema.getDirectives()) but see the same schema as your framework use but at the same time can't print schema in the same way as you do (because code for printing directives is not part of the schema).

[spawnia]

I removed everything that was tied directly to the SDL source. The implementation is now fully agnostic to what will result in the printed directives in the final schema.

I think this implementation is useful on its own. graphql-js being the fundamental base library for all GraphQL implementations does not need to be any more opinionated than this. Imposing any further structure also imposes additional constraints on potential use cases. It should be left up to users how they want to decide the directives that are printed.