Skip to content

Commit 4883f29

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 3f694fd commit 4883f29

17 files changed

+233
-49
lines changed

src/execution/values.ts

Lines changed: 11 additions & 4 deletions
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

Lines changed: 1 addition & 0 deletions
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

Lines changed: 60 additions & 0 deletions
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

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import { toObjMap } from '../jsutils/toObjMap.js';
1717
import { GraphQLError } from '../error/GraphQLError.js';
1818

1919
import type {
20+
ConstValueNode,
2021
EnumTypeDefinitionNode,
2122
EnumTypeExtensionNode,
2223
EnumValueDefinitionNode,
@@ -913,6 +914,7 @@ export interface GraphQLArgumentConfig {
913914
description?: Maybe<string>;
914915
type: GraphQLInputType;
915916
defaultValue?: unknown;
917+
defaultValueLiteral?: ConstValueNode | undefined;
916918
deprecationReason?: Maybe<string>;
917919
extensions?: Maybe<Readonly<GraphQLArgumentExtensions>>;
918920
astNode?: Maybe<InputValueDefinitionNode>;
@@ -1002,7 +1004,7 @@ export class GraphQLArgument extends GraphQLSchemaElement {
10021004
name: string;
10031005
description: Maybe<string>;
10041006
type: GraphQLInputType;
1005-
defaultValue: unknown;
1007+
defaultValue: GraphQLDefaultValueUsage | undefined;
10061008
deprecationReason: Maybe<string>;
10071009
extensions: Readonly<GraphQLArgumentExtensions>;
10081010
astNode: Maybe<InputValueDefinitionNode>;
@@ -1017,7 +1019,7 @@ export class GraphQLArgument extends GraphQLSchemaElement {
10171019
this.name = assertName(name);
10181020
this.description = config.description;
10191021
this.type = config.type;
1020-
this.defaultValue = config.defaultValue;
1022+
this.defaultValue = defineDefaultValue(coordinate, config);
10211023
this.deprecationReason = config.deprecationReason;
10221024
this.extensions = toObjMap(config.extensions);
10231025
this.astNode = config.astNode;
@@ -1031,7 +1033,8 @@ export class GraphQLArgument extends GraphQLSchemaElement {
10311033
return {
10321034
description: this.description,
10331035
type: this.type,
1034-
defaultValue: this.defaultValue,
1036+
defaultValue: this.defaultValue?.value,
1037+
defaultValueLiteral: this.defaultValue?.literal,
10351038
deprecationReason: this.deprecationReason,
10361039
extensions: this.extensions,
10371040
astNode: this.astNode,
@@ -1047,6 +1050,26 @@ export type GraphQLFieldMap<TSource, TContext> = ObjMap<
10471050
GraphQLField<TSource, TContext>
10481051
>;
10491052

1053+
export type GraphQLDefaultValueUsage =
1054+
| { value: unknown; literal?: never }
1055+
| { literal: ConstValueNode; value?: never };
1056+
1057+
function defineDefaultValue(
1058+
coordinate: string,
1059+
config: GraphQLArgumentConfig | GraphQLInputFieldConfig,
1060+
): GraphQLDefaultValueUsage | undefined {
1061+
if (config.defaultValue === undefined && !config.defaultValueLiteral) {
1062+
return;
1063+
}
1064+
devAssert(
1065+
!(config.defaultValue !== undefined && config.defaultValueLiteral),
1066+
`${coordinate} has both a defaultValue and a defaultValueLiteral property, but only one must be provided.`,
1067+
);
1068+
return config.defaultValueLiteral
1069+
? { literal: config.defaultValueLiteral }
1070+
: { value: config.defaultValue };
1071+
}
1072+
10501073
/**
10511074
* Custom extensions
10521075
*
@@ -1640,6 +1663,7 @@ export interface GraphQLInputFieldConfig {
16401663
description?: Maybe<string>;
16411664
type: GraphQLInputType;
16421665
defaultValue?: unknown;
1666+
defaultValueLiteral?: ConstValueNode | undefined;
16431667
deprecationReason?: Maybe<string>;
16441668
extensions?: Maybe<Readonly<GraphQLInputFieldExtensions>>;
16451669
astNode?: Maybe<InputValueDefinitionNode>;
@@ -1651,7 +1675,7 @@ export class GraphQLInputField extends GraphQLSchemaElement {
16511675
name: string;
16521676
description: Maybe<string>;
16531677
type: GraphQLInputType;
1654-
defaultValue: unknown;
1678+
defaultValue: GraphQLDefaultValueUsage | undefined;
16551679
deprecationReason: Maybe<string>;
16561680
extensions: Readonly<GraphQLInputFieldExtensions>;
16571681
astNode: Maybe<InputValueDefinitionNode>;
@@ -1672,7 +1696,7 @@ export class GraphQLInputField extends GraphQLSchemaElement {
16721696
this.name = assertName(name);
16731697
this.description = config.description;
16741698
this.type = config.type;
1675-
this.defaultValue = config.defaultValue;
1699+
this.defaultValue = defineDefaultValue(coordinate, config);
16761700
this.deprecationReason = config.deprecationReason;
16771701
this.extensions = toObjMap(config.extensions);
16781702
this.astNode = config.astNode;
@@ -1686,7 +1710,8 @@ export class GraphQLInputField extends GraphQLSchemaElement {
16861710
return {
16871711
description: this.description,
16881712
type: this.type,
1689-
defaultValue: this.defaultValue,
1713+
defaultValue: this.defaultValue?.value,
1714+
defaultValueLiteral: this.defaultValue?.literal,
16901715
deprecationReason: this.deprecationReason,
16911716
extensions: this.extensions,
16921717
astNode: this.astNode,

src/type/index.ts

Lines changed: 1 addition & 0 deletions
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

Lines changed: 7 additions & 2 deletions
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

Lines changed: 3 additions & 2 deletions
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

Lines changed: 23 additions & 0 deletions
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

Lines changed: 34 additions & 2 deletions
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)