Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ease upgrade path for programmatic default values #4296

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion integrationTests/ts/basic-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ const queryType: GraphQLObjectType = new GraphQLObjectType({
args: {
who: {
type: GraphQLString,
defaultValue: 'World',
externalDefaultValue: 'World',
},
},
resolve(_root, args: { who: string }) {
Expand Down
2 changes: 1 addition & 1 deletion integrationTests/ts/esm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ const queryType: GraphQLObjectType = new GraphQLObjectType({
args: {
who: {
type: GraphQLString,
defaultValue: 'World',
externalDefaultValue: 'World',
},
},
resolve(_root, args: { who: string }) {
Expand Down
2 changes: 1 addition & 1 deletion src/execution/__tests__/executor-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ describe('Execute: Handles basic execution tasks', () => {
signature: {
name: 'var',
type: GraphQLString,
defaultValue: undefined,
externalDefaultValue: undefined,
},
value: 'abc',
},
Expand Down
6 changes: 3 additions & 3 deletions src/execution/__tests__/variables-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,11 +142,11 @@ const TestType = new GraphQLObjectType({
}),
fieldWithDefaultArgumentValue: fieldWithInputArg({
type: GraphQLString,
defaultValue: 'Hello World',
externalDefaultValue: 'Hello World',
}),
fieldWithNonNullableStringInputAndDefaultArgumentValue: fieldWithInputArg({
type: new GraphQLNonNull(GraphQLString),
defaultValue: 'Hello World',
externalDefaultValue: 'Hello World',
}),
fieldWithNestedInputObject: fieldWithInputArg({
type: TestNestedInputObject,
Expand Down Expand Up @@ -187,7 +187,7 @@ const schema = new GraphQLSchema({
type: new GraphQLNonNull(GraphQLBoolean),
description: 'Skipped when true.',
// default values will override operation variables in the setting of defined fragment variables that are not provided
defaultValue: true,
externalDefaultValue: true,
},
},
}),
Expand Down
16 changes: 8 additions & 8 deletions src/execution/getVariableSignature.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
import { GraphQLError } from '../error/GraphQLError.js';

import type { VariableDefinitionNode } from '../language/ast.js';
import type {
ConstValueNode,
VariableDefinitionNode,
} from '../language/ast.js';
import { print } from '../language/printer.js';

import { isInputType } from '../type/definition.js';
import type {
GraphQLDefaultValueUsage,
GraphQLInputType,
GraphQLSchema,
} from '../type/index.js';
import type { GraphQLInputType, GraphQLSchema } from '../type/index.js';

import { typeFromAST } from '../utilities/typeFromAST.js';

Expand All @@ -21,7 +20,8 @@ import { typeFromAST } from '../utilities/typeFromAST.js';
export interface GraphQLVariableSignature {
name: string;
type: GraphQLInputType;
defaultValue: GraphQLDefaultValueUsage | undefined;
defaultValue?: never;
externalDefaultValue: { literal: ConstValueNode } | undefined;
}

export function getVariableSignature(
Expand All @@ -46,6 +46,6 @@ export function getVariableSignature(
return {
name: varName,
type: varType,
defaultValue: defaultValue ? { literal: defaultValue } : undefined,
externalDefaultValue: defaultValue ? { literal: defaultValue } : undefined,
};
}
16 changes: 6 additions & 10 deletions src/execution/values.ts
Original file line number Diff line number Diff line change
Expand Up @@ -231,11 +231,9 @@ export function experimentalGetArgumentValues(
{ nodes: node },
);
}
if (argDef.defaultValue) {
coercedValues[name] = coerceDefaultValue(
argDef.defaultValue,
argDef.type,
);
const coercedDefaultValue = coerceDefaultValue(argDef);
if (coercedDefaultValue !== undefined) {
coercedValues[name] = coercedDefaultValue;
}
continue;
}
Expand All @@ -254,11 +252,9 @@ export function experimentalGetArgumentValues(
!Object.hasOwn(scopedVariableValues.coerced, variableName)) &&
!isRequiredArgument(argDef)
) {
if (argDef.defaultValue) {
coercedValues[name] = coerceDefaultValue(
argDef.defaultValue,
argDef.type,
);
const coercedDefaultValue = coerceDefaultValue(argDef);
if (coercedDefaultValue !== undefined) {
coercedValues[name] = coercedDefaultValue;
}
continue;
}
Expand Down
22 changes: 15 additions & 7 deletions src/type/__tests__/definition-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,8 @@ describe('Type System: Objects', () => {
input: {
description: 'Argument description.',
type: ScalarType,
defaultValue: 'DefaultValue',
defaultValue: undefined,
externalDefaultValue: 'DefaultValue',
defaultValueLiteral: undefined,
deprecationReason: 'Argument deprecation reason.',
extensions: { someExtension: 'extension' },
Expand Down Expand Up @@ -377,6 +378,7 @@ describe('Type System: Objects', () => {
description: undefined,
type: ScalarType,
defaultValue: undefined,
externalDefaultValue: undefined,
deprecationReason: undefined,
extensions: {},
astNode: undefined,
Expand Down Expand Up @@ -493,6 +495,7 @@ describe('Type System: Interfaces', () => {
description: 'Argument description.',
type: ScalarType,
defaultValue: undefined,
externalDefaultValue: undefined,
defaultValueLiteral: dummyAny,
deprecationReason: 'Argument deprecation reason.',
extensions: { someExtension: 'extension' },
Expand Down Expand Up @@ -830,7 +833,8 @@ describe('Type System: Input Objects', () => {
input: {
description: 'Argument description.',
type: ScalarType,
defaultValue: 'DefaultValue',
defaultValue: undefined,
externalDefaultValue: 'DefaultValue',
defaultValueLiteral: undefined,
deprecationReason: 'Argument deprecation reason.',
extensions: { someExtension: 'extension' },
Expand Down Expand Up @@ -860,6 +864,7 @@ describe('Type System: Input Objects', () => {
description: undefined,
type: ScalarType,
defaultValue: undefined,
externalDefaultValue: undefined,
deprecationReason: undefined,
extensions: {},
astNode: undefined,
Expand All @@ -879,6 +884,7 @@ describe('Type System: Input Objects', () => {
description: undefined,
type: ScalarType,
defaultValue: undefined,
externalDefaultValue: undefined,
extensions: {},
deprecationReason: undefined,
astNode: undefined,
Expand Down Expand Up @@ -927,14 +933,15 @@ describe('Type System: Input Objects', () => {
const inputObjType = new GraphQLInputObjectType({
name: 'SomeInputObject',
fields: {
f: { type: ScalarType, defaultValue: 3 },
f: { type: ScalarType, externalDefaultValue: 3 },
},
});
expect(inputObjType.getFields().f).to.deep.include({
name: 'f',
description: undefined,
type: ScalarType,
defaultValue: { value: 3 },
defaultValue: undefined,
externalDefaultValue: { value: 3 },
deprecationReason: undefined,
extensions: {},
astNode: undefined,
Expand All @@ -955,7 +962,8 @@ describe('Type System: Input Objects', () => {
name: 'f',
description: undefined,
type: ScalarType,
defaultValue: { literal: { kind: 'IntValue', value: '3' } },
defaultValue: undefined,
externalDefaultValue: { literal: { kind: 'IntValue', value: '3' } },
deprecationReason: undefined,
extensions: {},
astNode: undefined,
Expand All @@ -968,13 +976,13 @@ describe('Type System: Input Objects', () => {
fields: {
f: {
type: ScalarType,
defaultValue: 3,
externalDefaultValue: 3,
defaultValueLiteral: { kind: Kind.INT, value: '3' },
},
},
});
expect(() => inputObjType.getFields()).to.throw(
'Argument "f" has both a defaultValue and a defaultValueLiteral property, but only one must be provided.',
'Argument "f" has both an externalDefaultValue and a defaultValueLiteral property, but only one must be provided.',
);
});
});
Expand Down
2 changes: 2 additions & 0 deletions src/type/__tests__/directive-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ describe('Type System: Directive', () => {
description: undefined,
type: GraphQLString,
defaultValue: undefined,
externalDefaultValue: undefined,
deprecationReason: undefined,
extensions: {},
astNode: undefined,
Expand All @@ -56,6 +57,7 @@ describe('Type System: Directive', () => {
description: undefined,
type: GraphQLInt,
defaultValue: undefined,
externalDefaultValue: undefined,
deprecationReason: undefined,
extensions: {},
astNode: undefined,
Expand Down
28 changes: 27 additions & 1 deletion src/type/__tests__/enumType-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ const QueryType = new GraphQLObjectType({
args: {
fromEnum: {
type: ComplexEnum,
defaultValue: 'ONE',
externalDefaultValue: 'ONE',
},
provideGoodValue: { type: GraphQLBoolean },
provideBadValue: { type: GraphQLBoolean },
Expand All @@ -90,6 +90,18 @@ const QueryType = new GraphQLObjectType({
return fromEnum;
},
},
complexEnumWithLegacyDefault: {
type: ComplexEnum,
args: {
fromEnum: {
type: ComplexEnum,
defaultValue: Complex1,
},
},
resolve(_source, { fromEnum }) {
return fromEnum;
},
},
thunkValuesString: {
type: GraphQLString,
args: {
Expand Down Expand Up @@ -442,6 +454,20 @@ describe('Type System: Enum Values', () => {
});
});

it('may be internally represented with complex values using legacy internal defaults', () => {
const result = executeQuery(`
{
complexEnumWithLegacyDefault
}
`);

expectJSON(result).toDeepEqual({
data: {
complexEnumWithLegacyDefault: 'ONE',
},
});
});

it('may have values specified via a callback', () => {
const result = executeQuery('{ thunkValuesString(fromEnum: B) }');

Expand Down
12 changes: 6 additions & 6 deletions src/type/__tests__/predicate-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -634,7 +634,7 @@ describe('Type predicates', () => {
describe('isRequiredArgument', () => {
function buildArg(config: {
type: GraphQLInputType;
defaultValue?: unknown;
externalDefaultValue?: unknown;
}): GraphQLArgument {
const objectType = new GraphQLObjectType({
name: 'SomeType',
Expand All @@ -660,7 +660,7 @@ describe('Type predicates', () => {

const optArg2 = buildArg({
type: GraphQLString,
defaultValue: null,
externalDefaultValue: null,
});
expect(isRequiredArgument(optArg2)).to.equal(false);

Expand All @@ -671,7 +671,7 @@ describe('Type predicates', () => {

const optArg4 = buildArg({
type: new GraphQLNonNull(GraphQLString),
defaultValue: 'default',
externalDefaultValue: 'default',
});
expect(isRequiredArgument(optArg4)).to.equal(false);
});
Expand All @@ -680,7 +680,7 @@ describe('Type predicates', () => {
describe('isRequiredInputField', () => {
function buildInputField(config: {
type: GraphQLInputType;
defaultValue?: unknown;
externalDefaultValue?: unknown;
}): GraphQLInputField {
const inputObjectType = new GraphQLInputObjectType({
name: 'SomeType',
Expand All @@ -706,7 +706,7 @@ describe('Type predicates', () => {

const optField2 = buildInputField({
type: GraphQLString,
defaultValue: null,
externalDefaultValue: null,
});
expect(isRequiredInputField(optField2)).to.equal(false);

Expand All @@ -717,7 +717,7 @@ describe('Type predicates', () => {

const optField4 = buildInputField({
type: new GraphQLNonNull(GraphQLString),
defaultValue: 'default',
externalDefaultValue: 'default',
});
expect(isRequiredInputField(optField4)).to.equal(false);
});
Expand Down
Loading
Loading