From 4317476bf6e3189e364442ce84b5b8f2ce4d58b4 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Mon, 30 Sep 2024 12:44:52 +0300 Subject: [PATCH 1/2] provide better upgrade experience for custom scalars for the move to parseConstValue() `parseValue()` has been marked as deprecated, prompting but not forcing our users to convert to `parseConstValue()`. With this additional change: - in v17, if not supplied, the new `parseConstValue()` method will be left as undefined, and during execution, we will fall back to`parseValue()`. - in v18, we will remove `parseValue()` and set up a default function for `parseConstValue()` when not supplied. Prior to this change, users of custom scalars with custom `parseValue()` functions who did not provide a custom `parseConstValue()` function would just get the default `parseConstValue()` function during execution, which might not work as expected. This scheme will work except for users of custom scalars who want to embed experimental fragment variables in their custom scalars, which will only work with the new `parseConstValue()` method. --- src/type/__tests__/definition-test.ts | 9 +++++---- src/type/definition.ts | 8 +++----- src/utilities/coerceInputValue.ts | 12 +++++------- src/validation/rules/ValuesOfCorrectTypeRule.ts | 6 +++--- 4 files changed, 16 insertions(+), 19 deletions(-) diff --git a/src/type/__tests__/definition-test.ts b/src/type/__tests__/definition-test.ts index 1e82312660..617209206d 100644 --- a/src/type/__tests__/definition-test.ts +++ b/src/type/__tests__/definition-test.ts @@ -72,10 +72,11 @@ describe('Type System: Scalars', () => { expect(scalar.serialize).to.equal(identityFunc); expect(scalar.parseValue).to.equal(identityFunc); expect(scalar.parseLiteral).to.be.a('function'); - expect(scalar.parseConstLiteral).to.be.a('function'); + /* default will be provided in v18 when parseLiteral is removed */ + // expect(scalar.parseConstLiteral).to.be.a('function'); }); - it('use parseValue for parsing literals if parseConstLiteral omitted', () => { + it('use parseValue for parsing literals if parseLiteral omitted', () => { const scalar = new GraphQLScalarType({ name: 'Foo', parseValue(value) { @@ -83,11 +84,11 @@ describe('Type System: Scalars', () => { }, }); - expect(scalar.parseConstLiteral(parseConstValue('null'))).to.equal( + expect(scalar.parseLiteral(parseConstValue('null'), undefined)).to.equal( 'parseValue: null', ); expect( - scalar.parseConstLiteral(parseConstValue('{ foo: "bar" }')), + scalar.parseLiteral(parseConstValue('{ foo: "bar" }'), undefined), ).to.equal('parseValue: { foo: "bar" }'); }); diff --git a/src/type/definition.ts b/src/type/definition.ts index 15cda87d4b..f00e0d5694 100644 --- a/src/type/definition.ts +++ b/src/type/definition.ts @@ -588,7 +588,7 @@ export class GraphQLScalarType { parseValue: GraphQLScalarValueParser; /** @deprecated use `replaceVariables()` and `parseConstLiteral()` instead, `parseLiteral()` will be deprecated in v18 */ parseLiteral: GraphQLScalarLiteralParser; - parseConstLiteral: GraphQLScalarConstLiteralParser; + parseConstLiteral: GraphQLScalarConstLiteralParser | undefined; valueToLiteral: GraphQLScalarValueToLiteral | undefined; extensions: Readonly; astNode: Maybe; @@ -608,9 +608,7 @@ export class GraphQLScalarType { this.parseLiteral = config.parseLiteral ?? ((node, variables) => parseValue(valueFromASTUntyped(node, variables))); - this.parseConstLiteral = - config.parseConstLiteral ?? - ((node) => parseValue(valueFromASTUntyped(node))); + this.parseConstLiteral = config.parseConstLiteral; this.valueToLiteral = config.valueToLiteral; this.extensions = toObjMap(config.extensions); this.astNode = config.astNode; @@ -708,7 +706,7 @@ interface GraphQLScalarTypeNormalizedConfig serialize: GraphQLScalarSerializer; parseValue: GraphQLScalarValueParser; parseLiteral: GraphQLScalarLiteralParser; - parseConstLiteral: GraphQLScalarConstLiteralParser; + parseConstLiteral: GraphQLScalarConstLiteralParser | undefined; extensions: Readonly; extensionASTNodes: ReadonlyArray; } diff --git a/src/utilities/coerceInputValue.ts b/src/utilities/coerceInputValue.ts index 757121b3ae..cca2010bac 100644 --- a/src/utilities/coerceInputValue.ts +++ b/src/utilities/coerceInputValue.ts @@ -371,14 +371,12 @@ export function coerceInputLiteral( } const leafType = assertLeafType(type); - const constValueNode = replaceVariables( - valueNode, - variableValues, - fragmentVariableValues, - ); - try { - return leafType.parseConstLiteral(constValueNode); + return leafType.parseConstLiteral + ? leafType.parseConstLiteral( + replaceVariables(valueNode, variableValues, fragmentVariableValues), + ) + : leafType.parseLiteral(valueNode, variableValues?.coerced); } catch (_error) { // Invalid: ignore error and intentionally return no value. } diff --git a/src/validation/rules/ValuesOfCorrectTypeRule.ts b/src/validation/rules/ValuesOfCorrectTypeRule.ts index d2b566823d..73357e1317 100644 --- a/src/validation/rules/ValuesOfCorrectTypeRule.ts +++ b/src/validation/rules/ValuesOfCorrectTypeRule.ts @@ -153,12 +153,12 @@ function isValidValueNode(context: ValidationContext, node: ValueNode): void { return; } - const constValueNode = replaceVariables(node); - // Scalars and Enums determine if a literal value is valid via parseConstLiteral(), // which may throw or return undefined to indicate an invalid value. try { - const parseResult = type.parseConstLiteral(constValueNode); + const parseResult = type.parseConstLiteral + ? type.parseConstLiteral(replaceVariables(node)) + : type.parseLiteral(node, undefined); if (parseResult === undefined) { const typeStr = inspect(locationType); context.reportError( From b14f5fd3b270b4cf78972e89af5ebc22c5550af4 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Mon, 30 Sep 2024 12:51:37 +0300 Subject: [PATCH 2/2] fix types --- src/type/__tests__/scalars-test.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/type/__tests__/scalars-test.ts b/src/type/__tests__/scalars-test.ts index eff300918d..fbb2cd9087 100644 --- a/src/type/__tests__/scalars-test.ts +++ b/src/type/__tests__/scalars-test.ts @@ -66,6 +66,7 @@ describe('Type System: Specified scalar types', () => { it('parseConstLiteral', () => { function parseConstLiteral(str: string) { + /* @ts-expect-error to be removed in v18 when all custom scalars will have default method */ return GraphQLInt.parseConstLiteral(parseConstValue(str)); } @@ -228,6 +229,7 @@ describe('Type System: Specified scalar types', () => { it('parseConstLiteral', () => { function parseConstLiteral(str: string) { + /* @ts-expect-error to be removed in v18 when all custom scalars will have default method */ return GraphQLFloat.parseConstLiteral(parseConstValue(str)); } @@ -338,6 +340,7 @@ describe('Type System: Specified scalar types', () => { it('parseConstLiteral', () => { function parseConstLiteral(str: string) { + /* @ts-expect-error to be removed in v18 when all custom scalars will have default method */ return GraphQLString.parseConstLiteral(parseConstValue(str)); } @@ -447,6 +450,7 @@ describe('Type System: Specified scalar types', () => { it('parseConstLiteral', () => { function parseConstLiteral(str: string) { + /* @ts-expect-error to be removed in v18 when all custom scalars will have default method */ return GraphQLBoolean.parseConstLiteral(parseConstValue(str)); } @@ -559,6 +563,7 @@ describe('Type System: Specified scalar types', () => { it('parseConstLiteral', () => { function parseConstLiteral(str: string) { + /* @ts-expect-error to be removed in v18 when all custom scalars will have default method */ return GraphQLID.parseConstLiteral(parseConstValue(str)); }