Skip to content

Commit f6feca9

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 b80ff6d commit f6feca9

18 files changed

+242
-54
lines changed

src/execution/getVariableSignature.ts

+7-6
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,12 @@ import type { VariableDefinitionNode } from '../language/ast.js';
44
import { print } from '../language/printer.js';
55

66
import { isInputType } from '../type/definition.js';
7-
import type { GraphQLInputType, GraphQLSchema } from '../type/index.js';
7+
import type {
8+
GraphQLDefaultValueUsage,
9+
GraphQLInputType,
10+
GraphQLSchema,
11+
} from '../type/index.js';
812

9-
import { coerceInputLiteral } from '../utilities/coerceInputValue.js';
1013
import { typeFromAST } from '../utilities/typeFromAST.js';
1114

1215
/**
@@ -18,7 +21,7 @@ import { typeFromAST } from '../utilities/typeFromAST.js';
1821
export interface GraphQLVariableSignature {
1922
name: string;
2023
type: GraphQLInputType;
21-
defaultValue: unknown;
24+
defaultValue: GraphQLDefaultValueUsage | undefined;
2225
}
2326

2427
export function getVariableSignature(
@@ -43,8 +46,6 @@ export function getVariableSignature(
4346
return {
4447
name: varName,
4548
type: varType,
46-
defaultValue: defaultValue
47-
? coerceInputLiteral(varDefNode.defaultValue, varType)
48-
: undefined,
49+
defaultValue: defaultValue ? { literal: defaultValue } : undefined,
4950
};
5051
}

src/execution/values.ts

+16-6
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';
@@ -90,8 +91,11 @@ function coerceVariableValues(
9091

9192
const { name: varName, type: varType } = varSignature;
9293
if (!Object.hasOwn(inputs, varName)) {
93-
if (varDefNode.defaultValue) {
94-
coercedValues[varName] = varSignature.defaultValue;
94+
if (varSignature.defaultValue) {
95+
coercedValues[varName] = coerceDefaultValue(
96+
varSignature.defaultValue,
97+
varType,
98+
);
9599
} else if (isNonNullType(varType)) {
96100
const varTypeStr = inspect(varType);
97101
onError(
@@ -173,8 +177,11 @@ export function experimentalGetArgumentValues(
173177
const argumentNode = argNodeMap.get(name);
174178

175179
if (argumentNode == null) {
176-
if (argDef.defaultValue !== undefined) {
177-
coercedValues[name] = argDef.defaultValue;
180+
if (argDef.defaultValue) {
181+
coercedValues[name] = coerceDefaultValue(
182+
argDef.defaultValue,
183+
argDef.type,
184+
);
178185
} else if (isNonNullType(argType)) {
179186
throw new GraphQLError(
180187
`Argument "${name}" of required type "${inspect(argType)}" ` +
@@ -197,8 +204,11 @@ export function experimentalGetArgumentValues(
197204
scopedVariableValues == null ||
198205
!Object.hasOwn(scopedVariableValues, variableName)
199206
) {
200-
if (argDef.defaultValue !== undefined) {
201-
coercedValues[name] = argDef.defaultValue;
207+
if (argDef.defaultValue) {
208+
coercedValues[name] = coerceDefaultValue(
209+
argDef.defaultValue,
210+
argDef.type,
211+
);
202212
} else if (isNonNullType(argType)) {
203213
throw new GraphQLError(
204214
`Argument "${name}" of required type "${inspect(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

+58
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';
@@ -581,6 +582,63 @@ describe('Type System: Input Objects', () => {
581582
'not used anymore',
582583
);
583584
});
585+
586+
describe('Input Object fields may have default values', () => {
587+
it('accepts an Input Object type with a default value', () => {
588+
const inputObjType = new GraphQLInputObjectType({
589+
name: 'SomeInputObject',
590+
fields: {
591+
f: { type: ScalarType, defaultValue: 3 },
592+
},
593+
});
594+
expect(inputObjType.getFields().f).to.deep.include({
595+
name: 'f',
596+
description: undefined,
597+
type: ScalarType,
598+
defaultValue: { value: 3 },
599+
deprecationReason: undefined,
600+
extensions: {},
601+
astNode: undefined,
602+
});
603+
});
604+
605+
it('accepts an Input Object type with a default value literal', () => {
606+
const inputObjType = new GraphQLInputObjectType({
607+
name: 'SomeInputObject',
608+
fields: {
609+
f: {
610+
type: ScalarType,
611+
defaultValueLiteral: { kind: Kind.INT, value: '3' },
612+
},
613+
},
614+
});
615+
expect(inputObjType.getFields().f).to.deep.include({
616+
name: 'f',
617+
description: undefined,
618+
type: ScalarType,
619+
defaultValue: { literal: { kind: 'IntValue', value: '3' } },
620+
deprecationReason: undefined,
621+
extensions: {},
622+
astNode: undefined,
623+
});
624+
});
625+
626+
it('rejects an Input Object type with potentially conflicting default values', () => {
627+
const inputObjType = new GraphQLInputObjectType({
628+
name: 'SomeInputObject',
629+
fields: {
630+
f: {
631+
type: ScalarType,
632+
defaultValue: 3,
633+
defaultValueLiteral: { kind: Kind.INT, value: '3' },
634+
},
635+
},
636+
});
637+
expect(() => inputObjType.getFields()).to.throw(
638+
'Argument "f" has both a defaultValue and a defaultValueLiteral property, but only one must be provided.',
639+
);
640+
});
641+
});
584642
});
585643

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

src/type/__tests__/predicate-test.ts

+8-2
Original file line numberDiff line numberDiff line change
@@ -574,7 +574,10 @@ describe('Type predicates', () => {
574574
name: 'someArg',
575575
type: config.type,
576576
description: undefined,
577-
defaultValue: config.defaultValue,
577+
defaultValue:
578+
config.defaultValue !== undefined
579+
? { value: config.defaultValue }
580+
: undefined,
578581
deprecationReason: null,
579582
extensions: Object.create(null),
580583
astNode: undefined,
@@ -622,7 +625,10 @@ describe('Type predicates', () => {
622625
name: 'someInputField',
623626
type: config.type,
624627
description: undefined,
625-
defaultValue: config.defaultValue,
628+
defaultValue:
629+
config.defaultValue !== undefined
630+
? { value: config.defaultValue }
631+
: undefined,
626632
deprecationReason: null,
627633
extensions: Object.create(null),
628634
astNode: undefined,

src/type/definition.ts

+27-4
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,
@@ -799,7 +800,7 @@ export function defineArguments(
799800
name: assertName(argName),
800801
description: argConfig.description,
801802
type: argConfig.type,
802-
defaultValue: argConfig.defaultValue,
803+
defaultValue: defineDefaultValue(argName, argConfig),
803804
deprecationReason: argConfig.deprecationReason,
804805
extensions: toObjMap(argConfig.extensions),
805806
astNode: argConfig.astNode,
@@ -946,6 +947,7 @@ export interface GraphQLArgumentConfig {
946947
description?: Maybe<string>;
947948
type: GraphQLInputType;
948949
defaultValue?: unknown;
950+
defaultValueLiteral?: ConstValueNode | undefined;
949951
deprecationReason?: Maybe<string>;
950952
extensions?: Maybe<Readonly<GraphQLArgumentExtensions>>;
951953
astNode?: Maybe<InputValueDefinitionNode>;
@@ -971,7 +973,7 @@ export interface GraphQLArgument {
971973
name: string;
972974
description: Maybe<string>;
973975
type: GraphQLInputType;
974-
defaultValue: unknown;
976+
defaultValue: GraphQLDefaultValueUsage | undefined;
975977
deprecationReason: Maybe<string>;
976978
extensions: Readonly<GraphQLArgumentExtensions>;
977979
astNode: Maybe<InputValueDefinitionNode>;
@@ -985,6 +987,26 @@ export type GraphQLFieldMap<TSource, TContext> = ObjMap<
985987
GraphQLField<TSource, TContext>
986988
>;
987989

990+
export type GraphQLDefaultValueUsage =
991+
| { value: unknown; literal?: never }
992+
| { literal: ConstValueNode; value?: never };
993+
994+
export function defineDefaultValue(
995+
argName: string,
996+
config: GraphQLArgumentConfig | GraphQLInputFieldConfig,
997+
): GraphQLDefaultValueUsage | undefined {
998+
if (config.defaultValue === undefined && !config.defaultValueLiteral) {
999+
return;
1000+
}
1001+
devAssert(
1002+
!(config.defaultValue !== undefined && config.defaultValueLiteral),
1003+
`Argument "${argName}" has both a defaultValue and a defaultValueLiteral property, but only one must be provided.`,
1004+
);
1005+
return config.defaultValueLiteral
1006+
? { literal: config.defaultValueLiteral }
1007+
: { value: config.defaultValue };
1008+
}
1009+
9881010
/**
9891011
* Custom extensions
9901012
*
@@ -1572,7 +1594,7 @@ function defineInputFieldMap(
15721594
name: assertName(fieldName),
15731595
description: fieldConfig.description,
15741596
type: fieldConfig.type,
1575-
defaultValue: fieldConfig.defaultValue,
1597+
defaultValue: defineDefaultValue(fieldName, fieldConfig),
15761598
deprecationReason: fieldConfig.deprecationReason,
15771599
extensions: toObjMap(fieldConfig.extensions),
15781600
astNode: fieldConfig.astNode,
@@ -1613,6 +1635,7 @@ export interface GraphQLInputFieldConfig {
16131635
description?: Maybe<string>;
16141636
type: GraphQLInputType;
16151637
defaultValue?: unknown;
1638+
defaultValueLiteral?: ConstValueNode | undefined;
16161639
deprecationReason?: Maybe<string>;
16171640
extensions?: Maybe<Readonly<GraphQLInputFieldExtensions>>;
16181641
astNode?: Maybe<InputValueDefinitionNode>;
@@ -1624,7 +1647,7 @@ export interface GraphQLInputField {
16241647
name: string;
16251648
description: Maybe<string>;
16261649
type: GraphQLInputType;
1627-
defaultValue: unknown;
1650+
defaultValue: GraphQLDefaultValueUsage | undefined;
16281651
deprecationReason: Maybe<string>;
16291652
extensions: Readonly<GraphQLInputFieldExtensions>;
16301653
astNode: Maybe<InputValueDefinitionNode>;

src/type/index.ts

+1
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,7 @@ export type {
119119
GraphQLScalarSerializer,
120120
GraphQLScalarValueParser,
121121
GraphQLScalarLiteralParser,
122+
GraphQLDefaultValueUsage,
122123
} from './definition.js';
123124

124125
export {

src/type/introspection.ts

+7-2
Original file line numberDiff line numberDiff line change
@@ -407,8 +407,13 @@ export const __InputValue: GraphQLObjectType = new GraphQLObjectType({
407407
'A GraphQL-formatted string representing the default value for this input value.',
408408
resolve(inputValue) {
409409
const { type, defaultValue } = inputValue;
410-
const valueAST = astFromValue(defaultValue, type);
411-
return valueAST ? print(valueAST) : null;
410+
if (!defaultValue) {
411+
return null;
412+
}
413+
const literal =
414+
defaultValue.literal ?? astFromValue(defaultValue.value, type);
415+
invariant(literal != null, 'Invalid default value');
416+
return print(literal);
412417
},
413418
},
414419
isDeprecated: {

src/utilities/TypeInfo.ts

+3-2
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import { getEnterLeaveForKind } from '../language/visitor.js';
1414
import type {
1515
GraphQLArgument,
1616
GraphQLCompositeType,
17+
GraphQLDefaultValueUsage,
1718
GraphQLEnumValue,
1819
GraphQLField,
1920
GraphQLInputField,
@@ -53,7 +54,7 @@ export class TypeInfo {
5354
private _parentTypeStack: Array<Maybe<GraphQLCompositeType>>;
5455
private _inputTypeStack: Array<Maybe<GraphQLInputType>>;
5556
private _fieldDefStack: Array<Maybe<GraphQLField<unknown, unknown>>>;
56-
private _defaultValueStack: Array<Maybe<unknown>>;
57+
private _defaultValueStack: Array<GraphQLDefaultValueUsage | undefined>;
5758
private _directive: Maybe<GraphQLDirective>;
5859
private _argument: Maybe<GraphQLArgument>;
5960
private _enumValue: Maybe<GraphQLEnumValue>;
@@ -124,7 +125,7 @@ export class TypeInfo {
124125
return this._fieldDefStack.at(-1);
125126
}
126127

127-
getDefaultValue(): Maybe<unknown> {
128+
getDefaultValue(): GraphQLDefaultValueUsage | undefined {
128129
return this._defaultValueStack.at(-1);
129130
}
130131

src/utilities/__tests__/buildClientSchema-test.ts

+23
Original file line numberDiff line numberDiff line change
@@ -439,6 +439,7 @@ describe('Type System: build schema from introspection', () => {
439439
}
440440
441441
type Query {
442+
defaultID(intArg: ID = "123"): String
442443
defaultInt(intArg: Int = 30): String
443444
defaultList(listArg: [Int] = [1, 2, 3]): String
444445
defaultObject(objArg: Geo = { lat: 37.485, lon: -122.148 }): String
@@ -609,6 +610,28 @@ describe('Type System: build schema from introspection', () => {
609610
expect(result.data).to.deep.equal({ foo: 'bar' });
610611
});
611612

613+
it('can use client schema for execution if resolvers are added', () => {
614+
const schema = buildSchema(`
615+
type Query {
616+
foo(bar: String = "abc"): String
617+
}
618+
`);
619+
620+
const introspection = introspectionFromSchema(schema);
621+
const clientSchema = buildClientSchema(introspection);
622+
623+
const QueryType: GraphQLObjectType = clientSchema.getType('Query') as any;
624+
QueryType.getFields().foo.resolve = (_value, args) => args.bar;
625+
626+
const result = graphqlSync({
627+
schema: clientSchema,
628+
source: '{ foo }',
629+
});
630+
631+
expect(result.data).to.deep.equal({ foo: 'abc' });
632+
expect(result.data).to.deep.equal({ foo: 'abc' });
633+
});
634+
612635
it('can build invalid schema', () => {
613636
const schema = buildSchema('type Query', { assumeValid: true });
614637

0 commit comments

Comments
 (0)