Skip to content

Commit 53748bc

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 50ef860 commit 53748bc

19 files changed

+249
-56
lines changed

src/execution/getVariableSignature.ts

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

Lines changed: 16 additions & 6 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';
@@ -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

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: 58 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';
@@ -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

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

Lines changed: 27 additions & 4 deletions
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,
@@ -802,7 +803,7 @@ export function defineArguments(
802803
name: assertName(argName),
803804
description: argConfig.description,
804805
type: argConfig.type,
805-
defaultValue: argConfig.defaultValue,
806+
defaultValue: defineDefaultValue(argName, argConfig),
806807
deprecationReason: argConfig.deprecationReason,
807808
extensions: toObjMap(argConfig.extensions),
808809
astNode: argConfig.astNode,
@@ -949,6 +950,7 @@ export interface GraphQLArgumentConfig {
949950
description?: Maybe<string>;
950951
type: GraphQLInputType;
951952
defaultValue?: unknown;
953+
defaultValueLiteral?: ConstValueNode | undefined;
952954
deprecationReason?: Maybe<string>;
953955
extensions?: Maybe<Readonly<GraphQLArgumentExtensions>>;
954956
astNode?: Maybe<InputValueDefinitionNode>;
@@ -974,7 +976,7 @@ export interface GraphQLArgument {
974976
name: string;
975977
description: Maybe<string>;
976978
type: GraphQLInputType;
977-
defaultValue: unknown;
979+
defaultValue: GraphQLDefaultValueUsage | undefined;
978980
deprecationReason: Maybe<string>;
979981
extensions: Readonly<GraphQLArgumentExtensions>;
980982
astNode: Maybe<InputValueDefinitionNode>;
@@ -988,6 +990,26 @@ export type GraphQLFieldMap<TSource, TContext> = ObjMap<
988990
GraphQLField<TSource, TContext>
989991
>;
990992

993+
export type GraphQLDefaultValueUsage =
994+
| { value: unknown; literal?: never }
995+
| { literal: ConstValueNode; value?: never };
996+
997+
export function defineDefaultValue(
998+
argName: string,
999+
config: GraphQLArgumentConfig | GraphQLInputFieldConfig,
1000+
): GraphQLDefaultValueUsage | undefined {
1001+
if (config.defaultValue === undefined && !config.defaultValueLiteral) {
1002+
return;
1003+
}
1004+
devAssert(
1005+
!(config.defaultValue !== undefined && config.defaultValueLiteral),
1006+
`Argument "${argName}" has both a defaultValue and a defaultValueLiteral property, but only one must be provided.`,
1007+
);
1008+
return config.defaultValueLiteral
1009+
? { literal: config.defaultValueLiteral }
1010+
: { value: config.defaultValue };
1011+
}
1012+
9911013
/**
9921014
* Custom extensions
9931015
*
@@ -1576,7 +1598,7 @@ function defineInputFieldMap(
15761598
name: assertName(fieldName),
15771599
description: fieldConfig.description,
15781600
type: fieldConfig.type,
1579-
defaultValue: fieldConfig.defaultValue,
1601+
defaultValue: defineDefaultValue(fieldName, fieldConfig),
15801602
deprecationReason: fieldConfig.deprecationReason,
15811603
extensions: toObjMap(fieldConfig.extensions),
15821604
astNode: fieldConfig.astNode,
@@ -1617,6 +1639,7 @@ export interface GraphQLInputFieldConfig {
16171639
description?: Maybe<string>;
16181640
type: GraphQLInputType;
16191641
defaultValue?: unknown;
1642+
defaultValueLiteral?: ConstValueNode | undefined;
16201643
deprecationReason?: Maybe<string>;
16211644
extensions?: Maybe<Readonly<GraphQLInputFieldExtensions>>;
16221645
astNode?: Maybe<InputValueDefinitionNode>;
@@ -1628,7 +1651,7 @@ export interface GraphQLInputField {
16281651
name: string;
16291652
description: Maybe<string>;
16301653
type: GraphQLInputType;
1631-
defaultValue: unknown;
1654+
defaultValue: GraphQLDefaultValueUsage | undefined;
16321655
deprecationReason: Maybe<string>;
16331656
extensions: Readonly<GraphQLInputFieldExtensions>;
16341657
astNode: Maybe<InputValueDefinitionNode>;

src/type/index.ts

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

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

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

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