Skip to content

Commit

Permalink
Add valueToLiteral() and literalToValue()
Browse files Browse the repository at this point in the history
* Adds `valueToLiteral()` which takes an external value and translates it to a literal, allowing for custom scalars to define this behavior.
* Adds `literalToValue()` which does the same in reverse, also allowing custom scalars to define it.
* Deprecates `valueFromASTUntyped()` in favor of `literalToValue()`, replacing all use of it in the codebase and adding a printed warning (via a new `deprecationWarning` method).

This also adds important changes to Input Coercion, especially for custom scalars:

* The value provided to `parseLiteral` is now `ConstValueNode` and the second `variables` argument has been removed. For all built-in scalars this has no effect, but any custom scalars which use complex literals no longer need to do variable reconciliation manually (in fact most do not -- this has been an easy subtle bug to miss).

  This behavior is possible with the addition of `replaceASTVariables`
* The `parseLiteral` function is no longer filled with a default implementation. Callsites need to check for it before calling it. This untangles what would otherwise be a circular dependency. Both callsites are updated.
  • Loading branch information
leebyron committed May 6, 2021
1 parent 4954529 commit 3c0ba0b
Show file tree
Hide file tree
Showing 26 changed files with 953 additions and 69 deletions.
7 changes: 7 additions & 0 deletions src/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -408,13 +408,20 @@ export {
// Create a JavaScript value from a GraphQL language AST with a Type.
valueFromAST,
// Create a JavaScript value from a GraphQL language AST without a Type.
// DEPRECATED: use literalToValue
valueFromASTUntyped,
// Create a GraphQL language AST from a JavaScript value.
astFromValue,
// A helper to use within recursive-descent visitors which need to be aware of
// the GraphQL type system.
TypeInfo,
visitWithTypeInfo,
// Converts a value to a const value by replacing variables.
replaceASTVariables,
// Create a GraphQL Literal AST from a JavaScript input value.
valueToLiteral,
// Create a JavaScript input value from a GraphQL Literal AST.
literalToValue,
// Coerces a JavaScript value to a GraphQL type, or produces errors.
coerceInputValue,
// Concatenates multiple AST together.
Expand Down
7 changes: 7 additions & 0 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -397,13 +397,20 @@ export {
// Create a JavaScript value from a GraphQL language AST with a Type.
valueFromAST,
// Create a JavaScript value from a GraphQL language AST without a Type.
// DEPRECATED: use literalToValue
valueFromASTUntyped,
// Create a GraphQL language AST from a JavaScript value.
astFromValue,
// A helper to use within recursive-descent visitors which need to be aware of
// the GraphQL type system.
TypeInfo,
visitWithTypeInfo,
// Converts a value to a const value by replacing variables.
replaceASTVariables,
// Create a GraphQL Literal AST from a JavaScript input value.
valueToLiteral,
// Create a JavaScript input value from a GraphQL Literal AST.
literalToValue,
// Coerces a JavaScript value to a GraphQL type, or produces errors.
coerceInputValue,
// Concatenates multiple AST together.
Expand Down
15 changes: 15 additions & 0 deletions src/jsutils/deprecationWarning.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/* eslint-disable no-console */
const canWarn = console && console.warn;
const hasIssuedWarning = {};

export function deprecationWarning(
deprecatedFunction: string,
resolution: string,
): void {
if (canWarn && !hasIssuedWarning[deprecatedFunction]) {
hasIssuedWarning[deprecatedFunction] = true;
console.warn(
`DEPRECATION WARNING: The function "${deprecatedFunction}" is deprecated and may be removed in a future version. ${resolution}`,
);
}
}
23 changes: 0 additions & 23 deletions src/type/__tests__/definition-test.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,8 @@
import { expect } from 'chai';
import { describe, it } from 'mocha';

import { inspect } from '../../jsutils/inspect';
import { identityFunc } from '../../jsutils/identityFunc';

import { parseValue } from '../../language/parser';

import type { GraphQLType, GraphQLNullableType } from '../definition';
import {
GraphQLList,
Expand Down Expand Up @@ -72,26 +69,6 @@ describe('Type System: Scalars', () => {

expect(scalar.serialize).to.equal(identityFunc);
expect(scalar.parseValue).to.equal(identityFunc);
expect(scalar.parseLiteral).to.be.a('function');
});

it('use parseValue for parsing literals if parseLiteral omitted', () => {
const scalar = new GraphQLScalarType({
name: 'Foo',
parseValue(value) {
return 'parseValue: ' + inspect(value);
},
});

expect(scalar.parseLiteral(parseValue('null'))).to.equal(
'parseValue: null',
);
expect(scalar.parseLiteral(parseValue('{ foo: "bar" }'))).to.equal(
'parseValue: { foo: "bar" }',
);
expect(
scalar.parseLiteral(parseValue('{ foo: { bar: $var } }'), { var: 'baz' }),
).to.equal('parseValue: { foo: { bar: "baz" } }');
});

it('rejects a Scalar type without name', () => {
Expand Down
32 changes: 11 additions & 21 deletions src/type/__tests__/scalars-test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { expect } from 'chai';
import { describe, it } from 'mocha';

import { parseValue as parseValueToAST } from '../../language/parser';
import { parseConstValue } from '../../language/parser';

import {
GraphQLID,
Expand Down Expand Up @@ -66,7 +66,8 @@ describe('Type System: Specified scalar types', () => {

it('parseLiteral', () => {
function parseLiteral(str: string) {
return GraphQLInt.parseLiteral(parseValueToAST(str), undefined);
// $FlowExpectedError[not-a-function]
return GraphQLInt.parseLiteral(parseConstValue(str));
}

expect(parseLiteral('1')).to.equal(1);
Expand Down Expand Up @@ -104,9 +105,6 @@ describe('Type System: Specified scalar types', () => {
expect(() => parseLiteral('ENUM_VALUE')).to.throw(
'Int cannot represent non-integer value: ENUM_VALUE',
);
expect(() => parseLiteral('$var')).to.throw(
'Int cannot represent non-integer value: $var',
);
});

it('serialize', () => {
Expand Down Expand Up @@ -231,7 +229,8 @@ describe('Type System: Specified scalar types', () => {

it('parseLiteral', () => {
function parseLiteral(str: string) {
return GraphQLFloat.parseLiteral(parseValueToAST(str), undefined);
// $FlowExpectedError[not-a-function]
return GraphQLFloat.parseLiteral(parseConstValue(str));
}

expect(parseLiteral('1')).to.equal(1);
Expand Down Expand Up @@ -264,9 +263,6 @@ describe('Type System: Specified scalar types', () => {
expect(() => parseLiteral('ENUM_VALUE')).to.throw(
'Float cannot represent non numeric value: ENUM_VALUE',
);
expect(() => parseLiteral('$var')).to.throw(
'Float cannot represent non numeric value: $var',
);
});

it('serialize', () => {
Expand Down Expand Up @@ -344,7 +340,8 @@ describe('Type System: Specified scalar types', () => {

it('parseLiteral', () => {
function parseLiteral(str: string) {
return GraphQLString.parseLiteral(parseValueToAST(str), undefined);
// $FlowExpectedError[not-a-function]
return GraphQLString.parseLiteral(parseConstValue(str));
}

expect(parseLiteral('"foo"')).to.equal('foo');
Expand All @@ -371,9 +368,6 @@ describe('Type System: Specified scalar types', () => {
expect(() => parseLiteral('ENUM_VALUE')).to.throw(
'String cannot represent a non string value: ENUM_VALUE',
);
expect(() => parseLiteral('$var')).to.throw(
'String cannot represent a non string value: $var',
);
});

it('serialize', () => {
Expand Down Expand Up @@ -456,7 +450,8 @@ describe('Type System: Specified scalar types', () => {

it('parseLiteral', () => {
function parseLiteral(str: string) {
return GraphQLBoolean.parseLiteral(parseValueToAST(str), undefined);
// $FlowExpectedError[not-a-function]
return GraphQLBoolean.parseLiteral(parseConstValue(str));
}

expect(parseLiteral('true')).to.equal(true);
Expand Down Expand Up @@ -489,9 +484,6 @@ describe('Type System: Specified scalar types', () => {
expect(() => parseLiteral('ENUM_VALUE')).to.throw(
'Boolean cannot represent a non boolean value: ENUM_VALUE',
);
expect(() => parseLiteral('$var')).to.throw(
'Boolean cannot represent a non boolean value: $var',
);
});

it('serialize', () => {
Expand Down Expand Up @@ -571,7 +563,8 @@ describe('Type System: Specified scalar types', () => {

it('parseLiteral', () => {
function parseLiteral(str: string) {
return GraphQLID.parseLiteral(parseValueToAST(str), undefined);
// $FlowExpectedError[not-a-function]
return GraphQLID.parseLiteral(parseConstValue(str));
}

expect(parseLiteral('""')).to.equal('');
Expand Down Expand Up @@ -604,9 +597,6 @@ describe('Type System: Specified scalar types', () => {
expect(() => parseLiteral('ENUM_VALUE')).to.throw(
'ID cannot represent a non-string and non-integer value: ENUM_VALUE',
);
expect(() => parseLiteral('$var')).to.throw(
'ID cannot represent a non-string and non-integer value: $var',
);
});

it('serialize', () => {
Expand Down
30 changes: 21 additions & 9 deletions src/type/definition.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import {
OperationDefinitionNode,
FieldNode,
FragmentDefinitionNode,
ValueNode,
ConstValueNode,
ScalarTypeExtensionNode,
UnionTypeExtensionNode,
EnumTypeExtensionNode,
Expand Down Expand Up @@ -315,7 +315,9 @@ export class GraphQLScalarType {
specifiedByURL: Maybe<string>;
serialize: GraphQLScalarSerializer<unknown>;
parseValue: GraphQLScalarValueParser<unknown>;
parseLiteral: GraphQLScalarLiteralParser<unknown>;
parseLiteral: Maybe<GraphQLScalarLiteralParser<unknown>>;
valueToLiteral: Maybe<GraphQLScalarValueToLiteral>;
literalToValue: Maybe<GraphQLScalarLiteralToValue>;
extensions: Maybe<Readonly<GraphQLScalarTypeExtensions>>;
astNode: Maybe<ScalarTypeDefinitionNode>;
extensionASTNodes: ReadonlyArray<ScalarTypeExtensionNode>;
Expand All @@ -326,7 +328,9 @@ export class GraphQLScalarType {
specifiedByURL: Maybe<string>;
serialize: GraphQLScalarSerializer<unknown>;
parseValue: GraphQLScalarValueParser<unknown>;
parseLiteral: GraphQLScalarLiteralParser<unknown>;
parseLiteral: Maybe<GraphQLScalarLiteralParser<unknown>>;
valueToLiteral: Maybe<GraphQLScalarValueToLiteral>;
literalToValue: Maybe<GraphQLScalarLiteralToValue>;
extensions: Maybe<Readonly<GraphQLScalarTypeExtensions>>;
extensionASTNodes: ReadonlyArray<ScalarTypeExtensionNode>;
};
Expand All @@ -343,9 +347,14 @@ export type GraphQLScalarValueParser<TInternal> = (
value: unknown,
) => Maybe<TInternal>;
export type GraphQLScalarLiteralParser<TInternal> = (
valueNode: ValueNode,
variables: Maybe<ObjMap<unknown>>,
valueNode: ConstValueNode,
) => Maybe<TInternal>;
export type GraphQLScalarValueToLiteral = (
inputValue: unknown,
) => Maybe<ConstValueNode>;
export type GraphQLScalarLiteralToValue = (
valueNode: ConstValueNode,
) => unknown;

export interface GraphQLScalarTypeConfig<TInternal, TExternal> {
name: string;
Expand All @@ -357,6 +366,10 @@ export interface GraphQLScalarTypeConfig<TInternal, TExternal> {
parseValue?: GraphQLScalarValueParser<TInternal>;
// Parses an externally provided literal value to use as an input.
parseLiteral?: GraphQLScalarLiteralParser<TInternal>;
// Translates an external input value to a literal (AST).
valueToLiteral?: Maybe<GraphQLScalarValueToLiteral>;
// Translates a literal (AST) to external input value.
literalToValue?: Maybe<GraphQLScalarLiteralToValue>;
extensions?: Maybe<Readonly<GraphQLScalarTypeExtensions>>;
astNode?: Maybe<ScalarTypeDefinitionNode>;
extensionASTNodes?: Maybe<ReadonlyArray<ScalarTypeExtensionNode>>;
Expand Down Expand Up @@ -782,10 +795,9 @@ export class GraphQLEnumType {
getValue(name: string): Maybe<GraphQLEnumValue>;
serialize(value: unknown): Maybe<string>;
parseValue(value: unknown): Maybe<any>;
parseLiteral(
valueNode: ValueNode,
_variables: Maybe<ObjMap<unknown>>,
): Maybe<any>;
parseLiteral(valueNode: ConstValueNode): Maybe<any>;
valueToLiteral(value: unknown): Maybe<ConstValueNode>;
literalToValue(valueNode: ConstValueNode): unknown;

toConfig(): GraphQLEnumTypeConfig & {
extensions: Maybe<Readonly<GraphQLEnumTypeExtensions>>;
Expand Down
45 changes: 32 additions & 13 deletions src/type/definition.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,9 @@ import type {
OperationDefinitionNode,
FieldNode,
FragmentDefinitionNode,
ValueNode,
ConstValueNode,
} from '../language/ast';

import { valueFromASTUntyped } from '../utilities/valueFromASTUntyped';

import type { GraphQLSchema } from './schema';

// Predicates & Assertions
Expand Down Expand Up @@ -560,7 +558,9 @@ export class GraphQLScalarType {
specifiedByURL: ?string;
serialize: GraphQLScalarSerializer<mixed>;
parseValue: GraphQLScalarValueParser<mixed>;
parseLiteral: GraphQLScalarLiteralParser<mixed>;
parseLiteral: ?GraphQLScalarLiteralParser<mixed>;
valueToLiteral: ?GraphQLScalarValueToLiteral;
literalToValue: ?GraphQLScalarLiteralToValue;
extensions: ?ReadOnlyObjMap<mixed>;
astNode: ?ScalarTypeDefinitionNode;
extensionASTNodes: $ReadOnlyArray<ScalarTypeExtensionNode>;
Expand All @@ -572,9 +572,9 @@ export class GraphQLScalarType {
this.specifiedByURL = config.specifiedByURL;
this.serialize = config.serialize ?? identityFunc;
this.parseValue = parseValue;
this.parseLiteral =
config.parseLiteral ??
((node, variables) => parseValue(valueFromASTUntyped(node, variables)));
this.parseLiteral = config.parseLiteral;
this.valueToLiteral = config.valueToLiteral;
this.literalToValue = config.literalToValue;
this.extensions = config.extensions && toObjMap(config.extensions);
this.astNode = config.astNode;
this.extensionASTNodes = config.extensionASTNodes ?? [];
Expand Down Expand Up @@ -610,6 +610,8 @@ export class GraphQLScalarType {
serialize: this.serialize,
parseValue: this.parseValue,
parseLiteral: this.parseLiteral,
valueToLiteral: this.valueToLiteral,
literalToValue: this.literalToValue,
extensions: this.extensions,
astNode: this.astNode,
extensionASTNodes: this.extensionASTNodes,
Expand Down Expand Up @@ -639,10 +641,15 @@ export type GraphQLScalarValueParser<TInternal> = (
) => ?TInternal;
export type GraphQLScalarLiteralParser<TInternal> = (
valueNode: ValueNode,
variables: ?ObjMap<mixed>,
valueNode: ConstValueNode,
) => ?TInternal;
export type GraphQLScalarValueToLiteral = (
inputValue: mixed,
) => ?ConstValueNode;
export type GraphQLScalarLiteralToValue = (valueNode: ConstValueNode) => mixed;
export type GraphQLScalarTypeConfig<TInternal, TExternal> = {|
name: string,
description?: ?string,
Expand All @@ -652,7 +659,11 @@ export type GraphQLScalarTypeConfig<TInternal, TExternal> = {|
// Parses an externally provided value to use as an input.
parseValue?: GraphQLScalarValueParser<TInternal>,
// Parses an externally provided literal value to use as an input.
parseLiteral?: GraphQLScalarLiteralParser<TInternal>,
parseLiteral?: ?GraphQLScalarLiteralParser<TInternal>,
// Translates an external input value to a literal (AST).
valueToLiteral?: ?GraphQLScalarValueToLiteral,
// Translates a literal (AST) to external input value.
literalToValue?: ?GraphQLScalarLiteralToValue,
extensions?: ?ReadOnlyObjMapLike<mixed>,
astNode?: ?ScalarTypeDefinitionNode,
extensionASTNodes?: ?$ReadOnlyArray<ScalarTypeExtensionNode>,
Expand All @@ -662,7 +673,6 @@ type GraphQLScalarTypeNormalizedConfig = {|
...GraphQLScalarTypeConfig<mixed, mixed>,
serialize: GraphQLScalarSerializer<mixed>,
parseValue: GraphQLScalarValueParser<mixed>,
parseLiteral: GraphQLScalarLiteralParser<mixed>,
extensions: ?ReadOnlyObjMap<mixed>,
extensionASTNodes: $ReadOnlyArray<ScalarTypeExtensionNode>,
|};
Expand Down Expand Up @@ -1320,8 +1330,7 @@ export class GraphQLEnumType /* <T> */ {
return enumValue.value;
}

parseLiteral(valueNode: ValueNode, _variables: ?ObjMap<mixed>): ?any /* T */ {
// Note: variables will be resolved to a value before calling this function.
parseLiteral(valueNode: ConstValueNode): ?any /* T */ {
if (valueNode.kind !== Kind.ENUM) {
const valueStr = print(valueNode);
throw new GraphQLError(
Expand All @@ -1343,6 +1352,16 @@ export class GraphQLEnumType /* <T> */ {
return enumValue.value;
}

valueToLiteral(value: mixed): ?ConstValueNode {
if (typeof value === 'string') {
// https://spec.graphql.org/draft/#Name
if (/^[_a-zA-Z][_a-zA-Z0-9]*$/.test(value)) {
return { kind: Kind.ENUM, value };
}
return { kind: Kind.STRING, value };
}
}

toConfig(): GraphQLEnumTypeNormalizedConfig {
const values = keyValMap(
this.getValues(),
Expand Down
Loading

0 comments on commit 3c0ba0b

Please sign in to comment.