Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit e2002dc

Browse files
leebyronleebyron
committedJun 1, 2021
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 2f04eed commit e2002dc

17 files changed

+235
-50
lines changed
 

‎src/execution/values.ts

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import { typeFromAST } from '../utilities/typeFromAST';
2323
import {
2424
coerceInputValue,
2525
coerceInputLiteral,
26+
coerceDefaultValue,
2627
} from '../utilities/coerceInputValue';
2728

2829
type CoercedVariableValues =
@@ -179,8 +180,11 @@ export function getArgumentValues(
179180
const argumentNode = argNodeMap[name];
180181

181182
if (!argumentNode) {
182-
if (argDef.defaultValue !== undefined) {
183-
coercedValues[name] = argDef.defaultValue;
183+
if (argDef.defaultValue) {
184+
coercedValues[name] = coerceDefaultValue(
185+
argDef.defaultValue,
186+
argDef.type,
187+
);
184188
} else if (isNonNullType(argType)) {
185189
throw new GraphQLError(
186190
`Argument "${name}" of required type "${inspect(argType)}" ` +
@@ -200,8 +204,11 @@ export function getArgumentValues(
200204
variableValues == null ||
201205
!hasOwnProperty(variableValues, variableName)
202206
) {
203-
if (argDef.defaultValue !== undefined) {
204-
coercedValues[name] = argDef.defaultValue;
207+
if (argDef.defaultValue) {
208+
coercedValues[name] = coerceDefaultValue(
209+
argDef.defaultValue,
210+
argDef.type,
211+
);
205212
} else if (isNonNullType(argType)) {
206213
throw new GraphQLError(
207214
`Argument "${name}" of required type "${inspect(argType)}" ` +

‎src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,7 @@ export type {
181181
GraphQLScalarSerializer,
182182
GraphQLScalarValueParser,
183183
GraphQLScalarLiteralParser,
184+
GraphQLDefaultValueUsage,
184185
} from './type/index';
185186

186187
/** Parse and operate on GraphQL language source files. */

‎src/type/__tests__/definition-test.ts

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -826,6 +826,69 @@ describe('Type System: Input Objects', () => {
826826
);
827827
});
828828
});
829+
830+
describe('Input Object fields may have default values', () => {
831+
it('accepts an Input Object type with a default value', () => {
832+
const inputObjType = new GraphQLInputObjectType({
833+
name: 'SomeInputObject',
834+
fields: {
835+
f: { type: ScalarType, defaultValue: 3 },
836+
},
837+
});
838+
expect(inputObjType.getFields()).to.deep.equal({
839+
f: {
840+
coordinate: 'SomeInputObject.f',
841+
name: 'f',
842+
description: undefined,
843+
type: ScalarType,
844+
defaultValue: { value: 3 },
845+
deprecationReason: undefined,
846+
extensions: undefined,
847+
astNode: undefined,
848+
},
849+
});
850+
});
851+
852+
it('accepts an Input Object type with a default value literal', () => {
853+
const inputObjType = new GraphQLInputObjectType({
854+
name: 'SomeInputObject',
855+
fields: {
856+
f: {
857+
type: ScalarType,
858+
defaultValueLiteral: { kind: 'IntValue', value: '3' },
859+
},
860+
},
861+
});
862+
expect(inputObjType.getFields()).to.deep.equal({
863+
f: {
864+
coordinate: 'SomeInputObject.f',
865+
name: 'f',
866+
description: undefined,
867+
type: ScalarType,
868+
defaultValue: { literal: { kind: 'IntValue', value: '3' } },
869+
deprecationReason: undefined,
870+
extensions: undefined,
871+
astNode: undefined,
872+
},
873+
});
874+
});
875+
876+
it('rejects an Input Object type with potentially conflicting default values', () => {
877+
const inputObjType = new GraphQLInputObjectType({
878+
name: 'SomeInputObject',
879+
fields: {
880+
f: {
881+
type: ScalarType,
882+
defaultValue: 3,
883+
defaultValueLiteral: { kind: 'IntValue', value: '3' },
884+
},
885+
},
886+
});
887+
expect(() => inputObjType.getFields()).to.throw(
888+
'f has both a defaultValue and a defaultValueLiteral property, but only one must be provided.',
889+
);
890+
});
891+
});
829892
});
830893

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

‎src/type/definition.ts

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import { print } from '../language/printer';
2121
import type {
2222
FieldNode,
2323
ValueNode,
24+
ConstValueNode,
2425
OperationDefinitionNode,
2526
FragmentDefinitionNode,
2627
ScalarTypeDefinitionNode,
@@ -964,6 +965,7 @@ export interface GraphQLArgumentConfig {
964965
description?: Maybe<string>;
965966
type: GraphQLInputType;
966967
defaultValue?: unknown;
968+
defaultValueLiteral?: ConstValueNode;
967969
deprecationReason?: Maybe<string>;
968970
extensions?: Maybe<Readonly<GraphQLArgumentExtensions>>;
969971
astNode?: Maybe<InputValueDefinitionNode>;
@@ -1049,7 +1051,7 @@ export class GraphQLArgument extends GraphQLSchemaElement {
10491051
name: string;
10501052
description: Maybe<string>;
10511053
type: GraphQLInputType;
1052-
defaultValue: unknown;
1054+
defaultValue: GraphQLDefaultValueUsage | undefined;
10531055
deprecationReason: Maybe<string>;
10541056
extensions: Maybe<Readonly<GraphQLArgumentExtensions>>;
10551057
astNode: Maybe<InputValueDefinitionNode>;
@@ -1064,7 +1066,7 @@ export class GraphQLArgument extends GraphQLSchemaElement {
10641066
this.name = name;
10651067
this.description = config.description;
10661068
this.type = config.type;
1067-
this.defaultValue = config.defaultValue;
1069+
this.defaultValue = defineDefaultValue(coordinate, config);
10681070
this.deprecationReason = config.deprecationReason;
10691071
this.extensions = config.extensions && toObjMap(config.extensions);
10701072
this.astNode = config.astNode;
@@ -1074,7 +1076,8 @@ export class GraphQLArgument extends GraphQLSchemaElement {
10741076
return {
10751077
description: this.description,
10761078
type: this.type,
1077-
defaultValue: this.defaultValue,
1079+
defaultValue: this.defaultValue?.value,
1080+
defaultValueLiteral: this.defaultValue?.literal,
10781081
deprecationReason: this.deprecationReason,
10791082
extensions: this.extensions,
10801083
astNode: this.astNode,
@@ -1090,6 +1093,26 @@ export type GraphQLFieldMap<TSource, TContext> = ObjMap<
10901093
GraphQLField<TSource, TContext>
10911094
>;
10921095

1096+
export type GraphQLDefaultValueUsage =
1097+
| { value: unknown; literal?: never }
1098+
| { literal: ConstValueNode; value?: never };
1099+
1100+
function defineDefaultValue(
1101+
coordinate: string,
1102+
config: GraphQLArgumentConfig | GraphQLInputFieldConfig,
1103+
): GraphQLDefaultValueUsage | undefined {
1104+
if (config.defaultValue === undefined && !config.defaultValueLiteral) {
1105+
return;
1106+
}
1107+
devAssert(
1108+
!(config.defaultValue !== undefined && config.defaultValueLiteral),
1109+
`${coordinate} has both a defaultValue and a defaultValueLiteral property, but only one must be provided.`,
1110+
);
1111+
return config.defaultValueLiteral
1112+
? { literal: config.defaultValueLiteral }
1113+
: { value: config.defaultValue };
1114+
}
1115+
10931116
/**
10941117
* Custom extensions
10951118
*
@@ -1694,6 +1717,7 @@ export interface GraphQLInputFieldConfig {
16941717
description?: Maybe<string>;
16951718
type: GraphQLInputType;
16961719
defaultValue?: unknown;
1720+
defaultValueLiteral?: ConstValueNode;
16971721
deprecationReason?: Maybe<string>;
16981722
extensions?: Maybe<Readonly<GraphQLInputFieldExtensions>>;
16991723
astNode?: Maybe<InputValueDefinitionNode>;
@@ -1705,7 +1729,7 @@ export class GraphQLInputField extends GraphQLSchemaElement {
17051729
name: string;
17061730
description: Maybe<string>;
17071731
type: GraphQLInputType;
1708-
defaultValue: unknown;
1732+
defaultValue: GraphQLDefaultValueUsage | undefined;
17091733
deprecationReason: Maybe<string>;
17101734
extensions: Maybe<Readonly<GraphQLInputFieldExtensions>>;
17111735
astNode: Maybe<InputValueDefinitionNode>;
@@ -1726,7 +1750,7 @@ export class GraphQLInputField extends GraphQLSchemaElement {
17261750
this.name = name;
17271751
this.description = config.description;
17281752
this.type = config.type;
1729-
this.defaultValue = config.defaultValue;
1753+
this.defaultValue = defineDefaultValue(coordinate, config);
17301754
this.deprecationReason = config.deprecationReason;
17311755
this.extensions = config.extensions && toObjMap(config.extensions);
17321756
this.astNode = config.astNode;
@@ -1736,7 +1760,8 @@ export class GraphQLInputField extends GraphQLSchemaElement {
17361760
return {
17371761
description: this.description,
17381762
type: this.type,
1739-
defaultValue: this.defaultValue,
1763+
defaultValue: this.defaultValue?.value,
1764+
defaultValueLiteral: this.defaultValue?.literal,
17401765
extensions: this.extensions,
17411766
astNode: this.astNode,
17421767
};

‎src/type/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ export type {
116116
GraphQLScalarSerializer,
117117
GraphQLScalarValueParser,
118118
GraphQLScalarLiteralParser,
119+
GraphQLDefaultValueUsage,
119120
} from './definition';
120121

121122
export {

‎src/type/introspection.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -384,8 +384,13 @@ export const __InputValue: GraphQLObjectType = new GraphQLObjectType({
384384
'A GraphQL-formatted string representing the default value for this input value.',
385385
resolve(inputValue) {
386386
const { type, defaultValue } = inputValue;
387-
const valueAST = astFromValue(defaultValue, type);
388-
return valueAST ? print(valueAST) : null;
387+
if (!defaultValue) {
388+
return null;
389+
}
390+
const literal =
391+
defaultValue.literal ?? astFromValue(defaultValue.value, type);
392+
invariant(literal, 'Invalid default value');
393+
return print(literal);
389394
},
390395
},
391396
isDeprecated: {

‎src/utilities/TypeInfo.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import type {
1717
GraphQLArgument,
1818
GraphQLInputField,
1919
GraphQLEnumValue,
20+
GraphQLDefaultValueUsage,
2021
} from '../type/definition';
2122
import {
2223
isObjectType,
@@ -49,7 +50,7 @@ export class TypeInfo {
4950
private _parentTypeStack: Array<Maybe<GraphQLCompositeType>>;
5051
private _inputTypeStack: Array<Maybe<GraphQLInputType>>;
5152
private _fieldDefStack: Array<Maybe<GraphQLField<unknown, unknown>>>;
52-
private _defaultValueStack: Array<Maybe<unknown>>;
53+
private _defaultValueStack: Array<GraphQLDefaultValueUsage | undefined>;
5354
private _directive: Maybe<GraphQLDirective>;
5455
private _argument: Maybe<GraphQLArgument>;
5556
private _enumValue: Maybe<GraphQLEnumValue>;
@@ -119,7 +120,7 @@ export class TypeInfo {
119120
}
120121
}
121122

122-
getDefaultValue(): Maybe<unknown> {
123+
getDefaultValue(): GraphQLDefaultValueUsage | undefined {
123124
if (this._defaultValueStack.length > 0) {
124125
return this._defaultValueStack[this._defaultValueStack.length - 1];
125126
}

‎src/utilities/__tests__/buildClientSchema-test.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -444,6 +444,7 @@ describe('Type System: build schema from introspection', () => {
444444
}
445445
446446
type Query {
447+
defaultID(intArg: ID = "123"): String
447448
defaultInt(intArg: Int = 30): String
448449
defaultList(listArg: [Int] = [1, 2, 3]): String
449450
defaultObject(objArg: Geo = {lat: 37.485, lon: -122.148}): String
@@ -599,6 +600,28 @@ describe('Type System: build schema from introspection', () => {
599600
expect(result.data).to.deep.equal({ foo: 'bar' });
600601
});
601602

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

‎src/utilities/__tests__/coerceInputValue-test.ts

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,11 @@ import {
2424
GraphQLInputObjectType,
2525
} from '../../type/definition';
2626

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

2933
interface CoerceResult {
3034
value: unknown;
@@ -610,10 +614,14 @@ describe('coerceInputLiteral', () => {
610614
name: 'TestInput',
611615
fields: {
612616
int: { type: GraphQLInt, defaultValue: 42 },
617+
float: {
618+
type: GraphQLFloat,
619+
defaultValueLiteral: { kind: 'FloatValue', value: '3.14' },
620+
},
613621
},
614622
});
615623

616-
test('{}', type, { int: 42 });
624+
test('{}', type, { int: 42, float: 3.14 });
617625
});
618626

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

‎src/utilities/astFromValue.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { isObjectLike } from '../jsutils/isObjectLike';
44
import { isIterableObject } from '../jsutils/isIterableObject';
55
import type { Maybe } from '../jsutils/Maybe';
66

7-
import type { ValueNode } from '../language/ast';
7+
import type { ConstValueNode } from '../language/ast';
88
import { Kind } from '../language/kinds';
99

1010
import type { GraphQLInputType } from '../type/definition';
@@ -41,7 +41,7 @@ import {
4141
export function astFromValue(
4242
value: unknown,
4343
type: GraphQLInputType,
44-
): Maybe<ValueNode> {
44+
): Maybe<ConstValueNode> {
4545
if (isNonNullType(type)) {
4646
const astValue = astFromValue(value, type.ofType);
4747
if (astValue?.kind === Kind.NULL) {

0 commit comments

Comments
 (0)
Please sign in to comment.