Skip to content

Commit fd87704

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 c1fa131 commit fd87704

16 files changed

+230
-46
lines changed

src/execution/values.ts

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

2121
import {
22+
coerceDefaultValue,
2223
coerceInputLiteral,
2324
coerceInputValue,
2425
} from '../utilities/coerceInputValue.js';
@@ -169,8 +170,11 @@ export function getArgumentValues(
169170
const argumentNode = argNodeMap.get(name);
170171

171172
if (argumentNode == null) {
172-
if (argDef.defaultValue !== undefined) {
173-
coercedValues[name] = argDef.defaultValue;
173+
if (argDef.defaultValue) {
174+
coercedValues[name] = coerceDefaultValue(
175+
argDef.defaultValue,
176+
argDef.type,
177+
);
174178
} else if (isNonNullType(argType)) {
175179
throw new GraphQLError(
176180
`Argument ${argDef} of required type ${argType} was not provided.`,
@@ -189,8 +193,11 @@ export function getArgumentValues(
189193
variableValues == null ||
190194
!Object.hasOwn(variableValues, variableName)
191195
) {
192-
if (argDef.defaultValue !== undefined) {
193-
coercedValues[name] = argDef.defaultValue;
196+
if (argDef.defaultValue) {
197+
coercedValues[name] = coerceDefaultValue(
198+
argDef.defaultValue,
199+
argDef.type,
200+
);
194201
} else if (isNonNullType(argType)) {
195202
throw new GraphQLError(
196203
`Argument ${argDef} of required type ${argType} ` +

src/index.ts

+1
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,7 @@ export type {
200200
GraphQLScalarSerializer,
201201
GraphQLScalarValueParser,
202202
GraphQLScalarLiteralParser,
203+
GraphQLDefaultValueUsage,
203204
} from './type/index.js';
204205

205206
// 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';
@@ -589,6 +590,65 @@ describe('Type System: Input Objects', () => {
589590
'not used anymore',
590591
);
591592
});
593+
594+
describe('Input Object fields may have default values', () => {
595+
it('accepts an Input Object type with a default value', () => {
596+
const inputObjType = new GraphQLInputObjectType({
597+
name: 'SomeInputObject',
598+
fields: {
599+
f: { type: ScalarType, defaultValue: 3 },
600+
},
601+
});
602+
expect(inputObjType.getFields().f).to.deep.include({
603+
coordinate: 'SomeInputObject.f',
604+
name: 'f',
605+
description: undefined,
606+
type: ScalarType,
607+
defaultValue: { value: 3 },
608+
deprecationReason: undefined,
609+
extensions: {},
610+
astNode: undefined,
611+
});
612+
});
613+
614+
it('accepts an Input Object type with a default value literal', () => {
615+
const inputObjType = new GraphQLInputObjectType({
616+
name: 'SomeInputObject',
617+
fields: {
618+
f: {
619+
type: ScalarType,
620+
defaultValueLiteral: { kind: Kind.INT, value: '3' },
621+
},
622+
},
623+
});
624+
expect(inputObjType.getFields().f).to.deep.include({
625+
coordinate: 'SomeInputObject.f',
626+
name: 'f',
627+
description: undefined,
628+
type: ScalarType,
629+
defaultValue: { literal: { kind: 'IntValue', value: '3' } },
630+
deprecationReason: undefined,
631+
extensions: {},
632+
astNode: undefined,
633+
});
634+
});
635+
636+
it('rejects an Input Object type with potentially conflicting default values', () => {
637+
const inputObjType = new GraphQLInputObjectType({
638+
name: 'SomeInputObject',
639+
fields: {
640+
f: {
641+
type: ScalarType,
642+
defaultValue: 3,
643+
defaultValueLiteral: { kind: Kind.INT, value: '3' },
644+
},
645+
},
646+
});
647+
expect(() => inputObjType.getFields()).to.throw(
648+
'f has both a defaultValue and a defaultValueLiteral property, but only one must be provided.',
649+
);
650+
});
651+
});
592652
});
593653

594654
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,
@@ -906,6 +907,7 @@ export interface GraphQLArgumentConfig {
906907
description?: Maybe<string>;
907908
type: GraphQLInputType;
908909
defaultValue?: unknown;
910+
defaultValueLiteral?: ConstValueNode | undefined;
909911
deprecationReason?: Maybe<string>;
910912
extensions?: Maybe<Readonly<GraphQLArgumentExtensions>>;
911913
astNode?: Maybe<InputValueDefinitionNode>;
@@ -982,7 +984,7 @@ export class GraphQLArgument extends GraphQLSchemaElement {
982984
name: string;
983985
description: Maybe<string>;
984986
type: GraphQLInputType;
985-
defaultValue: unknown;
987+
defaultValue: GraphQLDefaultValueUsage | undefined;
986988
deprecationReason: Maybe<string>;
987989
extensions: Readonly<GraphQLArgumentExtensions>;
988990
astNode: Maybe<InputValueDefinitionNode>;
@@ -997,7 +999,7 @@ export class GraphQLArgument extends GraphQLSchemaElement {
997999
this.name = assertName(name);
9981000
this.description = config.description;
9991001
this.type = config.type;
1000-
this.defaultValue = config.defaultValue;
1002+
this.defaultValue = defineDefaultValue(coordinate, config);
10011003
this.deprecationReason = config.deprecationReason;
10021004
this.extensions = toObjMap(config.extensions);
10031005
this.astNode = config.astNode;
@@ -1011,7 +1013,8 @@ export class GraphQLArgument extends GraphQLSchemaElement {
10111013
return {
10121014
description: this.description,
10131015
type: this.type,
1014-
defaultValue: this.defaultValue,
1016+
defaultValue: this.defaultValue?.value,
1017+
defaultValueLiteral: this.defaultValue?.literal,
10151018
deprecationReason: this.deprecationReason,
10161019
extensions: this.extensions,
10171020
astNode: this.astNode,
@@ -1027,6 +1030,26 @@ export type GraphQLFieldMap<TSource, TContext> = ObjMap<
10271030
GraphQLField<TSource, TContext>
10281031
>;
10291032

1033+
export type GraphQLDefaultValueUsage =
1034+
| { value: unknown; literal?: never }
1035+
| { literal: ConstValueNode; value?: never };
1036+
1037+
function defineDefaultValue(
1038+
coordinate: string,
1039+
config: GraphQLArgumentConfig | GraphQLInputFieldConfig,
1040+
): GraphQLDefaultValueUsage | undefined {
1041+
if (config.defaultValue === undefined && !config.defaultValueLiteral) {
1042+
return;
1043+
}
1044+
devAssert(
1045+
!(config.defaultValue !== undefined && config.defaultValueLiteral),
1046+
`${coordinate} has both a defaultValue and a defaultValueLiteral property, but only one must be provided.`,
1047+
);
1048+
return config.defaultValueLiteral
1049+
? { literal: config.defaultValueLiteral }
1050+
: { value: config.defaultValue };
1051+
}
1052+
10301053
/**
10311054
* Custom extensions
10321055
*
@@ -1620,6 +1643,7 @@ export interface GraphQLInputFieldConfig {
16201643
description?: Maybe<string>;
16211644
type: GraphQLInputType;
16221645
defaultValue?: unknown;
1646+
defaultValueLiteral?: ConstValueNode | undefined;
16231647
deprecationReason?: Maybe<string>;
16241648
extensions?: Maybe<Readonly<GraphQLInputFieldExtensions>>;
16251649
astNode?: Maybe<InputValueDefinitionNode>;
@@ -1631,7 +1655,7 @@ export class GraphQLInputField extends GraphQLSchemaElement {
16311655
name: string;
16321656
description: Maybe<string>;
16331657
type: GraphQLInputType;
1634-
defaultValue: unknown;
1658+
defaultValue: GraphQLDefaultValueUsage | undefined;
16351659
deprecationReason: Maybe<string>;
16361660
extensions: Readonly<GraphQLInputFieldExtensions>;
16371661
astNode: Maybe<InputValueDefinitionNode>;
@@ -1652,7 +1676,7 @@ export class GraphQLInputField extends GraphQLSchemaElement {
16521676
this.name = assertName(name);
16531677
this.description = config.description;
16541678
this.type = config.type;
1655-
this.defaultValue = config.defaultValue;
1679+
this.defaultValue = defineDefaultValue(coordinate, config);
16561680
this.deprecationReason = config.deprecationReason;
16571681
this.extensions = toObjMap(config.extensions);
16581682
this.astNode = config.astNode;
@@ -1666,7 +1690,8 @@ export class GraphQLInputField extends GraphQLSchemaElement {
16661690
return {
16671691
description: this.description,
16681692
type: this.type,
1669-
defaultValue: this.defaultValue,
1693+
defaultValue: this.defaultValue?.value,
1694+
defaultValueLiteral: this.defaultValue?.literal,
16701695
deprecationReason: this.deprecationReason,
16711696
extensions: this.extensions,
16721697
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
@@ -403,8 +403,13 @@ export const __InputValue: GraphQLObjectType = new GraphQLObjectType({
403403
'A GraphQL-formatted string representing the default value for this input value.',
404404
resolve(inputValue) {
405405
const { type, defaultValue } = inputValue;
406-
const valueAST = astFromValue(defaultValue, type);
407-
return valueAST ? print(valueAST) : null;
406+
if (!defaultValue) {
407+
return null;
408+
}
409+
const literal =
410+
defaultValue.literal ?? astFromValue(defaultValue.value, type);
411+
invariant(literal != null, 'Invalid default value');
412+
return print(literal);
408413
},
409414
},
410415
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>;
@@ -107,7 +108,7 @@ export class TypeInfo {
107108
return this._fieldDefStack.at(-1);
108109
}
109110

110-
getDefaultValue(): Maybe<unknown> {
111+
getDefaultValue(): GraphQLDefaultValueUsage | undefined {
111112
return this._defaultValueStack.at(-1);
112113
}
113114

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;
@@ -715,10 +720,14 @@ describe('coerceInputLiteral', () => {
715720
name: 'TestInput',
716721
fields: {
717722
int: { type: GraphQLInt, defaultValue: 42 },
723+
float: {
724+
type: GraphQLFloat,
725+
defaultValueLiteral: { kind: Kind.FLOAT, value: '3.14' },
726+
},
718727
},
719728
});
720729

721-
test('{}', type, { int: 42 });
730+
test('{}', type, { int: 42, float: 3.14 });
722731
});
723732

724733
const testInputObj = new GraphQLInputObjectType({
@@ -806,3 +815,26 @@ describe('coerceInputLiteral', () => {
806815
});
807816
});
808817
});
818+
819+
describe('coerceDefaultValue', () => {
820+
it('memoizes coercion', () => {
821+
const parseValueCalls: any = [];
822+
823+
const spyScalar = new GraphQLScalarType({
824+
name: 'SpyScalar',
825+
parseValue(value) {
826+
parseValueCalls.push(value);
827+
return value;
828+
},
829+
});
830+
831+
const defaultValueUsage = {
832+
literal: { kind: Kind.STRING, value: 'hello' },
833+
} as const;
834+
expect(coerceDefaultValue(defaultValueUsage, spyScalar)).to.equal('hello');
835+
836+
// Call a second time
837+
expect(coerceDefaultValue(defaultValueUsage, spyScalar)).to.equal('hello');
838+
expect(parseValueCalls).to.deep.equal(['hello']);
839+
});
840+
});

0 commit comments

Comments
 (0)