Graphql-js: Allow schemaPrinter to be customized / extended.

Created on 8 Jul 2019  路  5Comments  路  Source: graphql/graphql-js

Some libraries are having to copy the schemaPrinter code to do custom stuff to the output, (@apollo/federation, type-graphql). These libraries require outputting type and field level directives since this isn't supported in this library. Is there any support to do so, or can the schemPrinter just export the other functions so we don't have to copy every function or, even better, create a SchemaPrinter class so everything can be over-ridden, then printSchema options can set the printer for BC:

type Options = {
    commentDescriptions?: boolean,
    printer?: SchemaPrinter
};

const defaultPrinter = new SchemaPrinter();

export function printSchema(schema: GraphQLSchema, options?: Options): string {
    const printer = options.printer || defaultPrinter;

    return printer.print(
        schema,
        n => !isSpecifiedDirective(n),
        isDefinedType,
        options,
    );
}

export function printIntrospectionSchema(
    schema: GraphQLSchema,
    options?: Options,
): string {
    const printer = options.printer || defaultPrinter;

    return printer.print(
        schema,
        isSpecifiedDirective,
        isIntrospectionType,
        options,
    );
}
enhancement

Most helpful comment

@IvanGoncharov What if printSchema only printed directives that were not visited during schema construction? Using your example:

type Foo {
  foo: String
}

type Bar @copyFields(fromType: "Foo") {
  bar: String
}
// 1
printSchema(makeExecutableSchema({
    typeDefs,
    schemaDirectives: {
        copyFields: CopyFieldsVisitor
    }
}))

// 2
printSchema(makeExecutableSchema({
    typeDefs,
    schemaDirectives: {
    }
}))

The first snippet would print:

type Foo {
  foo: String
}

type Bar {
  foo: String
  bar: String
}

The second would print:

type Foo {
  foo: String
}

type Bar @copyFields(fromType: "Foo") {
  bar: String
}

All 5 comments

Instead of making an extensible printer, would it solve your issue if directives on schema definitions were included in the print step?

I believe this is actually a bug where we forgot to add directive printing for printSchema when we added directives on type and field definitions. I'd welcome both a PR for test cases that are currently missing and a PR for fixing the issue in the printer.

@j @mjmahone I don't think we should print directives based on astNode properties since directives are tight to SDL context. For example:

type Foo {
  foo: String
}

type Bar @copyFields(fromType: "Foo") {
  bar: String
}

Assuming @copyFields directive successfully modified the schema and assuming we implemented your proposal output will be like that:

type Foo {
  foo: String
}

type Bar @copyFields(fromType: "Foo") {
  foo: String
  bar: String
}

That doesn't make sense in the context of @copyFields or any other directive that do something more complicated than attaching metadata.

I think the correct solution would be to design GraphQLSDLDirective (derived from GraphQLDirective) and allow you to register callbacks for transforming types/fields (including extensions as described in #1527) and another callback that called during schema printing where directive can decide to return DirectiveNode for particular type/field (probably base on extensions).

This solution also solves the problem of code-first schema that wants to be printed with additional data represented as directives in SDL.
So I propose to freeze this issue until we start 16.0.0-alpha.x (~1 month) so we would be able to experiment with API design without breaking changes.

@IvanGoncharov

That definitely sounds like a much better solution. I was just noticing other libraries are simply copy/pasting schema printer to do their own logic and pull out directives as their library has them. extensions sounds elegant as well as the GraphQLSDLDirective. Combined pretty much solves everyone's issues. extensions alone will fix the code-first schema for many libraries creating schema using GraphQLSchema instead of SDL.

I'm super excited for this change as it'll marry everyone's wishes (having directives :P)

I've been working on implementing apollo-federation into type-graphql and they use astNode to do field-level directives. extensions will make it all feel less hacky.

Ref: https://github.com/19majkel94/type-graphql/pull/369 https://github.com/apollographql/apollo-server/pull/3013

Found this after I made appsync-schema-convert myself, also copying schemaPrinter.js.

Looking forward to 16.0.0 for a more elegant solution!

@IvanGoncharov What if printSchema only printed directives that were not visited during schema construction? Using your example:

type Foo {
  foo: String
}

type Bar @copyFields(fromType: "Foo") {
  bar: String
}
// 1
printSchema(makeExecutableSchema({
    typeDefs,
    schemaDirectives: {
        copyFields: CopyFieldsVisitor
    }
}))

// 2
printSchema(makeExecutableSchema({
    typeDefs,
    schemaDirectives: {
    }
}))

The first snippet would print:

type Foo {
  foo: String
}

type Bar {
  foo: String
  bar: String
}

The second would print:

type Foo {
  foo: String
}

type Bar @copyFields(fromType: "Foo") {
  bar: String
}
Was this page helpful?
0 / 5 - 0 ratings