Skip to content

Commit

Permalink
improve upgrade path for new custom scalar parseConstLiteral() meth…
Browse files Browse the repository at this point in the history
…od (#4209)

`parseLiteral()` has been marked as deprecated, prompting but not
forcing our users to convert to `parseConstLiteral()`.

With this additional change:

- in v17, if not supplied, the new `parseConstLiteral()` method will be
left as undefined, and during execution, we will fall back
to`parseLiteral()`.
- in v18, we will remove `parseLiteral()` and set up a default function
for `parseConstLiteral()` when not supplied.

Prior to this change, users of custom scalars with custom
`parseLiteral()` functions who did not provide a custom
`parseConstLiteral()` function would just get the default
`parseConstLiteral()` 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 `parseConstLiteral()` method.
  • Loading branch information
yaacovCR authored Sep 30, 2024
1 parent 4dfae39 commit 0e22cc4
Show file tree
Hide file tree
Showing 5 changed files with 21 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
5 changes: 5 additions & 0 deletions src/type/__tests__/scalars-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}

Expand Down Expand Up @@ -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));
}

Expand Down Expand Up @@ -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));
}

Expand Down Expand Up @@ -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));
}

Expand Down Expand Up @@ -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));
}

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 0e22cc4

Please sign in to comment.