Skip to content

Commit bd2fe71

Browse files
yaacovCRleebyron
andauthored
Preserve defaultValue literals (#3810)
[#3074 rebased on main](#3074). Depends on #3809 @leebyron comments from original PR (edited, hopefully correctly): > 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 (EDIT: but not both!) "value" and "literal" fields. > > 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) > ``` Co-authored-by: Lee Byron <[email protected]>
1 parent e52ed9a commit bd2fe71

19 files changed

+247
-57
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

+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,
@@ -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,
@@ -833,7 +834,8 @@ export function argsToArgsConfig(
833834
(arg) => ({
834835
description: arg.description,
835836
type: arg.type,
836-
defaultValue: arg.defaultValue,
837+
defaultValue: arg.defaultValue?.value,
838+
defaultValueLiteral: arg.defaultValue?.literal,
837839
deprecationReason: arg.deprecationReason,
838840
extensions: arg.extensions,
839841
astNode: arg.astNode,
@@ -946,6 +948,7 @@ export interface GraphQLArgumentConfig {
946948
description?: Maybe<string>;
947949
type: GraphQLInputType;
948950
defaultValue?: unknown;
951+
defaultValueLiteral?: ConstValueNode | undefined;
949952
deprecationReason?: Maybe<string>;
950953
extensions?: Maybe<Readonly<GraphQLArgumentExtensions>>;
951954
astNode?: Maybe<InputValueDefinitionNode>;
@@ -971,7 +974,7 @@ export interface GraphQLArgument {
971974
name: string;
972975
description: Maybe<string>;
973976
type: GraphQLInputType;
974-
defaultValue: unknown;
977+
defaultValue: GraphQLDefaultValueUsage | undefined;
975978
deprecationReason: Maybe<string>;
976979
extensions: Readonly<GraphQLArgumentExtensions>;
977980
astNode: Maybe<InputValueDefinitionNode>;
@@ -985,6 +988,26 @@ export type GraphQLFieldMap<TSource, TContext> = ObjMap<
985988
GraphQLField<TSource, TContext>
986989
>;
987990

991+
export type GraphQLDefaultValueUsage =
992+
| { value: unknown; literal?: never }
993+
| { literal: ConstValueNode; value?: never };
994+
995+
export function defineDefaultValue(
996+
argName: string,
997+
config: GraphQLArgumentConfig | GraphQLInputFieldConfig,
998+
): GraphQLDefaultValueUsage | undefined {
999+
if (config.defaultValue === undefined && !config.defaultValueLiteral) {
1000+
return;
1001+
}
1002+
devAssert(
1003+
!(config.defaultValue !== undefined && config.defaultValueLiteral),
1004+
`Argument "${argName}" has both a defaultValue and a defaultValueLiteral property, but only one must be provided.`,
1005+
);
1006+
return config.defaultValueLiteral
1007+
? { literal: config.defaultValueLiteral }
1008+
: { value: config.defaultValue };
1009+
}
1010+
9881011
/**
9891012
* Custom extensions
9901013
*
@@ -1538,7 +1561,8 @@ export class GraphQLInputObjectType {
15381561
const fields = mapValue(this.getFields(), (field) => ({
15391562
description: field.description,
15401563
type: field.type,
1541-
defaultValue: field.defaultValue,
1564+
defaultValue: field.defaultValue?.value,
1565+
defaultValueLiteral: field.defaultValue?.literal,
15421566
deprecationReason: field.deprecationReason,
15431567
extensions: field.extensions,
15441568
astNode: field.astNode,
@@ -1572,7 +1596,7 @@ function defineInputFieldMap(
15721596
name: assertName(fieldName),
15731597
description: fieldConfig.description,
15741598
type: fieldConfig.type,
1575-
defaultValue: fieldConfig.defaultValue,
1599+
defaultValue: defineDefaultValue(fieldName, fieldConfig),
15761600
deprecationReason: fieldConfig.deprecationReason,
15771601
extensions: toObjMap(fieldConfig.extensions),
15781602
astNode: fieldConfig.astNode,
@@ -1613,6 +1637,7 @@ export interface GraphQLInputFieldConfig {
16131637
description?: Maybe<string>;
16141638
type: GraphQLInputType;
16151639
defaultValue?: unknown;
1640+
defaultValueLiteral?: ConstValueNode | undefined;
16161641
deprecationReason?: Maybe<string>;
16171642
extensions?: Maybe<Readonly<GraphQLInputFieldExtensions>>;
16181643
astNode?: Maybe<InputValueDefinitionNode>;
@@ -1624,7 +1649,7 @@ export interface GraphQLInputField {
16241649
name: string;
16251650
description: Maybe<string>;
16261651
type: GraphQLInputType;
1627-
defaultValue: unknown;
1652+
defaultValue: GraphQLDefaultValueUsage | undefined;
16281653
deprecationReason: Maybe<string>;
16291654
extensions: Readonly<GraphQLInputFieldExtensions>;
16301655
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)