From a0cf9b2e84890e3397f1207ce6df3e8e858c85c7 Mon Sep 17 00:00:00 2001 From: Lee Byron Date: Thu, 15 Jun 2017 18:33:07 -0700 Subject: [PATCH] RFC: Define custom scalars in terms of built-in scalars. This proposes an additive change which allows custom scalars to be defined in terms of the built-in scalars. The motivation is for client-side code generators to understand how to map between the GraphQL type system and a native type system. As an example, a `URL` custom type may be defined in terms of the built-in scalar `String`. It could define additional serialization and parsing logic, however client tools can know to treat `URL` values as `String`. Presently, we do this by defining these mappings manually on the client, which is difficult to scale, or by giving up and making no assumptions of how the custom types serialize. Another real use case of giving client tools this information is GraphiQL: this change will allow GraphiQL to show more useful errors when a literal of an incorrect kind is provided to a custom scalar. Currently GraphiQL simply accepts all values. To accomplish this, this proposes adding the following: * A new property when defining `GraphQLScalarType` (`ofType`) which asserts that only built-in scalar types are provided. * A second type coercion to guarantee to a client that the serialized values match the `ofType`. * Delegating the `parseLiteral` and `parseValue` functions to those in `ofType` (this enables downstream validation / GraphiQL features) * Exposing `ofType` in the introspection system, and consuming that introspection in `buildClientSchema`. * Adding optional syntax to the SDL, and consuming that in `buildASTSchema` and `extendSchema` as well as in `schemaPrinter`. * Adding a case to `findBreakingChanges` which looks for a scalar's ofType changing. --- .../__tests__/schema-kitchen-sink.graphql | 2 ++ src/language/__tests__/schema-parser-test.js | 1 + src/language/__tests__/schema-printer-test.js | 2 ++ src/language/ast.js | 1 + src/language/parser.js | 26 ++++++++++++-- src/language/printer.js | 4 +-- src/language/visitor.js | 2 +- src/type/__tests__/introspection-test.js | 4 ++- src/type/definition.js | 14 ++++++-- src/type/introspection.js | 8 +++-- src/type/validate.js | 22 +++++++++++- src/utilities/__tests__/schemaPrinter-test.js | 35 +++++++++++++------ src/utilities/buildASTSchema.js | 4 +++ src/utilities/buildClientSchema.js | 4 +++ src/utilities/findBreakingChanges.js | 12 +++++++ src/utilities/introspectionQuery.js | 1 + src/utilities/schemaPrinter.js | 3 +- 17 files changed, 122 insertions(+), 23 deletions(-) diff --git a/src/language/__tests__/schema-kitchen-sink.graphql b/src/language/__tests__/schema-kitchen-sink.graphql index 1c7b5c3b30..aebc5a7d54 100644 --- a/src/language/__tests__/schema-kitchen-sink.graphql +++ b/src/language/__tests__/schema-kitchen-sink.graphql @@ -75,6 +75,8 @@ scalar CustomScalar scalar AnnotatedScalar @onScalar +scalar StringEncodedCustomScalar as String + extend scalar CustomScalar @onScalar enum Site { diff --git a/src/language/__tests__/schema-parser-test.js b/src/language/__tests__/schema-parser-test.js index d7e7ff943d..c59368915b 100644 --- a/src/language/__tests__/schema-parser-test.js +++ b/src/language/__tests__/schema-parser-test.js @@ -754,6 +754,7 @@ type Hello { kind: 'ScalarTypeDefinition', name: nameNode('Hello', { start: 7, end: 12 }), description: undefined, + type: undefined, directives: [], loc: { start: 0, end: 12 }, }, diff --git a/src/language/__tests__/schema-printer-test.js b/src/language/__tests__/schema-printer-test.js index 313d729d59..2651dedbda 100644 --- a/src/language/__tests__/schema-printer-test.js +++ b/src/language/__tests__/schema-printer-test.js @@ -119,6 +119,8 @@ describe('Printer: SDL document', () => { scalar AnnotatedScalar @onScalar + scalar StringEncodedCustomScalar as String + extend scalar CustomScalar @onScalar enum Site { diff --git a/src/language/ast.js b/src/language/ast.js index 22169c327d..b8fdf4c522 100644 --- a/src/language/ast.js +++ b/src/language/ast.js @@ -423,6 +423,7 @@ export type ScalarTypeDefinitionNode = { +loc?: Location, +description?: StringValueNode, +name: NameNode, + +type?: NamedTypeNode, +directives?: $ReadOnlyArray, }; diff --git a/src/language/parser.js b/src/language/parser.js index 37ba2e1fb3..521a773375 100644 --- a/src/language/parser.js +++ b/src/language/parser.js @@ -864,18 +864,26 @@ function parseOperationTypeDefinition( } /** - * ScalarTypeDefinition : Description? scalar Name Directives[Const]? + * ScalarTypeDefinition : + * - Description? scalar Name ScalarOfType? Directives[Const]? + * + * ScalarOfType : as NamedType */ function parseScalarTypeDefinition(lexer: Lexer<*>): ScalarTypeDefinitionNode { const start = lexer.token; const description = parseDescription(lexer); expectKeyword(lexer, 'scalar'); const name = parseName(lexer); + let type; + if (skipKeyword(lexer, 'as')) { + type = parseNamedType(lexer); + } const directives = parseDirectives(lexer, true); return { kind: Kind.SCALAR_TYPE_DEFINITION, description, name, + type, directives, loc: loc(lexer, start), }; @@ -1512,10 +1520,24 @@ function expect(lexer: Lexer<*>, kind: TokenKindEnum): Token { } /** - * If the next token is a keyword with the given value, return that token after + * If the next token is a keyword with the given value, return true after * advancing the lexer. Otherwise, do not change the parser state and return * false. */ +function skipKeyword(lexer: Lexer<*>, value: string): boolean { + const token = lexer.token; + const match = token.kind === TokenKind.NAME && token.value === value; + if (match) { + lexer.advance(); + } + return match; +} + +/** + * If the next token is a keyword with the given value, return that token after + * advancing the lexer. Otherwise, do not change the parser state and throw + * an error. + */ function expectKeyword(lexer: Lexer<*>, value: string): Token { const token = lexer.token; if (token.kind === TokenKind.NAME && token.value === value) { diff --git a/src/language/printer.js b/src/language/printer.js index 5254f46ee5..0e38f16580 100644 --- a/src/language/printer.js +++ b/src/language/printer.js @@ -113,8 +113,8 @@ const printDocASTReducer = { OperationTypeDefinition: ({ operation, type }) => operation + ': ' + type, - ScalarTypeDefinition: addDescription(({ name, directives }) => - join(['scalar', name, join(directives, ' ')], ' '), + ScalarTypeDefinition: addDescription(({ name, type, directives }) => + join(['scalar', name, wrap('as ', type), join(directives, ' ')], ' '), ), ObjectTypeDefinition: addDescription( diff --git a/src/language/visitor.js b/src/language/visitor.js index ab5839ada7..16b549ddb0 100644 --- a/src/language/visitor.js +++ b/src/language/visitor.js @@ -102,7 +102,7 @@ export const QueryDocumentKeys = { SchemaDefinition: ['directives', 'operationTypes'], OperationTypeDefinition: ['type'], - ScalarTypeDefinition: ['description', 'name', 'directives'], + ScalarTypeDefinition: ['description', 'name', 'type', 'directives'], ObjectTypeDefinition: [ 'description', 'name', diff --git a/src/type/__tests__/introspection-test.js b/src/type/__tests__/introspection-test.js index 5e7cc46038..3f971546b5 100644 --- a/src/type/__tests__/introspection-test.js +++ b/src/type/__tests__/introspection-test.js @@ -1355,7 +1355,9 @@ describe('Introspection', () => { 'An enum describing what kind of type a given `__Type` is.', enumValues: [ { - description: 'Indicates this type is a scalar.', + description: + 'Indicates this type is a scalar. ' + + '`ofType` may represent how this scalar is serialized.', name: 'SCALAR', }, { diff --git a/src/type/definition.js b/src/type/definition.js index 675092d0a7..bfb57d4c49 100644 --- a/src/type/definition.js +++ b/src/type/definition.js @@ -538,6 +538,7 @@ export class GraphQLScalarType { serialize: GraphQLScalarSerializer<*>; parseValue: GraphQLScalarValueParser<*>; parseLiteral: GraphQLScalarLiteralParser<*>; + ofType: ?GraphQLScalarType; astNode: ?ScalarTypeDefinitionNode; extensionASTNodes: ?$ReadOnlyArray; @@ -545,10 +546,18 @@ export class GraphQLScalarType { this.name = config.name; this.description = config.description; this.serialize = config.serialize; - this.parseValue = config.parseValue || (value => value); - this.parseLiteral = config.parseLiteral || valueFromASTUntyped; + this.ofType = config.ofType || null; + this.parseValue = + config.parseValue || + (this.ofType && this.ofType.parseValue) || + (value => value); + this.parseLiteral = + config.parseLiteral || + (this.ofType && this.ofType.parseLiteral) || + valueFromASTUntyped; this.astNode = config.astNode; this.extensionASTNodes = config.extensionASTNodes; + invariant(typeof config.name === 'string', 'Must provide name.'); invariant( typeof config.serialize === 'function', @@ -591,6 +600,7 @@ export type GraphQLScalarTypeConfig = {| parseValue?: GraphQLScalarValueParser, // Parses an externally provided literal value to use as an input. parseLiteral?: GraphQLScalarLiteralParser, + ofType?: ?GraphQLScalarType, astNode?: ?ScalarTypeDefinitionNode, extensionASTNodes?: ?$ReadOnlyArray, |}; diff --git a/src/type/introspection.js b/src/type/introspection.js index 63630d95c9..f1b3bd98ac 100644 --- a/src/type/introspection.js +++ b/src/type/introspection.js @@ -192,8 +192,8 @@ export const __Type = new GraphQLObjectType({ 'The fundamental unit of any GraphQL Schema is the type. There are ' + 'many kinds of types in GraphQL as represented by the `__TypeKind` enum.' + '\n\nDepending on the kind of a type, certain fields describe ' + - 'information about that type. Scalar types provide no information ' + - 'beyond a name and description, while Enum types provide their values. ' + + 'information about that type. Scalar types provide a name, description ' + + 'and how they serialize, while Enum types provide their possible values. ' + 'Object and Interface types provide the fields they describe. Abstract ' + 'types, Union and Interface, provide the Object types possible ' + 'at runtime. List and NonNull types compose other types.', @@ -399,7 +399,9 @@ export const __TypeKind = new GraphQLEnumType({ values: { SCALAR: { value: TypeKind.SCALAR, - description: 'Indicates this type is a scalar.', + description: + 'Indicates this type is a scalar. ' + + '`ofType` may represent how this scalar is serialized.', }, OBJECT: { value: TypeKind.OBJECT, diff --git a/src/type/validate.js b/src/type/validate.js index 22f3423d58..db36cb9f57 100644 --- a/src/type/validate.js +++ b/src/type/validate.js @@ -8,6 +8,7 @@ */ import { + isScalarType, isObjectType, isInterfaceType, isUnionType, @@ -19,6 +20,7 @@ import { isRequiredArgument, } from './definition'; import type { + GraphQLScalarType, GraphQLObjectType, GraphQLInterfaceType, GraphQLUnionType, @@ -28,6 +30,7 @@ import type { import { isDirective } from './directives'; import type { GraphQLDirective } from './directives'; import { isIntrospectionType } from './introspection'; +import { isSpecifiedScalarType } from './scalars'; import { isSchema } from './schema'; import type { GraphQLSchema } from './schema'; import inspect from '../jsutils/inspect'; @@ -244,7 +247,10 @@ function validateTypes(context: SchemaValidationContext): void { validateName(context, type); } - if (isObjectType(type)) { + if (isScalarType(type)) { + // Ensure Scalars can serialize as expected. + validateScalarSerialization(context, type); + } else if (isObjectType(type)) { // Ensure fields are valid validateFields(context, type); @@ -266,6 +272,20 @@ function validateTypes(context: SchemaValidationContext): void { } } +function validateScalarSerialization( + context: SchemaValidationContext, + scalarType: GraphQLScalarType, +): void { + if (scalarType.ofType && !isSpecifiedScalarType(scalarType.ofType)) { + context.reportError( + `Scalar type ${scalarType.name} may only be described in terms of a ` + + `spec-defined scalar type. However ${String(scalarType.ofType)} is ` + + 'not a built-in scalar type.', + scalarType.astNode && scalarType.astNode.type, + ); + } +} + function validateFields( context: SchemaValidationContext, type: GraphQLObjectType | GraphQLInterfaceType, diff --git a/src/utilities/__tests__/schemaPrinter-test.js b/src/utilities/__tests__/schemaPrinter-test.js index cc2177eed2..41452d5589 100644 --- a/src/utilities/__tests__/schemaPrinter-test.js +++ b/src/utilities/__tests__/schemaPrinter-test.js @@ -461,8 +461,17 @@ describe('Type System Printer', () => { }); it('Custom Scalar', () => { + const EvenType = new GraphQLScalarType({ + name: 'Even', + ofType: GraphQLInt, + serialize(value) { + return value % 2 === 1 ? value : null; + }, + }); + const OddType = new GraphQLScalarType({ name: 'Odd', + // No ofType in this test case. serialize(value) { return value % 2 === 1 ? value : null; }, @@ -471,6 +480,7 @@ describe('Type System Printer', () => { const Query = new GraphQLObjectType({ name: 'Query', fields: { + even: { type: EvenType }, odd: { type: OddType }, }, }); @@ -478,9 +488,12 @@ describe('Type System Printer', () => { const Schema = new GraphQLSchema({ query: Query }); const output = printForTest(Schema); expect(output).to.equal(dedent` + scalar Even as Int + scalar Odd type Query { + even: Even odd: Odd } `); @@ -785,10 +798,10 @@ describe('Type System Printer', () => { types in GraphQL as represented by the \`__TypeKind\` enum. Depending on the kind of a type, certain fields describe information about that - type. Scalar types provide no information beyond a name and description, while - Enum types provide their values. Object and Interface types provide the fields - they describe. Abstract types, Union and Interface, provide the Object types - possible at runtime. List and NonNull types compose other types. + type. Scalar types provide a name, description and how they serialize, while + Enum types provide their possible values. Object and Interface types provide the + fields they describe. Abstract types, Union and Interface, provide the Object + types possible at runtime. List and NonNull types compose other types. """ type __Type { kind: __TypeKind! @@ -804,7 +817,9 @@ describe('Type System Printer', () => { """An enum describing what kind of type a given \`__Type\` is.""" enum __TypeKind { - """Indicates this type is a scalar.""" + """ + Indicates this type is a scalar. \`ofType\` may represent how this scalar is serialized. + """ SCALAR """ @@ -1001,10 +1016,10 @@ describe('Type System Printer', () => { # types in GraphQL as represented by the \`__TypeKind\` enum. # # Depending on the kind of a type, certain fields describe information about that - # type. Scalar types provide no information beyond a name and description, while - # Enum types provide their values. Object and Interface types provide the fields - # they describe. Abstract types, Union and Interface, provide the Object types - # possible at runtime. List and NonNull types compose other types. + # type. Scalar types provide a name, description and how they serialize, while + # Enum types provide their possible values. Object and Interface types provide the + # fields they describe. Abstract types, Union and Interface, provide the Object + # types possible at runtime. List and NonNull types compose other types. type __Type { kind: __TypeKind! name: String @@ -1019,7 +1034,7 @@ describe('Type System Printer', () => { # An enum describing what kind of type a given \`__Type\` is. enum __TypeKind { - # Indicates this type is a scalar. + # Indicates this type is a scalar. \`ofType\` may represent how this scalar is serialized. SCALAR # Indicates this type is an object. \`fields\` and \`interfaces\` are valid fields. diff --git a/src/utilities/buildASTSchema.js b/src/utilities/buildASTSchema.js index 2319e740b3..ecf7b604a0 100644 --- a/src/utilities/buildASTSchema.js +++ b/src/utilities/buildASTSchema.js @@ -421,6 +421,10 @@ export class ASTDefinitionBuilder { return new GraphQLScalarType({ name: def.name.value, description: getDescription(def, this._options), + // Note: While this could make assertions to get the correctly typed + // values below, that would throw immediately while type system + // validation with validateSchema() will produce more actionable results. + ofType: def.type && (this.buildType(def.type): any), astNode: def, serialize: value => value, }); diff --git a/src/utilities/buildClientSchema.js b/src/utilities/buildClientSchema.js index 531036dca3..3fe9a2c539 100644 --- a/src/utilities/buildClientSchema.js +++ b/src/utilities/buildClientSchema.js @@ -202,9 +202,13 @@ export function buildClientSchema( function buildScalarDef( scalarIntrospection: IntrospectionScalarType, ): GraphQLScalarType { + const ofType = scalarIntrospection.ofType + ? (getType(scalarIntrospection.ofType): any) + : undefined; return new GraphQLScalarType({ name: scalarIntrospection.name, description: scalarIntrospection.description, + ofType, serialize: value => value, }); } diff --git a/src/utilities/findBreakingChanges.js b/src/utilities/findBreakingChanges.js index 9f7962138f..1b27e0775b 100644 --- a/src/utilities/findBreakingChanges.js +++ b/src/utilities/findBreakingChanges.js @@ -163,6 +163,18 @@ export function findTypesThatChangedKind( `${typeName} changed from ` + `${typeKindName(oldType)} to ${typeKindName(newType)}.`, }); + } else if (isScalarType(oldType) && isScalarType(newType)) { + const oldOfType = oldType.ofType; + const newOfType = newType.ofType; + if (oldOfType && newOfType && oldOfType !== newOfType) { + breakingChanges.push({ + type: BreakingChangeType.TYPE_CHANGED_KIND, + description: + `${typeName} changed from ` + + `${typeKindName(oldType)} serialized as ${oldOfType.name} ` + + `to ${typeKindName(newType)} serialized as ${newOfType.name}.`, + }); + } } } return breakingChanges; diff --git a/src/utilities/introspectionQuery.js b/src/utilities/introspectionQuery.js index b6ebe58485..1ad3a4a7e4 100644 --- a/src/utilities/introspectionQuery.js +++ b/src/utilities/introspectionQuery.js @@ -155,6 +155,7 @@ export type IntrospectionScalarType = { +kind: 'SCALAR', +name: string, +description?: ?string, + +ofType?: ?IntrospectionNamedTypeRef, }; export type IntrospectionObjectType = { diff --git a/src/utilities/schemaPrinter.js b/src/utilities/schemaPrinter.js index aa9b01c096..dd6665c650 100644 --- a/src/utilities/schemaPrinter.js +++ b/src/utilities/schemaPrinter.js @@ -180,7 +180,8 @@ export function printType(type: GraphQLNamedType, options?: Options): string { } function printScalar(type: GraphQLScalarType, options): string { - return printDescription(options, type) + `scalar ${type.name}`; + const ofType = type.ofType ? ` as ${type.ofType.name}` : ''; + return printDescription(options, type) + `scalar ${type.name}${ofType}`; } function printObject(type: GraphQLObjectType, options): string {