Skip to content

Switch GraphQLScalarType methods to callbacks #1412

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jul 16, 2018
Merged

Switch GraphQLScalarType methods to callbacks #1412

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 3 additions & 0 deletions src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,9 @@ export type {
GraphQLTypeResolver,
GraphQLUnionTypeConfig,
GraphQLDirectiveConfig,
GraphQLScalarSerializer,
GraphQLScalarValueParser,
GraphQLScalarLiteralParser,
} from './type';

// Parse and operate on GraphQL language source files.
Expand Down
48 changes: 19 additions & 29 deletions src/type/definition.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -535,17 +535,20 @@ function resolveThunk<+T>(thunk: Thunk<T>): T {
export class GraphQLScalarType {
name: string;
description: ?string;
serialize: GraphQLScalarSerializer<*>;
parseValue: GraphQLScalarValueParser<*>;
parseLiteral: GraphQLScalarLiteralParser<*>;
astNode: ?ScalarTypeDefinitionNode;
extensionASTNodes: ?$ReadOnlyArray<ScalarTypeExtensionNode>;

_scalarConfig: GraphQLScalarTypeConfig<*, *>;

constructor(config: GraphQLScalarTypeConfig<*, *>): void {
this.name = config.name;
this.description = config.description;
this.serialize = config.serialize;
this.parseValue = config.parseValue || (value => value);
this.parseLiteral = config.parseLiteral || valueFromASTUntyped;
this.astNode = config.astNode;
this.extensionASTNodes = config.extensionASTNodes;
this._scalarConfig = config;
invariant(typeof config.name === 'string', 'Must provide name.');
invariant(
typeof config.serialize === 'function',
Expand All @@ -563,26 +566,6 @@ export class GraphQLScalarType {
}
}

// Serializes an internal value to include in a response.
serialize(value: mixed): mixed {
const serializer = this._scalarConfig.serialize;
Copy link
Member Author

@IvanGoncharov IvanGoncharov Jul 16, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it seems like an improvement based purely on the fact that the flow types are better

@mjmahone This wrapper function depends on this so before proposed change you can't do:

const serialize = GraphQLString.serialize;
serialize(5);
// TypeError: Cannot read property '_scalarConfig' of undefined

Or more complicated scenario:

const newScalar = new GraphQLScalarType({
  name: GraphQLString.name,
  serialize: GraphQLString.serialize,
});
console.log(GraphQLString.serialize(5))
// 5
console.log(newScalar.serialize(5))
// TypeError: Cannot read property '_scalarConfig' of undefined

return serializer(value);
}

// Parses an externally provided value to use as an input.
parseValue(value: mixed): mixed {
const parser = this._scalarConfig.parseValue;
return parser ? parser(value) : value;
}

// Parses an externally provided literal value to use as an input.
parseLiteral(valueNode: ValueNode, variables: ?ObjMap<mixed>): mixed {
const parser = this._scalarConfig.parseLiteral;
return parser
? parser(valueNode, variables)
: valueFromASTUntyped(valueNode, variables);
}

toString(): string {
return this.name;
}
Expand All @@ -592,17 +575,24 @@ export class GraphQLScalarType {
defineToStringTag(GraphQLScalarType);
defineToJSON(GraphQLScalarType);

export type GraphQLScalarSerializer<TExternal> = (value: mixed) => ?TExternal;
export type GraphQLScalarValueParser<TInternal> = (value: mixed) => ?TInternal;
export type GraphQLScalarLiteralParser<TInternal> = (
valueNode: ValueNode,
variables: ?ObjMap<mixed>,
) => ?TInternal;

export type GraphQLScalarTypeConfig<TInternal, TExternal> = {|
name: string,
description?: ?string,
// Serializes an internal value to include in a response.
serialize: GraphQLScalarSerializer<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>,
astNode?: ?ScalarTypeDefinitionNode,
extensionASTNodes?: ?$ReadOnlyArray<ScalarTypeExtensionNode>,
serialize: (value: mixed) => ?TExternal,
parseValue?: (value: mixed) => ?TInternal,
parseLiteral?: (
valueNode: ValueNode,
variables: ?ObjMap<mixed>,
) => ?TInternal,
|};

/**
Expand Down
3 changes: 3 additions & 0 deletions src/type/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,9 @@ export type {
GraphQLScalarTypeConfig,
GraphQLTypeResolver,
GraphQLUnionTypeConfig,
GraphQLScalarSerializer,
GraphQLScalarValueParser,
GraphQLScalarLiteralParser,
} from './definition';

export { validateSchema, assertValidSchema } from './validate';
6 changes: 3 additions & 3 deletions src/utilities/extendSchema.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -439,9 +439,9 @@ export function extendSchema(
description: type.description,
astNode: type.astNode,
extensionASTNodes,
serialize: type._scalarConfig.serialize,
parseValue: type._scalarConfig.parseValue,
parseLiteral: type._scalarConfig.parseLiteral,
Copy link
Member Author

@IvanGoncharov IvanGoncharov Jul 3, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the moment to copy scalar config you should either use type._scalarConfig.serialize or type.serialize.bind(type).
This is a big problem for 3rd-party libraries doing schema transformations.

serialize: type.serialize,
parseValue: type.parseValue,
parseLiteral: type.parseLiteral,
});
}

Expand Down