From 780cec2b01091465a5492f7ba84791906ae0bdd1 Mon Sep 17 00:00:00 2001 From: Lee Byron Date: Mon, 10 May 2021 11:44:12 -0700 Subject: [PATCH] Preserve defaultValue literals Fixes #3051 --- src/execution/values.js | 15 +++-- src/index.d.ts | 1 + src/index.js | 1 + src/type/__tests__/definition-test.js | 61 +++++++++++++++++++ src/type/__tests__/predicate-test.js | 31 ++++------ src/type/definition.d.ts | 7 ++- src/type/definition.js | 27 +++++++- src/type/index.d.ts | 1 + src/type/index.js | 1 + src/type/introspection.js | 10 ++- src/utilities/TypeInfo.d.ts | 3 +- src/utilities/TypeInfo.js | 11 ++-- src/utilities/__tests__/astFromValue-test.js | 2 + .../__tests__/buildClientSchema-test.js | 23 +++++++ .../__tests__/coerceInputValue-test.js | 35 ++++++++++- src/utilities/astFromValue.js | 12 ++-- src/utilities/buildClientSchema.js | 16 ++--- src/utilities/coerceInputValue.d.ts | 10 ++- src/utilities/coerceInputValue.js | 42 +++++++++++-- src/utilities/extendSchema.js | 6 +- src/utilities/findBreakingChanges.js | 22 ++++--- src/utilities/printSchema.js | 9 ++- src/validation/ValidationContext.d.ts | 3 +- src/validation/ValidationContext.js | 3 +- .../rules/VariablesInAllowedPositionRule.js | 7 ++- 25 files changed, 278 insertions(+), 81 deletions(-) diff --git a/src/execution/values.js b/src/execution/values.js index b738e745efd..78cb4176e0c 100644 --- a/src/execution/values.js +++ b/src/execution/values.js @@ -22,6 +22,7 @@ import { typeFromAST } from '../utilities/typeFromAST'; import { coerceInputValue, coerceInputLiteral, + coerceDefaultValue, } from '../utilities/coerceInputValue'; type CoercedVariableValues = @@ -178,8 +179,11 @@ export function getArgumentValues( const argumentNode = argNodeMap[name]; if (!argumentNode) { - if (argDef.defaultValue !== undefined) { - coercedValues[name] = argDef.defaultValue; + if (argDef.defaultValue) { + coercedValues[name] = coerceDefaultValue( + argDef.defaultValue, + argDef.type, + ); } else if (isNonNullType(argType)) { throw new GraphQLError( `Argument "${name}" of required type "${inspect(argType)}" ` + @@ -199,8 +203,11 @@ export function getArgumentValues( variableValues == null || !hasOwnProperty(variableValues, variableName) ) { - if (argDef.defaultValue !== undefined) { - coercedValues[name] = argDef.defaultValue; + if (argDef.defaultValue) { + coercedValues[name] = coerceDefaultValue( + argDef.defaultValue, + argDef.type, + ); } else if (isNonNullType(argType)) { throw new GraphQLError( `Argument "${name}" of required type "${inspect(argType)}" ` + diff --git a/src/index.d.ts b/src/index.d.ts index d2a0c53ea41..10f2b929643 100644 --- a/src/index.d.ts +++ b/src/index.d.ts @@ -150,6 +150,7 @@ export { GraphQLArgumentConfig, GraphQLArgumentExtensions, GraphQLInputValue, + GraphQLDefaultValueUsage, GraphQLInputValueConfig, GraphQLEnumTypeConfig, GraphQLEnumTypeExtensions, diff --git a/src/index.js b/src/index.js index 6d74453d786..b51ea30dd4b 100644 --- a/src/index.js +++ b/src/index.js @@ -146,6 +146,7 @@ export type { GraphQLArgument, GraphQLArgumentConfig, GraphQLInputValue, + GraphQLDefaultValueUsage, GraphQLInputValueConfig, GraphQLEnumTypeConfig, GraphQLEnumValue, diff --git a/src/type/__tests__/definition-test.js b/src/type/__tests__/definition-test.js index 809e7007d34..68bb7bee527 100644 --- a/src/type/__tests__/definition-test.js +++ b/src/type/__tests__/definition-test.js @@ -818,6 +818,67 @@ describe('Type System: Input Objects', () => { ); }); }); + + describe('Input Object fields may have default values', () => { + it('accepts an Input Object type with a default value', () => { + const inputObjType = new GraphQLInputObjectType({ + name: 'SomeInputObject', + fields: { + f: { type: ScalarType, defaultValue: 3 }, + }, + }); + expect(inputObjType.getFields()).to.deep.equal({ + f: { + name: 'f', + description: undefined, + type: ScalarType, + defaultValue: { value: 3 }, + deprecationReason: undefined, + extensions: undefined, + astNode: undefined, + }, + }); + }); + + it('accepts an Input Object type with a default value literal', () => { + const inputObjType = new GraphQLInputObjectType({ + name: 'SomeInputObject', + fields: { + f: { + type: ScalarType, + defaultValueLiteral: { kind: 'IntValue', value: '3' }, + }, + }, + }); + expect(inputObjType.getFields()).to.deep.equal({ + f: { + name: 'f', + description: undefined, + type: ScalarType, + defaultValue: { literal: { kind: 'IntValue', value: '3' } }, + deprecationReason: undefined, + extensions: undefined, + astNode: undefined, + }, + }); + }); + + it('rejects an Input Object type with potentially conflicting default values', () => { + const inputObjType = new GraphQLInputObjectType({ + name: 'SomeInputObject', + fields: { + f: { + type: ScalarType, + defaultValue: 3, + defaultValueLiteral: { kind: 'IntValue', value: '3' }, + }, + }, + }); + expect(() => inputObjType.getFields()).to.throw( + 'f has both a defaultValue and a defaultValueLiteral property, but only one must be provided.', + ); + }); + }); }); describe('Type System: List', () => { diff --git a/src/type/__tests__/predicate-test.js b/src/type/__tests__/predicate-test.js index 899752864cd..2ac688b4c8b 100644 --- a/src/type/__tests__/predicate-test.js +++ b/src/type/__tests__/predicate-test.js @@ -69,6 +69,7 @@ import { assertNamedType, getNullableType, getNamedType, + defineInputValue, } from '../definition'; const ObjectType = new GraphQLObjectType({ name: 'Object', fields: {} }); @@ -562,19 +563,14 @@ describe('Type predicates', () => { }); describe('isRequiredInput', () => { - function buildArg(config: {| + function buildArg({ + type, + defaultValue, + }: {| type: GraphQLInputType, defaultValue?: mixed, |}): GraphQLArgument { - return { - name: 'someArg', - type: config.type, - description: undefined, - defaultValue: config.defaultValue, - deprecationReason: null, - extensions: undefined, - astNode: undefined, - }; + return defineInputValue({ type, defaultValue }, 'someArg'); } it('returns true for required arguments', () => { @@ -608,19 +604,14 @@ describe('Type predicates', () => { expect(isRequiredInput(optArg4)).to.equal(false); }); - function buildInputField(config: {| + function buildInputField({ + type, + defaultValue, + }: {| type: GraphQLInputType, defaultValue?: mixed, |}): GraphQLInputField { - return { - name: 'someInputField', - type: config.type, - description: undefined, - defaultValue: config.defaultValue, - deprecationReason: null, - extensions: undefined, - astNode: undefined, - }; + return defineInputValue({ type, defaultValue }, 'someInputField'); } it('returns true for required input field', () => { diff --git a/src/type/definition.d.ts b/src/type/definition.d.ts index f07202d91af..b690c883207 100644 --- a/src/type/definition.d.ts +++ b/src/type/definition.d.ts @@ -23,6 +23,7 @@ import { FieldNode, FragmentDefinitionNode, ValueNode, + ConstValueNode, ScalarTypeExtensionNode, UnionTypeExtensionNode, EnumTypeExtensionNode, @@ -575,12 +576,16 @@ export interface GraphQLInputValue { name: string; description: Maybe; type: GraphQLInputType; - defaultValue: unknown; + defaultValue: Maybe; deprecationReason: Maybe; extensions: Maybe>; astNode: Maybe; } +export type GraphQLDefaultValueUsage = + | { value: unknown; literal?: never } + | { literal: ConstValueNode; value?: never }; + export interface GraphQLInputValueConfig { description?: Maybe; type: GraphQLInputType; diff --git a/src/type/definition.js b/src/type/definition.js index c16433d5b31..795c41dd190 100644 --- a/src/type/definition.js +++ b/src/type/definition.js @@ -41,6 +41,7 @@ import type { FieldNode, FragmentDefinitionNode, ValueNode, + ConstValueNode, } from '../language/ast'; import { valueFromASTUntyped } from '../utilities/valueFromASTUntyped'; @@ -971,11 +972,21 @@ export function defineInputValue( !('resolve' in config), `${name} has a resolve property, but inputs cannot define resolvers.`, ); + let defaultValue; + if (config.defaultValue !== undefined || config.defaultValueLiteral) { + devAssert( + config.defaultValue === undefined || !config.defaultValueLiteral, + `${name} has both a defaultValue and a defaultValueLiteral property, but only one must be provided.`, + ); + defaultValue = config.defaultValueLiteral + ? { literal: config.defaultValueLiteral } + : { value: config.defaultValue }; + } return { name, description: config.description, type: config.type, - defaultValue: config.defaultValue, + defaultValue, deprecationReason: config.deprecationReason, extensions: config.extensions && toObjMap(config.extensions), astNode: config.astNode, @@ -991,7 +1002,12 @@ export function inputValueToConfig( return { description: inputValue.description, type: inputValue.type, - defaultValue: inputValue.defaultValue, + defaultValue: inputValue.defaultValue?.literal + ? undefined + : inputValue.defaultValue?.value, + defaultValueLiteral: inputValue.defaultValue?.literal + ? inputValue.defaultValue.literal + : undefined, deprecationReason: inputValue.deprecationReason, extensions: inputValue.extensions, astNode: inputValue.astNode, @@ -1002,16 +1018,21 @@ export type GraphQLInputValue = {| name: string, description: ?string, type: GraphQLInputType, - defaultValue: mixed, + defaultValue: ?GraphQLDefaultValueUsage, deprecationReason: ?string, extensions: ?ReadOnlyObjMap, astNode: ?InputValueDefinitionNode, |}; +export type GraphQLDefaultValueUsage = + | {| value: mixed |} + | {| literal: ConstValueNode |}; + export type GraphQLInputValueConfig = {| description?: ?string, type: GraphQLInputType, defaultValue?: mixed, + defaultValueLiteral?: ?ConstValueNode, deprecationReason?: ?string, extensions?: ?ReadOnlyObjMapLike, astNode?: ?InputValueDefinitionNode, diff --git a/src/type/index.d.ts b/src/type/index.d.ts index 365256dfd1d..996f644ced6 100644 --- a/src/type/index.d.ts +++ b/src/type/index.d.ts @@ -82,6 +82,7 @@ export { GraphQLArgumentConfig, GraphQLArgumentExtensions, GraphQLInputValue, + GraphQLDefaultValueUsage, GraphQLInputValueConfig, GraphQLEnumTypeConfig, GraphQLEnumTypeExtensions, diff --git a/src/type/index.js b/src/type/index.js index 6bf0943f228..eb175f866e7 100644 --- a/src/type/index.js +++ b/src/type/index.js @@ -136,6 +136,7 @@ export type { GraphQLArgument, GraphQLArgumentConfig, GraphQLInputValue, + GraphQLDefaultValueUsage, GraphQLInputValueConfig, GraphQLEnumTypeConfig, GraphQLEnumValue, diff --git a/src/type/introspection.js b/src/type/introspection.js index f0bce5838a0..ed111f892d6 100644 --- a/src/type/introspection.js +++ b/src/type/introspection.js @@ -384,8 +384,14 @@ export const __InputValue: GraphQLObjectType = new GraphQLObjectType({ 'A GraphQL-formatted string representing the default value for this input value.', resolve(inputValue) { const { type, defaultValue } = inputValue; - const valueAST = astFromValue(defaultValue, type); - return valueAST ? print(valueAST) : null; + if (!defaultValue) { + return null; + } + const literal = defaultValue.literal + ? defaultValue.literal + : astFromValue(defaultValue.value, type); + invariant(literal, 'Invalid default value'); + return print(literal); }, }, isDeprecated: { diff --git a/src/utilities/TypeInfo.d.ts b/src/utilities/TypeInfo.d.ts index cb737373901..59ed824ff45 100644 --- a/src/utilities/TypeInfo.d.ts +++ b/src/utilities/TypeInfo.d.ts @@ -12,6 +12,7 @@ import { GraphQLField, GraphQLArgument, GraphQLEnumValue, + GraphQLDefaultValueUsage, } from '../type/definition'; /** @@ -35,7 +36,7 @@ export class TypeInfo { getInputType(): Maybe; getParentInputType(): Maybe; getFieldDef(): Maybe>; - getDefaultValue(): Maybe; + getDefaultValue(): Maybe; getDirective(): Maybe; getArgument(): Maybe; getEnumValue(): Maybe; diff --git a/src/utilities/TypeInfo.js b/src/utilities/TypeInfo.js index 10225d17549..2141d27db97 100644 --- a/src/utilities/TypeInfo.js +++ b/src/utilities/TypeInfo.js @@ -15,6 +15,7 @@ import type { GraphQLArgument, GraphQLInputField, GraphQLEnumValue, + GraphQLDefaultValueUsage, } from '../type/definition'; import { isObjectType, @@ -47,7 +48,7 @@ export class TypeInfo { _parentTypeStack: Array; _inputTypeStack: Array; _fieldDefStack: Array>; - _defaultValueStack: Array; + _defaultValueStack: Array; _directive: ?GraphQLDirective; _argument: ?GraphQLArgument; _enumValue: ?GraphQLEnumValue; @@ -115,7 +116,7 @@ export class TypeInfo { } } - getDefaultValue(): ?mixed { + getDefaultValue(): ?GraphQLDefaultValueUsage { if (this._defaultValueStack.length > 0) { return this._defaultValueStack[this._defaultValueStack.length - 1]; } @@ -209,7 +210,7 @@ export class TypeInfo { } } this._argument = argDef; - this._defaultValueStack.push(argDef ? argDef.defaultValue : undefined); + this._defaultValueStack.push(argDef?.defaultValue); this._inputTypeStack.push(isInputType(argType) ? argType : undefined); break; } @@ -233,9 +234,7 @@ export class TypeInfo { inputFieldType = inputField.type; } } - this._defaultValueStack.push( - inputField ? inputField.defaultValue : undefined, - ); + this._defaultValueStack.push(inputField?.defaultValue); this._inputTypeStack.push( isInputType(inputFieldType) ? inputFieldType : undefined, ); diff --git a/src/utilities/__tests__/astFromValue-test.js b/src/utilities/__tests__/astFromValue-test.js index 3641f00227e..99dc3fdb5ae 100644 --- a/src/utilities/__tests__/astFromValue-test.js +++ b/src/utilities/__tests__/astFromValue-test.js @@ -51,6 +51,8 @@ describe('astFromValue', () => { kind: 'BooleanValue', value: false, }); + + expect(astFromValue(null, NonNullBoolean)).to.equal(null); }); it('converts Int values to Int ASTs', () => { diff --git a/src/utilities/__tests__/buildClientSchema-test.js b/src/utilities/__tests__/buildClientSchema-test.js index d3d0be8899e..504a990c2a1 100644 --- a/src/utilities/__tests__/buildClientSchema-test.js +++ b/src/utilities/__tests__/buildClientSchema-test.js @@ -441,6 +441,7 @@ describe('Type System: build schema from introspection', () => { } type Query { + defaultID(intArg: ID = "123"): String defaultInt(intArg: Int = 30): String defaultList(listArg: [Int] = [1, 2, 3]): String defaultObject(objArg: Geo = {lat: 37.485, lon: -122.148}): String @@ -596,6 +597,28 @@ describe('Type System: build schema from introspection', () => { expect(result.data).to.deep.equal({ foo: 'bar' }); }); + it('can use client schema for execution if resolvers are added', () => { + const schema = buildSchema(` + type Query { + foo(bar: String = "abc"): String + } + `); + + const introspection = introspectionFromSchema(schema); + const clientSchema = buildClientSchema(introspection); + + const QueryType: GraphQLObjectType = (clientSchema.getType('Query'): any); + QueryType.getFields().foo.resolve = (_value, args) => args.bar; + + const result = graphqlSync({ + schema: clientSchema, + source: '{ foo }', + }); + + expect(result.data).to.deep.equal({ foo: 'abc' }); + expect(result.data).to.deep.equal({ foo: 'abc' }); + }); + it('can build invalid schema', () => { const schema = buildSchema('type Query', { assumeValid: true }); diff --git a/src/utilities/__tests__/coerceInputValue-test.js b/src/utilities/__tests__/coerceInputValue-test.js index 28cbb06f5ee..fbfa5494d7f 100644 --- a/src/utilities/__tests__/coerceInputValue-test.js +++ b/src/utilities/__tests__/coerceInputValue-test.js @@ -24,7 +24,11 @@ import { GraphQLInputObjectType, } from '../../type/definition'; -import { coerceInputValue, coerceInputLiteral } from '../coerceInputValue'; +import { + coerceInputValue, + coerceInputLiteral, + coerceDefaultValue, +} from '../coerceInputValue'; type CoerceResult = {| value: mixed, @@ -601,10 +605,14 @@ describe('coerceInputLiteral', () => { name: 'TestInput', fields: { int: { type: GraphQLInt, defaultValue: 42 }, + float: { + type: GraphQLFloat, + defaultValueLiteral: { kind: 'FloatValue', value: '3.14' }, + }, }, }); - test('{}', type, { int: 42 }); + test('{}', type, { int: 42, float: 3.14 }); }); const testInputObj = new GraphQLInputObjectType({ @@ -672,3 +680,26 @@ describe('coerceInputLiteral', () => { }); }); }); + +describe('coerceDefaultValue', () => { + it('memoizes coercion', () => { + const parseValueCalls = []; + + const spyScalar = new GraphQLScalarType({ + name: 'SpyScalar', + parseValue(value) { + parseValueCalls.push(value); + return value; + }, + }); + + const defaultValueUsage = { + literal: { kind: 'StringValue', value: 'hello' }, + }; + expect(coerceDefaultValue(defaultValueUsage, spyScalar)).to.equal('hello'); + + // Call a second time + expect(coerceDefaultValue(defaultValueUsage, spyScalar)).to.equal('hello'); + expect(parseValueCalls).to.deep.equal(['hello']); + }); +}); diff --git a/src/utilities/astFromValue.js b/src/utilities/astFromValue.js index 5621659f73a..af9b9c74a6d 100644 --- a/src/utilities/astFromValue.js +++ b/src/utilities/astFromValue.js @@ -3,7 +3,7 @@ import { invariant } from '../jsutils/invariant'; import { isObjectLike } from '../jsutils/isObjectLike'; import { isIterableObject } from '../jsutils/isIterableObject'; -import type { ValueNode } from '../language/ast'; +import type { ConstValueNode } from '../language/ast'; import { Kind } from '../language/kinds'; import type { GraphQLInputType } from '../type/definition'; @@ -37,13 +37,15 @@ import { * | null | NullValue | * */ -export function astFromValue(value: mixed, type: GraphQLInputType): ?ValueNode { +export function astFromValue( + value: mixed, + type: GraphQLInputType, +): ?ConstValueNode { if (isNonNullType(type)) { - const astValue = astFromValue(value, type.ofType); - if (astValue?.kind === Kind.NULL) { + if (value === null) { return null; } - return astValue; + return astFromValue(value, type.ofType); } // only explicit null, not undefined, NaN diff --git a/src/utilities/buildClientSchema.js b/src/utilities/buildClientSchema.js index cf6b2a084ec..29daeb282cb 100644 --- a/src/utilities/buildClientSchema.js +++ b/src/utilities/buildClientSchema.js @@ -3,7 +3,7 @@ import { devAssert } from '../jsutils/devAssert'; import { keyValMap } from '../jsutils/keyValMap'; import { isObjectLike } from '../jsutils/isObjectLike'; -import { parseValue } from '../language/parser'; +import { parseConstValue } from '../language/parser'; import type { GraphQLSchemaValidationOptions } from '../type/schema'; import type { @@ -48,8 +48,6 @@ import type { IntrospectionNamedTypeRef, } from './getIntrospectionQuery'; -import { coerceInputLiteral } from './coerceInputValue'; - /** * Build a GraphQLSchema for use by client tools. * @@ -368,17 +366,13 @@ export function buildClientSchema( ); } - const defaultValue = - inputValueIntrospection.defaultValue != null - ? coerceInputLiteral( - parseValue(inputValueIntrospection.defaultValue), - type, - ) - : undefined; return { description: inputValueIntrospection.description, type, - defaultValue, + defaultValueLiteral: + inputValueIntrospection.defaultValue != null + ? parseConstValue(inputValueIntrospection.defaultValue) + : undefined, deprecationReason: inputValueIntrospection.deprecationReason, }; } diff --git a/src/utilities/coerceInputValue.d.ts b/src/utilities/coerceInputValue.d.ts index 45785330cef..8cccf4bfa34 100644 --- a/src/utilities/coerceInputValue.d.ts +++ b/src/utilities/coerceInputValue.d.ts @@ -3,7 +3,7 @@ import { ObjMap } from '../jsutils/ObjMap'; import { ValueNode } from '../language/ast'; -import { GraphQLInputType } from '../type/definition'; +import { GraphQLInputType, GraphQLDefaultValueUsage } from '../type/definition'; import { GraphQLError } from '../error/GraphQLError'; @@ -33,3 +33,11 @@ export function coerceInputLiteral( type: GraphQLInputType, variables?: Maybe>, ): unknown; + +/** + * @internal + */ +export function coerceDefaultValue( + usage: GraphQLDefaultValueUsage, + type: GraphQLInputType, +): unknown; diff --git a/src/utilities/coerceInputValue.js b/src/utilities/coerceInputValue.js index 4ba348a42b7..bb78c00aed4 100644 --- a/src/utilities/coerceInputValue.js +++ b/src/utilities/coerceInputValue.js @@ -13,7 +13,10 @@ import { isIterableObject } from '../jsutils/isIterableObject'; import { GraphQLError } from '../error/GraphQLError'; -import type { GraphQLInputType } from '../type/definition'; +import type { + GraphQLInputType, + GraphQLDefaultValueUsage, +} from '../type/definition'; import { isLeafType, isInputObjectType, @@ -109,8 +112,11 @@ function coerceInputValueImpl( const fieldValue = inputValue[field.name]; if (fieldValue === undefined) { - if (field.defaultValue !== undefined) { - coercedValue[field.name] = field.defaultValue; + if (field.defaultValue) { + coercedValue[field.name] = coerceDefaultValue( + field.defaultValue, + field.type, + ); } else if (isNonNullType(field.type)) { const typeStr = inspect(field.type); onError( @@ -277,8 +283,11 @@ export function coerceInputLiteral( if (isRequiredInput(field)) { return; // Invalid: intentionally return no value. } - if (field.defaultValue !== undefined) { - coercedValue[field.name] = field.defaultValue; + if (field.defaultValue) { + coercedValue[field.name] = coerceDefaultValue( + field.defaultValue, + field.type, + ); } } else { const fieldValue = coerceInputLiteral( @@ -319,3 +328,26 @@ function isMissingVariable( (variables == null || variables[valueNode.name.value] === undefined) ); } + +/** + * @internal + */ +export function coerceDefaultValue( + usage: GraphQLDefaultValueUsage, + type: GraphQLInputType, +): mixed { + if (!usage.literal) { + return usage.value; + } + // Memoize the result of coercing the default value in a hidden field. + let coercedValue = (usage: any)._memoizedCoercedValue; + if (coercedValue === undefined) { + coercedValue = coerceInputLiteral(usage.literal, type); + invariant( + coercedValue !== undefined, + 'Literal cannot be converted to value for this type', + ); + (usage: any)._memoizedCoercedValue = coercedValue; + } + return coercedValue; +} diff --git a/src/utilities/extendSchema.js b/src/utilities/extendSchema.js index d53b2e64678..9c2261a691d 100644 --- a/src/utilities/extendSchema.js +++ b/src/utilities/extendSchema.js @@ -79,8 +79,6 @@ import { GraphQLInputObjectType, } from '../type/definition'; -import { coerceInputLiteral } from './coerceInputValue'; - type Options = {| ...GraphQLSchemaValidationOptions, @@ -494,9 +492,7 @@ export function extendSchemaImpl( configMap[node.name.value] = { type, description: node.description?.value, - defaultValue: node.defaultValue - ? coerceInputLiteral(node.defaultValue, type) - : undefined, + defaultValueLiteral: node.defaultValue, deprecationReason: getDeprecationReason(node), astNode: node, }; diff --git a/src/utilities/findBreakingChanges.js b/src/utilities/findBreakingChanges.js index cc7ffe74433..a1ec47db077 100644 --- a/src/utilities/findBreakingChanges.js +++ b/src/utilities/findBreakingChanges.js @@ -17,6 +17,7 @@ import type { GraphQLObjectType, GraphQLInterfaceType, GraphQLInputObjectType, + GraphQLDefaultValueUsage, } from '../type/definition'; import { isSpecifiedScalarType } from '../type/scalars'; import { @@ -402,18 +403,19 @@ function findArgChanges( `${oldType.name}.${oldField.name} arg ${oldArg.name} has changed type from ` + `${String(oldArg.type)} to ${String(newArg.type)}.`, }); - } else if (oldArg.defaultValue !== undefined) { - if (newArg.defaultValue === undefined) { + } else if (oldArg.defaultValue) { + if (!newArg.defaultValue) { schemaChanges.push({ type: DangerousChangeType.ARG_DEFAULT_VALUE_CHANGE, description: `${oldType.name}.${oldField.name} arg ${oldArg.name} defaultValue was removed.`, }); } else { + const newArgDefaultValue = newArg.defaultValue; // Since we looking only for client's observable changes we should // compare default values in the same representation as they are // represented inside introspection. - const oldValueStr = stringifyValue(oldArg.defaultValue, oldArg.type); - const newValueStr = stringifyValue(newArg.defaultValue, newArg.type); + const oldValueStr = printDefaultValue(oldArg.defaultValue, oldArg.type); + const newValueStr = printDefaultValue(newArgDefaultValue, newArg.type); if (oldValueStr !== newValueStr) { schemaChanges.push({ @@ -533,10 +535,14 @@ function typeKindName(type: GraphQLNamedType): string { invariant(false, 'Unexpected type: ' + inspect((type: empty))); } -function stringifyValue(value: mixed, type: GraphQLInputType): string { - const ast = astFromValue(value, type); - invariant(ast != null); - +function printDefaultValue( + defaultValue: GraphQLDefaultValueUsage, + type: GraphQLInputType, +): string { + const ast = defaultValue.literal + ? defaultValue.literal + : astFromValue(defaultValue.value, type); + invariant(ast); const sortedAST = visit(ast, { ObjectValue(objectNode) { // Make a copy since sort mutates array diff --git a/src/utilities/printSchema.js b/src/utilities/printSchema.js index cc41725368a..941c405921d 100644 --- a/src/utilities/printSchema.js +++ b/src/utilities/printSchema.js @@ -258,10 +258,13 @@ function printArgs( } function printInputValue(arg: GraphQLInputField): string { - const defaultAST = astFromValue(arg.defaultValue, arg.type); let argDecl = arg.name + ': ' + String(arg.type); - if (defaultAST) { - argDecl += ` = ${print(defaultAST)}`; + if (arg.defaultValue) { + const literal = arg.defaultValue.literal + ? arg.defaultValue.literal + : astFromValue(arg.defaultValue.value, arg.type); + invariant(literal, 'Invalid default value'); + argDecl += ` = ${print(literal)}`; } return argDecl + printDeprecated(arg.deprecationReason); } diff --git a/src/validation/ValidationContext.d.ts b/src/validation/ValidationContext.d.ts index 0424c14da48..423b1be66e2 100644 --- a/src/validation/ValidationContext.d.ts +++ b/src/validation/ValidationContext.d.ts @@ -19,6 +19,7 @@ import { GraphQLField, GraphQLArgument, GraphQLEnumValue, + GraphQLDefaultValueUsage, } from '../type/definition'; import { TypeInfo } from '../utilities/TypeInfo'; @@ -26,7 +27,7 @@ type NodeWithSelectionSet = OperationDefinitionNode | FragmentDefinitionNode; interface VariableUsage { readonly node: VariableNode; readonly type: Maybe; - readonly defaultValue: Maybe; + readonly defaultValue: Maybe; } /** diff --git a/src/validation/ValidationContext.js b/src/validation/ValidationContext.js index 6db25ecc5b5..0282654812a 100644 --- a/src/validation/ValidationContext.js +++ b/src/validation/ValidationContext.js @@ -24,6 +24,7 @@ import type { GraphQLField, GraphQLArgument, GraphQLEnumValue, + GraphQLDefaultValueUsage, } from '../type/definition'; import { TypeInfo, visitWithTypeInfo } from '../utilities/TypeInfo'; @@ -32,7 +33,7 @@ type NodeWithSelectionSet = OperationDefinitionNode | FragmentDefinitionNode; type VariableUsage = {| +node: VariableNode, +type: ?GraphQLInputType, - +defaultValue: ?mixed, + +defaultValue: ?GraphQLDefaultValueUsage, |}; /** diff --git a/src/validation/rules/VariablesInAllowedPositionRule.js b/src/validation/rules/VariablesInAllowedPositionRule.js index 10701908762..d76aa12b9d8 100644 --- a/src/validation/rules/VariablesInAllowedPositionRule.js +++ b/src/validation/rules/VariablesInAllowedPositionRule.js @@ -7,7 +7,10 @@ import type { ValueNode } from '../../language/ast'; import type { ASTVisitor } from '../../language/visitor'; import type { GraphQLSchema } from '../../type/schema'; -import type { GraphQLType } from '../../type/definition'; +import type { + GraphQLType, + GraphQLDefaultValueUsage, +} from '../../type/definition'; import { isNonNullType } from '../../type/definition'; import { typeFromAST } from '../../utilities/typeFromAST'; @@ -81,7 +84,7 @@ function allowedVariableUsage( varType: GraphQLType, varDefaultValue: ?ValueNode, locationType: GraphQLType, - locationDefaultValue: ?mixed, + locationDefaultValue: ?GraphQLDefaultValueUsage, ): boolean { if (isNonNullType(locationType) && !isNonNullType(varType)) { const hasNonNullVariableDefaultValue =