From 028edb0c314ddbb8525e5bef3b36cfac33df9f79 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Fri, 5 Nov 2021 13:04:33 +0100 Subject: [PATCH 1/5] Add option printDirective to printSchema 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: ```ts 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. --- src/execution/__tests__/directives-test.ts | 4 + src/utilities/__tests__/printSchema-test.ts | 98 ++++++++++++++++++++- src/utilities/printSchema.ts | 60 +++++++++++-- 3 files changed, 151 insertions(+), 11 deletions(-) diff --git a/src/execution/__tests__/directives-test.ts b/src/execution/__tests__/directives-test.ts index 92c8fb9c5f..78f5dd98fb 100644 --- a/src/execution/__tests__/directives-test.ts +++ b/src/execution/__tests__/directives-test.ts @@ -174,6 +174,7 @@ describe('Execute: handles directives', () => { data: { a: 'a', b: 'b' }, }); }); + it('unless false includes inline fragment', () => { const result = executeTestQuery(` query { @@ -188,6 +189,7 @@ describe('Execute: handles directives', () => { data: { a: 'a', b: 'b' }, }); }); + it('unless true includes inline fragment', () => { const result = executeTestQuery(` query { @@ -234,6 +236,7 @@ describe('Execute: handles directives', () => { data: { a: 'a', b: 'b' }, }); }); + it('unless false includes anonymous inline fragment', () => { const result = executeTestQuery(` query Q { @@ -248,6 +251,7 @@ describe('Execute: handles directives', () => { data: { a: 'a', b: 'b' }, }); }); + it('unless true includes anonymous inline fragment', () => { const result = executeTestQuery(` query { diff --git a/src/utilities/__tests__/printSchema-test.ts b/src/utilities/__tests__/printSchema-test.ts index c9c16f0d5c..bb5f09c25b 100644 --- a/src/utilities/__tests__/printSchema-test.ts +++ b/src/utilities/__tests__/printSchema-test.ts @@ -21,12 +21,19 @@ import { } from '../../type/definition'; import { buildSchema } from '../buildASTSchema'; -import { printSchema, printIntrospectionSchema } from '../printSchema'; +import type { PrintSchemaOptions } from '../printSchema'; +import { + printSchema, + printIntrospectionSchema, +} from '../printSchema'; -function expectPrintedSchema(schema: GraphQLSchema) { - const schemaText = printSchema(schema); +function expectPrintedSchema( + schema: GraphQLSchema, + options?: PrintSchemaOptions, +) { + const schemaText = printSchema(schema, options); // keep printSchema and buildSchema in sync - expect(printSchema(buildSchema(schemaText))).to.equal(schemaText); + expect(printSchema(buildSchema(schemaText), options)).to.equal(schemaText); return expect(schemaText); } @@ -260,6 +267,16 @@ describe('Type System Printer', () => { `); }); + it('Omits schema of common names', () => { + const schema = new GraphQLSchema({ + query: new GraphQLObjectType({ name: 'Query', fields: {} }), + }); + + expectPrintedSchema(schema).to.equal(dedent` + type Query + `); + }); + it('Prints schema with description', () => { const schema = new GraphQLSchema({ description: 'Schema description.', @@ -318,6 +335,79 @@ describe('Type System Printer', () => { `); }); + it('Prints schema with directives', () => { + const schema = buildSchema(` + schema @foo { + query: Query + } + + directive @foo on SCHEMA + + type Query + `); + + expectPrintedSchema(schema, { shouldPrintDirective: () => true }).to + .equal(dedent` + schema @foo { + query: Query + } + + directive @foo on SCHEMA + + type Query + `); + }); + + it('Includes directives conditionally', () => { + const schema = buildSchema(` + schema @foo @bar { + query: Query + } + + directive @foo on SCHEMA + + directive @bar on SCHEMA + + type Query + `); + + expectPrintedSchema(schema, { + shouldPrintDirective: (directive) => directive.name.value === 'foo', + }).to.equal(dedent` + schema @foo { + query: Query + } + + directive @foo on SCHEMA + + directive @bar on SCHEMA + + type Query + `); + + expectPrintedSchema(schema, { shouldPrintDirective: () => true }).to + .equal(dedent` + schema @foo @bar { + query: Query + } + + directive @foo on SCHEMA + + directive @bar on SCHEMA + + type Query + `); + + expectPrintedSchema(schema, { shouldPrintDirective: () => false }).to + .equal(dedent` + directive @foo on SCHEMA + + directive @bar on SCHEMA + + type Query + `); + }); + it('Print Interface', () => { const FooType = new GraphQLInterfaceType({ name: 'Foo', diff --git a/src/utilities/printSchema.ts b/src/utilities/printSchema.ts index 16b93e7b1a..9a1de2a01f 100644 --- a/src/utilities/printSchema.ts +++ b/src/utilities/printSchema.ts @@ -34,18 +34,39 @@ import { isInputObjectType, } from '../type/definition'; +import type { DirectiveNode } from '../language/ast'; + import { astFromValue } from './astFromValue'; -export function printSchema(schema: GraphQLSchema): string { +export interface PrintSchemaOptions { + /** + * Should the given directive node be included in the print output? + */ + shouldPrintDirective?: (directiveNode: DirectiveNode) => boolean; +} + +export function printSchema( + schema: GraphQLSchema, + options?: PrintSchemaOptions, +): string { return printFilteredSchema( schema, (n) => !isSpecifiedDirective(n), isDefinedType, + options, ); } -export function printIntrospectionSchema(schema: GraphQLSchema): string { - return printFilteredSchema(schema, isSpecifiedDirective, isIntrospectionType); +export function printIntrospectionSchema( + schema: GraphQLSchema, + options?: PrintSchemaOptions, +): string { + return printFilteredSchema( + schema, + isSpecifiedDirective, + isIntrospectionType, + options, + ); } function isDefinedType(type: GraphQLNamedType): boolean { @@ -56,12 +77,13 @@ function printFilteredSchema( schema: GraphQLSchema, directiveFilter: (type: GraphQLDirective) => boolean, typeFilter: (type: GraphQLNamedType) => boolean, + options?: PrintSchemaOptions, ): string { const directives = schema.getDirectives().filter(directiveFilter); const types = Object.values(schema.getTypeMap()).filter(typeFilter); return [ - printSchemaDefinition(schema), + printSchemaDefinition(schema, options), ...directives.map((directive) => printDirective(directive)), ...types.map((type) => printType(type)), ] @@ -69,8 +91,27 @@ function printFilteredSchema( .join('\n\n'); } -function printSchemaDefinition(schema: GraphQLSchema): Maybe { - if (schema.description == null && isSchemaOfCommonNames(schema)) { +function printSchemaDefinition( + schema: GraphQLSchema, + options?: PrintSchemaOptions, +): Maybe { + const directives: Array = []; + const printDirectives = options?.shouldPrintDirective; + if (printDirectives) { + [schema.astNode, ...schema.extensionASTNodes].forEach((node) => { + node?.directives?.forEach((directive) => { + if (printDirectives(directive)) { + directives.push(print(directive)); + } + }); + }); + } + + if ( + schema.description == null && + directives.length === 0 && + isSchemaOfCommonNames(schema) + ) { return; } @@ -91,7 +132,12 @@ function printSchemaDefinition(schema: GraphQLSchema): Maybe { operationTypes.push(` subscription: ${subscriptionType.name}`); } - return printDescription(schema) + `schema {\n${operationTypes.join('\n')}\n}`; + return ( + printDescription(schema) + + ['schema', directives.join(' '), `{\n${operationTypes.join('\n')}\n}`] + .filter(Boolean) + .join(' ') + ); } /** From c39eb2d9bdc034903276f19f575baa5c5d15fff8 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Mon, 8 Nov 2021 17:18:04 +0100 Subject: [PATCH 2/5] Refocus on AST agnostic implementation --- src/language/ast.ts | 11 ++- src/language/parser.ts | 16 +++++ src/utilities/__tests__/printSchema-test.ts | 51 ++++++------- src/utilities/printSchema.ts | 79 ++++++++++++++------- 4 files changed, 101 insertions(+), 56 deletions(-) diff --git a/src/language/ast.ts b/src/language/ast.ts index 0b30366df0..4200d97e3c 100644 --- a/src/language/ast.ts +++ b/src/language/ast.ts @@ -1,4 +1,4 @@ -import type { Kind } from './kinds'; +import { Kind } from './kinds'; import type { Source } from './source'; import type { TokenKind } from './tokenKind'; @@ -288,6 +288,15 @@ export function isNode(maybeNode: any): maybeNode is ASTNode { return typeof maybeKind === 'string' && kindValues.has(maybeKind); } +/** + * @internal + */ +export function isConstDirectiveNode( + maybeConstDirectiveNode: any, +): maybeConstDirectiveNode is ConstDirectiveNode { + return maybeConstDirectiveNode?.kind === Kind.DIRECTIVE; +} + /** Name */ export interface NameNode { diff --git a/src/language/parser.ts b/src/language/parser.ts index 1b0bc75f92..e91c18c7f1 100644 --- a/src/language/parser.ts +++ b/src/language/parser.ts @@ -165,6 +165,22 @@ export function parseType( return type; } +/** + * Parse the string containing a GraphQL directive (ex. `@foo(bar: "baz")`) into its AST. + * + * Throws GraphQLError if a syntax error is encountered. + */ +export function parseConstDirective( + source: string | Source, + options?: ParseOptions, +): ConstDirectiveNode { + const parser = new Parser(source, options); + parser.expectToken(TokenKind.SOF); + const directive = parser.parseDirective(true); + parser.expectToken(TokenKind.EOF); + return directive; +} + /** * This class is exported only to assist people in implementing their own parsers * without duplicating too much code and should be used only as last resort for cases diff --git a/src/utilities/__tests__/printSchema-test.ts b/src/utilities/__tests__/printSchema-test.ts index bb5f09c25b..db7775e52f 100644 --- a/src/utilities/__tests__/printSchema-test.ts +++ b/src/utilities/__tests__/printSchema-test.ts @@ -6,26 +6,28 @@ import { dedent, dedentString } from '../../__testUtils__/dedent'; import { DirectiveLocation } from '../../language/directiveLocation'; import type { GraphQLFieldConfig } from '../../type/definition'; -import { GraphQLSchema } from '../../type/schema'; -import { GraphQLDirective } from '../../type/directives'; -import { GraphQLInt, GraphQLString, GraphQLBoolean } from '../../type/scalars'; import { + GraphQLEnumType, + GraphQLInputObjectType, + GraphQLInterfaceType, GraphQLList, GraphQLNonNull, - GraphQLScalarType, GraphQLObjectType, - GraphQLInterfaceType, + GraphQLScalarType, GraphQLUnionType, - GraphQLEnumType, - GraphQLInputObjectType, } from '../../type/definition'; +import { GraphQLSchema, isSchema } from '../../type/schema'; +import { GraphQLDirective } from '../../type/directives'; +import { GraphQLBoolean, GraphQLInt, GraphQLString } from '../../type/scalars'; import { buildSchema } from '../buildASTSchema'; import type { PrintSchemaOptions } from '../printSchema'; import { - printSchema, + directivesFromAstNodes, printIntrospectionSchema, + printSchema, } from '../printSchema'; +import { parseConstDirective } from '../../language/parser'; function expectPrintedSchema( schema: GraphQLSchema, @@ -346,8 +348,9 @@ describe('Type System Printer', () => { type Query `); - expectPrintedSchema(schema, { shouldPrintDirective: () => true }).to - .equal(dedent` + expectPrintedSchema(schema, { + printDirectives: directivesFromAstNodes, + }).to.equal(dedent` schema @foo { query: Query } @@ -360,7 +363,7 @@ describe('Type System Printer', () => { it('Includes directives conditionally', () => { const schema = buildSchema(` - schema @foo @bar { + schema @foo { query: Query } @@ -372,22 +375,15 @@ describe('Type System Printer', () => { `); expectPrintedSchema(schema, { - shouldPrintDirective: (directive) => directive.name.value === 'foo', - }).to.equal(dedent` - schema @foo { - query: Query - } - - directive @foo on SCHEMA - - directive @bar on SCHEMA - - type Query - `); + printDirectives: (definition) => { + if (isSchema(definition)) { + return [parseConstDirective('@bar')]; + } - expectPrintedSchema(schema, { shouldPrintDirective: () => true }).to - .equal(dedent` - schema @foo @bar { + return []; + }, + }).to.equal(dedent` + schema @bar { query: Query } @@ -398,8 +394,7 @@ describe('Type System Printer', () => { type Query `); - expectPrintedSchema(schema, { shouldPrintDirective: () => false }).to - .equal(dedent` + expectPrintedSchema(schema, { printDirectives: () => [] }).to.equal(dedent` directive @foo on SCHEMA directive @bar on SCHEMA diff --git a/src/utilities/printSchema.ts b/src/utilities/printSchema.ts index 9a1de2a01f..db04d3027e 100644 --- a/src/utilities/printSchema.ts +++ b/src/utilities/printSchema.ts @@ -7,42 +7,52 @@ import { print } from '../language/printer'; import { printBlockString } from '../language/blockString'; import type { GraphQLSchema } from '../type/schema'; +import { isSchema } from '../type/schema'; import type { GraphQLDirective } from '../type/directives'; +import { + DEFAULT_DEPRECATION_REASON, + isSpecifiedDirective, +} from '../type/directives'; import type { - GraphQLNamedType, GraphQLArgument, - GraphQLInputField, - GraphQLScalarType, GraphQLEnumType, - GraphQLObjectType, + GraphQLEnumValue, + GraphQLField, + GraphQLInputField, + GraphQLInputObjectType, GraphQLInterfaceType, + GraphQLNamedType, + GraphQLObjectType, + GraphQLScalarType, + GraphQLType, GraphQLUnionType, - GraphQLInputObjectType, } from '../type/definition'; -import { isIntrospectionType } from '../type/introspection'; -import { isSpecifiedScalarType } from '../type/scalars'; import { - DEFAULT_DEPRECATION_REASON, - isSpecifiedDirective, -} from '../type/directives'; -import { - isScalarType, - isObjectType, - isInterfaceType, - isUnionType, isEnumType, isInputObjectType, + isInterfaceType, + isObjectType, + isScalarType, + isUnionType, } from '../type/definition'; +import { isIntrospectionType } from '../type/introspection'; +import { isSpecifiedScalarType } from '../type/scalars'; -import type { DirectiveNode } from '../language/ast'; +import type { ConstDirectiveNode } from '../language/ast'; +import { isConstDirectiveNode } from '../language/ast'; import { astFromValue } from './astFromValue'; export interface PrintSchemaOptions { - /** - * Should the given directive node be included in the print output? - */ - shouldPrintDirective?: (directiveNode: DirectiveNode) => boolean; + printDirectives?: ( + definition: + | GraphQLSchema + | GraphQLType + | GraphQLField + | GraphQLEnumValue + | GraphQLInputField + | GraphQLArgument, + ) => ReadonlyArray; } export function printSchema( @@ -69,6 +79,25 @@ export function printIntrospectionSchema( ); } +/** + * Useful implementation of PrintSchemaOptions.printDirectives for users who + * define their schema through SDL and simply want to print all included directives. + */ +export const directivesFromAstNodes: PrintSchemaOptions['printDirectives'] = ( + definition, +) => { + if (isSchema(definition)) { + return [ + ...(definition.astNode?.directives ?? []), + ...(definition?.extensionASTNodes + ?.map((node) => node.directives) + ?.flat() ?? []), + ].filter(isConstDirectiveNode); + } + + return []; +}; + function isDefinedType(type: GraphQLNamedType): boolean { return !isSpecifiedScalarType(type) && !isIntrospectionType(type); } @@ -96,14 +125,10 @@ function printSchemaDefinition( options?: PrintSchemaOptions, ): Maybe { const directives: Array = []; - const printDirectives = options?.shouldPrintDirective; + const printDirectives = options?.printDirectives; if (printDirectives) { - [schema.astNode, ...schema.extensionASTNodes].forEach((node) => { - node?.directives?.forEach((directive) => { - if (printDirectives(directive)) { - directives.push(print(directive)); - } - }); + printDirectives(schema).forEach((directive) => { + directives.push(print(directive)); }); } From 2d1c73612967d97203a2fb18a387f51afedfbf19 Mon Sep 17 00:00:00 2001 From: spawnia Date: Sun, 12 Dec 2021 18:09:45 +0100 Subject: [PATCH 3/5] Remove any ties to source directives --- src/utilities/__tests__/printSchema-test.ts | 13 ++--- src/utilities/printSchema.ts | 62 ++++++++++----------- 2 files changed, 34 insertions(+), 41 deletions(-) diff --git a/src/utilities/__tests__/printSchema-test.ts b/src/utilities/__tests__/printSchema-test.ts index 054334f29b..30e9ccf130 100644 --- a/src/utilities/__tests__/printSchema-test.ts +++ b/src/utilities/__tests__/printSchema-test.ts @@ -23,7 +23,6 @@ import { GraphQLBoolean, GraphQLInt, GraphQLString } from '../../type/scalars'; import { buildSchema } from '../buildASTSchema'; import type { PrintSchemaOptions } from '../printSchema'; import { - directivesFromAstNodes, printIntrospectionSchema, printSchema, } from '../printSchema'; @@ -339,25 +338,21 @@ describe('Type System Printer', () => { it('Prints schema with directives', () => { const schema = buildSchema(` - schema @foo { - query: Query - } - - directive @foo on SCHEMA + directive @foo on SCHEMA | OBJECT type Query `); expectPrintedSchema(schema, { - printDirectives: directivesFromAstNodes, + printDirectives: () => [parseConstDirective('@foo')], }).to.equal(dedent` schema @foo { query: Query } - directive @foo on SCHEMA + directive @foo on SCHEMA | OBJECT - type Query + type Query @foo `); }); diff --git a/src/utilities/printSchema.ts b/src/utilities/printSchema.ts index 473e802bcb..cfd61d22c5 100644 --- a/src/utilities/printSchema.ts +++ b/src/utilities/printSchema.ts @@ -7,7 +7,6 @@ import { print } from '../language/printer'; import { isPrintableAsBlockString } from '../language/blockString'; import type { GraphQLSchema } from '../type/schema'; -import { isSchema } from '../type/schema'; import type { GraphQLDirective } from '../type/directives'; import { DEFAULT_DEPRECATION_REASON, @@ -39,7 +38,6 @@ import { isIntrospectionType } from '../type/introspection'; import { isSpecifiedScalarType } from '../type/scalars'; import type { ConstDirectiveNode } from '../language/ast'; -import { isConstDirectiveNode } from '../language/ast'; import { astFromValue } from './astFromValue'; @@ -79,25 +77,6 @@ export function printIntrospectionSchema( ); } -/** - * Useful implementation of PrintSchemaOptions.printDirectives for users who - * define their schema through SDL and simply want to print all included directives. - */ -export const directivesFromAstNodes: PrintSchemaOptions['printDirectives'] = ( - definition, -) => { - if (isSchema(definition)) { - return [ - ...(definition.astNode?.directives ?? []), - ...(definition?.extensionASTNodes - ?.map((node) => node.directives) - ?.flat() ?? []), - ].filter(isConstDirectiveNode); - } - - return []; -}; - function isDefinedType(type: GraphQLNamedType): boolean { return !isSpecifiedScalarType(type) && !isIntrospectionType(type); } @@ -114,23 +93,34 @@ function printFilteredSchema( return [ printSchemaDefinition(schema, options), ...directives.map((directive) => printDirective(directive)), - ...types.map((type) => printType(type)), + ...types.map((type) => printType(type, options)), ] .filter(Boolean) .join('\n\n'); } +function collectDirectives( + printDirectives: PrintSchemaOptions['printDirectives'] | undefined, + definition: + | GraphQLSchema + | GraphQLType + | GraphQLField + | GraphQLEnumValue + | GraphQLInputField + | GraphQLArgument, +) { + if (printDirectives == null) { + return []; + } + + return printDirectives(definition).map(print); +} + function printSchemaDefinition( schema: GraphQLSchema, options?: PrintSchemaOptions, ): Maybe { - const directives: Array = []; - const printDirectives = options?.printDirectives; - if (printDirectives) { - printDirectives(schema).forEach((directive) => { - directives.push(print(directive)); - }); - } + const directives = collectDirectives(options?.printDirectives, schema); if ( schema.description == null && @@ -199,12 +189,15 @@ function isSchemaOfCommonNames(schema: GraphQLSchema): boolean { return true; } -export function printType(type: GraphQLNamedType): string { +export function printType( + type: GraphQLNamedType, + options?: PrintSchemaOptions, +): string { if (isScalarType(type)) { return printScalar(type); } if (isObjectType(type)) { - return printObject(type); + return printObject(type, options); } if (isInterfaceType(type)) { return printInterface(type); @@ -238,10 +231,15 @@ function printImplementedInterfaces( : ''; } -function printObject(type: GraphQLObjectType): string { +function printObject( + type: GraphQLObjectType, + options?: PrintSchemaOptions, +): string { + const directives = collectDirectives(options?.printDirectives, type); return ( printDescription(type) + `type ${type.name}` + + (directives.length ? ' ' + directives.join(' ') : '') + printImplementedInterfaces(type) + printFields(type) ); From 60bce7c9d1f28aa237dba0102822bf754756287a Mon Sep 17 00:00:00 2001 From: spawnia Date: Sun, 12 Dec 2021 18:12:58 +0100 Subject: [PATCH 4/5] Cleanup --- src/language/ast.ts | 9 --------- src/utilities/__tests__/printSchema-test.ts | 5 +---- 2 files changed, 1 insertion(+), 13 deletions(-) diff --git a/src/language/ast.ts b/src/language/ast.ts index 4200d97e3c..3ce6465c89 100644 --- a/src/language/ast.ts +++ b/src/language/ast.ts @@ -288,15 +288,6 @@ export function isNode(maybeNode: any): maybeNode is ASTNode { return typeof maybeKind === 'string' && kindValues.has(maybeKind); } -/** - * @internal - */ -export function isConstDirectiveNode( - maybeConstDirectiveNode: any, -): maybeConstDirectiveNode is ConstDirectiveNode { - return maybeConstDirectiveNode?.kind === Kind.DIRECTIVE; -} - /** Name */ export interface NameNode { diff --git a/src/utilities/__tests__/printSchema-test.ts b/src/utilities/__tests__/printSchema-test.ts index 30e9ccf130..e8726cfe92 100644 --- a/src/utilities/__tests__/printSchema-test.ts +++ b/src/utilities/__tests__/printSchema-test.ts @@ -22,10 +22,7 @@ import { GraphQLBoolean, GraphQLInt, GraphQLString } from '../../type/scalars'; import { buildSchema } from '../buildASTSchema'; import type { PrintSchemaOptions } from '../printSchema'; -import { - printIntrospectionSchema, - printSchema, -} from '../printSchema'; +import { printIntrospectionSchema, printSchema } from '../printSchema'; import { parseConstDirective } from '../../language/parser'; function expectPrintedSchema( From 1ef6e19bd643f94e6294672030ed1fcd9f3eb4b7 Mon Sep 17 00:00:00 2001 From: spawnia Date: Sun, 12 Dec 2021 18:14:32 +0100 Subject: [PATCH 5/5] Revert ast --- src/language/ast.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/language/ast.ts b/src/language/ast.ts index 3ce6465c89..0b30366df0 100644 --- a/src/language/ast.ts +++ b/src/language/ast.ts @@ -1,4 +1,4 @@ -import { Kind } from './kinds'; +import type { Kind } from './kinds'; import type { Source } from './source'; import type { TokenKind } from './tokenKind';