Skip to content

Commit

Permalink
provide better upgrade experience for custom scalars for the move to …
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
yaacovCR committed Sep 30, 2024
1 parent 4dfae39 commit 4317476
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 19 deletions.
9 changes: 5 additions & 4 deletions src/type/__tests__/definition-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,22 +72,23 @@ 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) {
return 'parseValue: ' + inspect(value);
},
});

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" }');
});

Expand Down
8 changes: 3 additions & 5 deletions src/type/definition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -588,7 +588,7 @@ export class GraphQLScalarType<TInternal = unknown, TExternal = TInternal> {
parseValue: GraphQLScalarValueParser<TInternal>;
/** @deprecated use `replaceVariables()` and `parseConstLiteral()` instead, `parseLiteral()` will be deprecated in v18 */
parseLiteral: GraphQLScalarLiteralParser<TInternal>;
parseConstLiteral: GraphQLScalarConstLiteralParser<TInternal>;
parseConstLiteral: GraphQLScalarConstLiteralParser<TInternal> | undefined;
valueToLiteral: GraphQLScalarValueToLiteral | undefined;
extensions: Readonly<GraphQLScalarTypeExtensions>;
astNode: Maybe<ScalarTypeDefinitionNode>;
Expand All @@ -608,9 +608,7 @@ export class GraphQLScalarType<TInternal = unknown, TExternal = TInternal> {
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;
Expand Down Expand Up @@ -708,7 +706,7 @@ interface GraphQLScalarTypeNormalizedConfig<TInternal, TExternal>
serialize: GraphQLScalarSerializer<TExternal>;
parseValue: GraphQLScalarValueParser<TInternal>;
parseLiteral: GraphQLScalarLiteralParser<TInternal>;
parseConstLiteral: GraphQLScalarConstLiteralParser<TInternal>;
parseConstLiteral: GraphQLScalarConstLiteralParser<TInternal> | undefined;
extensions: Readonly<GraphQLScalarTypeExtensions>;
extensionASTNodes: ReadonlyArray<ScalarTypeExtensionNode>;
}
Expand Down
12 changes: 5 additions & 7 deletions src/utilities/coerceInputValue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
}
Expand Down
6 changes: 3 additions & 3 deletions src/validation/rules/ValuesOfCorrectTypeRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down

0 comments on commit 4317476

Please sign in to comment.