Skip to content

Commit

Permalink
RFC: Define custom scalars in terms of built-in scalars.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
leebyron committed Jun 16, 2017
1 parent fa44c33 commit 0a2aec0
Show file tree
Hide file tree
Showing 17 changed files with 106 additions and 20 deletions.
2 changes: 2 additions & 0 deletions src/language/__tests__/schema-kitchen-sink.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ union AnnotatedUnionTwo @onUnion = | A | B

scalar CustomScalar

scalar StringEncodedCustomScalar = String

scalar AnnotatedScalar @onScalar

enum Site {
Expand Down
1 change: 1 addition & 0 deletions src/language/__tests__/schema-parser-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -530,6 +530,7 @@ type Hello {
{
kind: 'ScalarTypeDefinition',
name: nameNode('Hello', { start: 7, end: 12 }),
type: null,
directives: [],
loc: { start: 0, end: 12 },
}
Expand Down
2 changes: 2 additions & 0 deletions src/language/__tests__/schema-printer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,8 @@ union AnnotatedUnionTwo @onUnion = A | B
scalar CustomScalar
scalar StringEncodedCustomScalar = String
scalar AnnotatedScalar @onScalar
enum Site {
Expand Down
1 change: 1 addition & 0 deletions src/language/ast.js
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,7 @@ export type ScalarTypeDefinitionNode = {
kind: 'ScalarTypeDefinition';
loc?: Location;
name: NameNode;
type?: ?NamedTypeNode;
directives?: ?Array<DirectiveNode>;
};

Expand Down
5 changes: 4 additions & 1 deletion src/language/parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -784,16 +784,19 @@ function parseOperationTypeDefinition(
}

/**
* ScalarTypeDefinition : scalar Name Directives?
* ScalarTypeDefinition : scalar Name ScalarOfType? Directives?
* ScalarOfType : = NamedType
*/
function parseScalarTypeDefinition(lexer: Lexer<*>): ScalarTypeDefinitionNode {
const start = lexer.token;
expectKeyword(lexer, 'scalar');
const name = parseName(lexer);
const type = skip(lexer, TokenKind.EQUALS) ? parseNamedType(lexer) : null;
const directives = parseDirectives(lexer);
return {
kind: SCALAR_TYPE_DEFINITION,
name,
type,
directives,
loc: loc(lexer, start),
};
Expand Down
4 changes: 2 additions & 2 deletions src/language/printer.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,8 @@ const printDocASTReducer = {
OperationTypeDefinition: ({ operation, type }) =>
operation + ': ' + type,

ScalarTypeDefinition: ({ name, directives }) =>
join([ 'scalar', name, join(directives, ' ') ], ' '),
ScalarTypeDefinition: ({ name, type, directives }) =>
join([ 'scalar', name, wrap('= ', type), join(directives, ' ') ], ' '),

ObjectTypeDefinition: ({ name, interfaces, directives, fields }) =>
join([
Expand Down
2 changes: 1 addition & 1 deletion src/language/visitor.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export const QueryDocumentKeys = {
SchemaDefinition: [ 'directives', 'operationTypes' ],
OperationTypeDefinition: [ 'type' ],

ScalarTypeDefinition: [ 'name', 'directives' ],
ScalarTypeDefinition: [ 'name', 'type', 'directives' ],
ObjectTypeDefinition: [ 'name', 'interfaces', 'directives', 'fields' ],
FieldDefinition: [ 'name', 'arguments', 'type', 'directives' ],
InputValueDefinition: [ 'name', 'type', 'defaultValue', 'directives' ],
Expand Down
3 changes: 2 additions & 1 deletion src/type/__tests__/introspection-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1313,7 +1313,8 @@ 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` is a valid field.',
name: 'SCALAR'
},
{
Expand Down
28 changes: 23 additions & 5 deletions src/type/definition.js
Original file line number Diff line number Diff line change
Expand Up @@ -294,13 +294,27 @@ function resolveThunk<T>(thunk: Thunk<T>): T {
export class GraphQLScalarType {
name: string;
description: ?string;
ofType: ?GraphQLScalarType;

_scalarConfig: GraphQLScalarTypeConfig<*, *>;

constructor(config: GraphQLScalarTypeConfig<*, *>): void {
assertValidName(config.name);
this.name = config.name;
this.description = config.description;
this.ofType = config.ofType || null;
if (this.ofType) {
const ofTypeName = this.ofType.name;
invariant(
ofTypeName === 'String' ||
ofTypeName === 'Int' ||
ofTypeName === 'Float' ||
ofTypeName === 'Boolean' ||
ofTypeName === 'ID',
`${this.name} may only be described in terms of a built-in scalar ` +
`type. However ${ofTypeName} is not a built-in scalar type.`
);
}
invariant(
typeof config.serialize === 'function',
`${this.name} must provide "serialize" function. If this custom Scalar ` +
Expand All @@ -321,7 +335,8 @@ export class GraphQLScalarType {
// Serializes an internal value to include in a response.
serialize(value: mixed): mixed {
const serializer = this._scalarConfig.serialize;
return serializer(value);
const serialized = serializer(value);
return this.ofType ? this.ofType.serialize(serialized) : serialized;
}

// Determines if an internal value is valid for this type.
Expand All @@ -332,7 +347,8 @@ export class GraphQLScalarType {

// Parses an externally provided value to use as an input.
parseValue(value: mixed): mixed {
const parser = this._scalarConfig.parseValue;
const parser = this._scalarConfig.parseValue ||
this.ofType && this.ofType.parseValue;
return parser && !isNullish(value) ? parser(value) : undefined;
}

Expand All @@ -344,7 +360,8 @@ export class GraphQLScalarType {

// Parses an externally provided literal value to use as an input.
parseLiteral(valueNode: ValueNode): mixed {
const parser = this._scalarConfig.parseLiteral;
const parser = this._scalarConfig.parseLiteral ||
this.ofType && this.ofType.parseLiteral;
return parser ? parser(valueNode) : undefined;
}

Expand All @@ -364,9 +381,10 @@ GraphQLScalarType.prototype.toJSON =
export type GraphQLScalarTypeConfig<TInternal, TExternal> = {
name: string;
description?: ?string;
ofType?: ?GraphQLScalarType;
serialize: (value: mixed) => ?TExternal;
parseValue?: (value: mixed) => ?TInternal;
parseLiteral?: (valueNode: ValueNode) => ?TInternal;
parseValue?: ?(value: mixed) => ?TInternal;
parseLiteral?: ?(valueNode: ValueNode) => ?TInternal;
};


Expand Down
3 changes: 2 additions & 1 deletion src/type/introspection.js
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,8 @@ 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` is a valid field.'
},
OBJECT: {
value: TypeKind.OBJECT,
Expand Down
15 changes: 14 additions & 1 deletion src/utilities/__tests__/schemaPrinter-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -540,8 +540,17 @@ type Root {
});

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;
}
Expand All @@ -550,6 +559,7 @@ type Root {
const Root = new GraphQLObjectType({
name: 'Root',
fields: {
even: { type: EvenType },
odd: { type: OddType },
},
});
Expand All @@ -561,9 +571,12 @@ schema {
query: Root
}
scalar Even = Int
scalar Odd
type Root {
even: Even
odd: Odd
}
`
Expand Down Expand Up @@ -788,7 +801,7 @@ type __Type {
# 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\` is a valid field.
SCALAR
# Indicates this type is an object. \`fields\` and \`interfaces\` are valid fields.
Expand Down
14 changes: 11 additions & 3 deletions src/utilities/buildASTSchema.js
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,12 @@ export function buildASTSchema(ast: DocumentNode): GraphQLSchema {
return type;
}

function produceScalarType(typeNode: TypeNode): GraphQLScalarType {
const type = produceType(typeNode);
invariant(type instanceof GraphQLScalarType, 'Expected Scalar type.');
return type;
}

function typeDefNamed(typeName: string): GraphQLNamedType {
if (innerTypeMap[typeName]) {
return innerTypeMap[typeName];
Expand Down Expand Up @@ -434,16 +440,18 @@ export function buildASTSchema(ast: DocumentNode): GraphQLSchema {
}

function makeScalarDef(def: ScalarTypeDefinitionNode) {
const ofType = def.type && produceScalarType(def.type);
return new GraphQLScalarType({
name: def.name.value,
description: getDescription(def),
serialize: () => null,
ofType,
serialize: id => id,
// Note: validation calls the parse functions to determine if a
// literal value is correct. Returning null would cause use of custom
// scalars to always fail validation. Returning false causes them to
// always pass validation.
parseValue: () => false,
parseLiteral: () => false,
parseValue: ofType ? null : () => false,
parseLiteral: ofType ? null : () => false,
});
}

Expand Down
16 changes: 14 additions & 2 deletions src/utilities/buildClientSchema.js
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,14 @@ export function buildClientSchema(
return type;
}

function getScalarType(typeRef: IntrospectionTypeRef): GraphQLScalarType {
const type = getType(typeRef);
invariant(
type instanceof GraphQLScalarType,
'Introspection must provide scalar type for custom scalars.'
);
return type;
}

// Given a type's introspection result, construct the correct
// GraphQLType instance.
Expand Down Expand Up @@ -223,16 +231,20 @@ export function buildClientSchema(
function buildScalarDef(
scalarIntrospection: IntrospectionScalarType
): GraphQLScalarType {
const ofType = scalarIntrospection.ofType ?
getScalarType(scalarIntrospection.ofType) :
undefined;
return new GraphQLScalarType({
name: scalarIntrospection.name,
description: scalarIntrospection.description,
ofType,
serialize: id => id,
// Note: validation calls the parse functions to determine if a
// literal value is correct. Returning null would cause use of custom
// scalars to always fail validation. Returning false causes them to
// always pass validation.
parseValue: () => false,
parseLiteral: () => false,
parseValue: ofType ? null : () => false,
parseLiteral: ofType ? null : () => false,
});
}

Expand Down
12 changes: 10 additions & 2 deletions src/utilities/extendSchema.js
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,12 @@ export function extendSchema(
return type;
}

function getScalarTypeFromAST(node: NamedTypeNode): GraphQLScalarType {
const type = getTypeFromAST(node);
invariant(type instanceof GraphQLScalarType, 'Must be Scalar type.');
return type;
}

function getInputTypeFromAST(node: NamedTypeNode): GraphQLInputType {
return assertInputType(getTypeFromAST(node));
}
Expand Down Expand Up @@ -478,16 +484,18 @@ export function extendSchema(
}

function buildScalarType(typeNode: ScalarTypeDefinitionNode) {
const ofType = typeNode.type && getScalarTypeFromAST(typeNode.type);
return new GraphQLScalarType({
name: typeNode.name.value,
description: getDescription(typeNode),
ofType,
serialize: id => id,
// Note: validation calls the parse functions to determine if a
// literal value is correct. Returning null would cause use of custom
// scalars to always fail validation. Returning false causes them to
// always pass validation.
parseValue: () => false,
parseLiteral: () => false,
parseValue: ofType ? null : () => false,
parseLiteral: ofType ? null : () => false,
});
}

Expand Down
14 changes: 14 additions & 0 deletions src/utilities/findBreakingChanges.js
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,20 @@ export function findTypesThatChangedKind(
description: `${typeName} changed from ` +
`${typeKindName(oldType)} to ${typeKindName(newType)}.`
});
} else if (
oldType instanceof GraphQLScalarType &&
newType instanceof GraphQLScalarType
) {
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;
Expand Down
1 change: 1 addition & 0 deletions src/utilities/introspectionQuery.js
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ export type IntrospectionScalarType = {
kind: 'SCALAR';
name: string;
description: ?string;
ofType: ?IntrospectionNamedTypeRef;
};

export type IntrospectionObjectType = {
Expand Down
3 changes: 2 additions & 1 deletion src/utilities/schemaPrinter.js
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,9 @@ export function printType(type: GraphQLType): string {
}

function printScalar(type: GraphQLScalarType): string {
const ofType = type.ofType ? ` = ${type.ofType.name}` : '';
return printDescription(type) +
`scalar ${type.name}`;
`scalar ${type.name}${ofType}`;
}

function printObject(type: GraphQLObjectType): string {
Expand Down

0 comments on commit 0a2aec0

Please sign in to comment.