Skip to content

Commit 023cd9c

Browse files
leebyronyaacovCR
authored andcommitted
Preserve defaultValue literals
Fixes #3051 This change solves the problem of default values defined via SDL not always resolving correctly through introspection by preserving the original GraphQL literal in the schema definition. This changes argument and input field definitions `defaultValue` field from just the "value" to a new `GraphQLDefaultValueUsage` type which contains either or both "value" and "literal" fields. Since either of these fields may be set, new functions for resolving a value or literal from either have been added - `getLiteralDefaultValue` and `getCoercedDefaultValue` - these replace uses that either assumed a set value or were manually converting a value back to a literal. Here is the flow for how a default value defined in an SDL would be converted into a functional schema and back to an SDL: **Before this change:** ``` (SDL) --parse-> (AST) --coerceInputLiteral--> (defaultValue config) --valueToAST--> (AST) --print --> (SDL) ``` `coerceInputLiteral` performs coercion which is a one-way function, and `valueToAST` is unsafe and set to be deprecated in #3049. **After this change:** ``` (SDL) --parse-> (defaultValue literal config) --print --> (SDL) ```
1 parent cdf3f24 commit 023cd9c

17 files changed

+233
-49
lines changed

src/execution/values.ts

+11-4
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import type { GraphQLDirective } from '../type/directives.js';
2020
import type { GraphQLSchema } from '../type/schema.js';
2121

2222
import {
23+
coerceDefaultValue,
2324
coerceInputLiteral,
2425
coerceInputValue,
2526
} from '../utilities/coerceInputValue.js';
@@ -170,8 +171,11 @@ export function getArgumentValues(
170171
const argumentNode = argNodeMap[name];
171172

172173
if (!argumentNode) {
173-
if (argDef.defaultValue !== undefined) {
174-
coercedValues[name] = argDef.defaultValue;
174+
if (argDef.defaultValue) {
175+
coercedValues[name] = coerceDefaultValue(
176+
argDef.defaultValue,
177+
argDef.type,
178+
);
175179
} else if (isNonNullType(argType)) {
176180
throw new GraphQLError(
177181
`Argument ${argDef} of required type ${argType} was not provided.`,
@@ -190,8 +194,11 @@ export function getArgumentValues(
190194
variableValues == null ||
191195
!hasOwnProperty(variableValues, variableName)
192196
) {
193-
if (argDef.defaultValue !== undefined) {
194-
coercedValues[name] = argDef.defaultValue;
197+
if (argDef.defaultValue) {
198+
coercedValues[name] = coerceDefaultValue(
199+
argDef.defaultValue,
200+
argDef.type,
201+
);
195202
} else if (isNonNullType(argType)) {
196203
throw new GraphQLError(
197204
`Argument ${argDef} of required type ${argType} ` +

src/index.ts

+1
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,7 @@ export type {
199199
GraphQLScalarSerializer,
200200
GraphQLScalarValueParser,
201201
GraphQLScalarLiteralParser,
202+
GraphQLDefaultValueUsage,
202203
} from './type/index.js';
203204

204205
// Parse and operate on GraphQL language source files.

src/type/__tests__/definition-test.ts

+60
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { describe, it } from 'mocha';
44
import { identityFunc } from '../../jsutils/identityFunc.js';
55
import { inspect } from '../../jsutils/inspect.js';
66

7+
import { Kind } from '../../language/kinds.js';
78
import { parseValue } from '../../language/parser.js';
89

910
import type { GraphQLNullableType, GraphQLType } from '../definition.js';
@@ -617,6 +618,65 @@ describe('Type System: Input Objects', () => {
617618
'not used anymore',
618619
);
619620
});
621+
622+
describe('Input Object fields may have default values', () => {
623+
it('accepts an Input Object type with a default value', () => {
624+
const inputObjType = new GraphQLInputObjectType({
625+
name: 'SomeInputObject',
626+
fields: {
627+
f: { type: ScalarType, defaultValue: 3 },
628+
},
629+
});
630+
expect(inputObjType.getFields().f).to.deep.include({
631+
coordinate: 'SomeInputObject.f',
632+
name: 'f',
633+
description: undefined,
634+
type: ScalarType,
635+
defaultValue: { value: 3 },
636+
deprecationReason: undefined,
637+
extensions: {},
638+
astNode: undefined,
639+
});
640+
});
641+
642+
it('accepts an Input Object type with a default value literal', () => {
643+
const inputObjType = new GraphQLInputObjectType({
644+
name: 'SomeInputObject',
645+
fields: {
646+
f: {
647+
type: ScalarType,
648+
defaultValueLiteral: { kind: Kind.INT, value: '3' },
649+
},
650+
},
651+
});
652+
expect(inputObjType.getFields().f).to.deep.include({
653+
coordinate: 'SomeInputObject.f',
654+
name: 'f',
655+
description: undefined,
656+
type: ScalarType,
657+
defaultValue: { literal: { kind: 'IntValue', value: '3' } },
658+
deprecationReason: undefined,
659+
extensions: {},
660+
astNode: undefined,
661+
});
662+
});
663+
664+
it('rejects an Input Object type with potentially conflicting default values', () => {
665+
const inputObjType = new GraphQLInputObjectType({
666+
name: 'SomeInputObject',
667+
fields: {
668+
f: {
669+
type: ScalarType,
670+
defaultValue: 3,
671+
defaultValueLiteral: { kind: Kind.INT, value: '3' },
672+
},
673+
},
674+
});
675+
expect(() => inputObjType.getFields()).to.throw(
676+
'f has both a defaultValue and a defaultValueLiteral property, but only one must be provided.',
677+
);
678+
});
679+
});
620680
});
621681

622682
describe('Type System: List', () => {

src/type/definition.ts

+31-6
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import { toObjMap } from '../jsutils/toObjMap.js';
1616
import { GraphQLError } from '../error/GraphQLError.js';
1717

1818
import type {
19+
ConstValueNode,
1920
EnumTypeDefinitionNode,
2021
EnumTypeExtensionNode,
2122
EnumValueDefinitionNode,
@@ -908,6 +909,7 @@ export interface GraphQLArgumentConfig {
908909
description?: Maybe<string>;
909910
type: GraphQLInputType;
910911
defaultValue?: unknown;
912+
defaultValueLiteral?: ConstValueNode | undefined;
911913
deprecationReason?: Maybe<string>;
912914
extensions?: Maybe<Readonly<GraphQLArgumentExtensions>>;
913915
astNode?: Maybe<InputValueDefinitionNode>;
@@ -984,7 +986,7 @@ export class GraphQLArgument extends GraphQLSchemaElement {
984986
name: string;
985987
description: Maybe<string>;
986988
type: GraphQLInputType;
987-
defaultValue: unknown;
989+
defaultValue: GraphQLDefaultValueUsage | undefined;
988990
deprecationReason: Maybe<string>;
989991
extensions: Readonly<GraphQLArgumentExtensions>;
990992
astNode: Maybe<InputValueDefinitionNode>;
@@ -999,7 +1001,7 @@ export class GraphQLArgument extends GraphQLSchemaElement {
9991001
this.name = assertName(name);
10001002
this.description = config.description;
10011003
this.type = config.type;
1002-
this.defaultValue = config.defaultValue;
1004+
this.defaultValue = defineDefaultValue(coordinate, config);
10031005
this.deprecationReason = config.deprecationReason;
10041006
this.extensions = toObjMap(config.extensions);
10051007
this.astNode = config.astNode;
@@ -1013,7 +1015,8 @@ export class GraphQLArgument extends GraphQLSchemaElement {
10131015
return {
10141016
description: this.description,
10151017
type: this.type,
1016-
defaultValue: this.defaultValue,
1018+
defaultValue: this.defaultValue?.value,
1019+
defaultValueLiteral: this.defaultValue?.literal,
10171020
deprecationReason: this.deprecationReason,
10181021
extensions: this.extensions,
10191022
astNode: this.astNode,
@@ -1029,6 +1032,26 @@ export type GraphQLFieldMap<TSource, TContext> = ObjMap<
10291032
GraphQLField<TSource, TContext>
10301033
>;
10311034

1035+
export type GraphQLDefaultValueUsage =
1036+
| { value: unknown; literal?: never }
1037+
| { literal: ConstValueNode; value?: never };
1038+
1039+
function defineDefaultValue(
1040+
coordinate: string,
1041+
config: GraphQLArgumentConfig | GraphQLInputFieldConfig,
1042+
): GraphQLDefaultValueUsage | undefined {
1043+
if (config.defaultValue === undefined && !config.defaultValueLiteral) {
1044+
return;
1045+
}
1046+
devAssert(
1047+
!(config.defaultValue !== undefined && config.defaultValueLiteral),
1048+
`${coordinate} has both a defaultValue and a defaultValueLiteral property, but only one must be provided.`,
1049+
);
1050+
return config.defaultValueLiteral
1051+
? { literal: config.defaultValueLiteral }
1052+
: { value: config.defaultValue };
1053+
}
1054+
10321055
/**
10331056
* Custom extensions
10341057
*
@@ -1608,6 +1631,7 @@ export interface GraphQLInputFieldConfig {
16081631
description?: Maybe<string>;
16091632
type: GraphQLInputType;
16101633
defaultValue?: unknown;
1634+
defaultValueLiteral?: ConstValueNode | undefined;
16111635
deprecationReason?: Maybe<string>;
16121636
extensions?: Maybe<Readonly<GraphQLInputFieldExtensions>>;
16131637
astNode?: Maybe<InputValueDefinitionNode>;
@@ -1619,7 +1643,7 @@ export class GraphQLInputField extends GraphQLSchemaElement {
16191643
name: string;
16201644
description: Maybe<string>;
16211645
type: GraphQLInputType;
1622-
defaultValue: unknown;
1646+
defaultValue: GraphQLDefaultValueUsage | undefined;
16231647
deprecationReason: Maybe<string>;
16241648
extensions: Readonly<GraphQLInputFieldExtensions>;
16251649
astNode: Maybe<InputValueDefinitionNode>;
@@ -1640,7 +1664,7 @@ export class GraphQLInputField extends GraphQLSchemaElement {
16401664
this.name = assertName(name);
16411665
this.description = config.description;
16421666
this.type = config.type;
1643-
this.defaultValue = config.defaultValue;
1667+
this.defaultValue = defineDefaultValue(coordinate, config);
16441668
this.deprecationReason = config.deprecationReason;
16451669
this.extensions = toObjMap(config.extensions);
16461670
this.astNode = config.astNode;
@@ -1654,7 +1678,8 @@ export class GraphQLInputField extends GraphQLSchemaElement {
16541678
return {
16551679
description: this.description,
16561680
type: this.type,
1657-
defaultValue: this.defaultValue,
1681+
defaultValue: this.defaultValue?.value,
1682+
defaultValueLiteral: this.defaultValue?.literal,
16581683
deprecationReason: this.deprecationReason,
16591684
extensions: this.extensions,
16601685
astNode: this.astNode,

src/type/index.ts

+1
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,7 @@ export type {
120120
GraphQLScalarSerializer,
121121
GraphQLScalarValueParser,
122122
GraphQLScalarLiteralParser,
123+
GraphQLDefaultValueUsage,
123124
} from './definition.js';
124125

125126
export {

src/type/introspection.ts

+7-2
Original file line numberDiff line numberDiff line change
@@ -395,8 +395,13 @@ export const __InputValue: GraphQLObjectType = new GraphQLObjectType({
395395
'A GraphQL-formatted string representing the default value for this input value.',
396396
resolve(inputValue) {
397397
const { type, defaultValue } = inputValue;
398-
const valueAST = astFromValue(defaultValue, type);
399-
return valueAST ? print(valueAST) : null;
398+
if (!defaultValue) {
399+
return null;
400+
}
401+
const literal =
402+
defaultValue.literal ?? astFromValue(defaultValue.value, type);
403+
invariant(literal != null, 'Invalid default value');
404+
return print(literal);
400405
},
401406
},
402407
isDeprecated: {

src/utilities/TypeInfo.ts

+3-2
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { getEnterLeaveForKind } from '../language/visitor.js';
99
import type {
1010
GraphQLArgument,
1111
GraphQLCompositeType,
12+
GraphQLDefaultValueUsage,
1213
GraphQLEnumValue,
1314
GraphQLField,
1415
GraphQLInputField,
@@ -43,7 +44,7 @@ export class TypeInfo {
4344
private _parentTypeStack: Array<Maybe<GraphQLCompositeType>>;
4445
private _inputTypeStack: Array<Maybe<GraphQLInputType>>;
4546
private _fieldDefStack: Array<Maybe<GraphQLField<unknown, unknown>>>;
46-
private _defaultValueStack: Array<Maybe<unknown>>;
47+
private _defaultValueStack: Array<GraphQLDefaultValueUsage | undefined>;
4748
private _directive: Maybe<GraphQLDirective>;
4849
private _argument: Maybe<GraphQLArgument>;
4950
private _enumValue: Maybe<GraphQLEnumValue>;
@@ -117,7 +118,7 @@ export class TypeInfo {
117118
}
118119
}
119120

120-
getDefaultValue(): Maybe<unknown> {
121+
getDefaultValue(): GraphQLDefaultValueUsage | undefined {
121122
if (this._defaultValueStack.length > 0) {
122123
return this._defaultValueStack[this._defaultValueStack.length - 1];
123124
}

src/utilities/__tests__/buildClientSchema-test.ts

+23
Original file line numberDiff line numberDiff line change
@@ -445,6 +445,7 @@ describe('Type System: build schema from introspection', () => {
445445
}
446446
447447
type Query {
448+
defaultID(intArg: ID = "123"): String
448449
defaultInt(intArg: Int = 30): String
449450
defaultList(listArg: [Int] = [1, 2, 3]): String
450451
defaultObject(objArg: Geo = { lat: 37.485, lon: -122.148 }): String
@@ -600,6 +601,28 @@ describe('Type System: build schema from introspection', () => {
600601
expect(result.data).to.deep.equal({ foo: 'bar' });
601602
});
602603

604+
it('can use client schema for execution if resolvers are added', () => {
605+
const schema = buildSchema(`
606+
type Query {
607+
foo(bar: String = "abc"): String
608+
}
609+
`);
610+
611+
const introspection = introspectionFromSchema(schema);
612+
const clientSchema = buildClientSchema(introspection);
613+
614+
const QueryType: GraphQLObjectType = clientSchema.getType('Query') as any;
615+
QueryType.getFields().foo.resolve = (_value, args) => args.bar;
616+
617+
const result = graphqlSync({
618+
schema: clientSchema,
619+
source: '{ foo }',
620+
});
621+
622+
expect(result.data).to.deep.equal({ foo: 'abc' });
623+
expect(result.data).to.deep.equal({ foo: 'abc' });
624+
});
625+
603626
it('can build invalid schema', () => {
604627
const schema = buildSchema('type Query', { assumeValid: true });
605628

src/utilities/__tests__/coerceInputValue-test.ts

+34-2
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { identityFunc } from '../../jsutils/identityFunc.js';
55
import { invariant } from '../../jsutils/invariant.js';
66
import type { ObjMap } from '../../jsutils/ObjMap.js';
77

8+
import { Kind } from '../../language/kinds.js';
89
import { parseValue } from '../../language/parser.js';
910
import { print } from '../../language/printer.js';
1011

@@ -24,7 +25,11 @@ import {
2425
GraphQLString,
2526
} from '../../type/scalars.js';
2627

27-
import { coerceInputLiteral, coerceInputValue } from '../coerceInputValue.js';
28+
import {
29+
coerceDefaultValue,
30+
coerceInputLiteral,
31+
coerceInputValue,
32+
} from '../coerceInputValue.js';
2833

2934
interface CoerceResult {
3035
value: unknown;
@@ -610,10 +615,14 @@ describe('coerceInputLiteral', () => {
610615
name: 'TestInput',
611616
fields: {
612617
int: { type: GraphQLInt, defaultValue: 42 },
618+
float: {
619+
type: GraphQLFloat,
620+
defaultValueLiteral: { kind: Kind.FLOAT, value: '3.14' },
621+
},
613622
},
614623
});
615624

616-
test('{}', type, { int: 42 });
625+
test('{}', type, { int: 42, float: 3.14 });
617626
});
618627

619628
const testInputObj = new GraphQLInputObjectType({
@@ -681,3 +690,26 @@ describe('coerceInputLiteral', () => {
681690
});
682691
});
683692
});
693+
694+
describe('coerceDefaultValue', () => {
695+
it('memoizes coercion', () => {
696+
const parseValueCalls: any = [];
697+
698+
const spyScalar = new GraphQLScalarType({
699+
name: 'SpyScalar',
700+
parseValue(value) {
701+
parseValueCalls.push(value);
702+
return value;
703+
},
704+
});
705+
706+
const defaultValueUsage = {
707+
literal: { kind: Kind.STRING, value: 'hello' },
708+
} as const;
709+
expect(coerceDefaultValue(defaultValueUsage, spyScalar)).to.equal('hello');
710+
711+
// Call a second time
712+
expect(coerceDefaultValue(defaultValueUsage, spyScalar)).to.equal('hello');
713+
expect(parseValueCalls).to.deep.equal(['hello']);
714+
});
715+
});

0 commit comments

Comments
 (0)