GraphQLScalarType serializer/parser typings

[hazelmeow]

graphql-js/src/type/definition.ts

Line 660 in 4175b26

export type GraphQLScalarSerializer<TExternal> = (

The GraphQLScalarType constructor and GraphQLScalarTypeConfig are typed with an Internal and External type, but the serializer/parser functions are partially typed with unknown. Is this intended/can it be changed? My first impression was that ScalarSerializer should be typed as (outputValue: TInternal) => TExternal but it is currently (outputValue: unknown) => TExternal.

For example I tried to do something like this:

new GraphQLScalarType<Date, number>({
	name: 'Date',
	serialize(value: unknown): number {
		return value.getTime();
	}
});

But there is a ts error since you can't call getTime on undefined.

Is the right solution to try and check if value is a Date object? Or can I just assume that value will always be a Date object and do something like (value as Date).getTime()?

[yaacovCR]

See: #3397 (comment)

[yaacovCR]

graphql-js highlights the type-safety of graphql, in that you can rely on the format and types of all fields of the response.

In theory, at least in my understanding, we can use TypeScript type safety magic to make sure that all resolver functions are type safe, and then we can at least appropriately type the serializer function for scalars, if not the parser, but that might not happen in this reference implementation, as then we wouldn't be explicitly demonstrating that GraphQL itself is type safe, instead relying on TS magic, which would sort of defeat the point of this reference implementation.

There are type-safe wrappers for GraphQL-JS like nexus, TypeGraphQL, giraphql, probably others, that would allow you, I believe, to have more TS convenience, but I am not personally familiar.

I believe also that @mike-marcacci had some TypeScript improvement plans after the Flow => TS migration happened, perhaps mike could chime in if there is anything in the works. @IvanGoncharov might also be able to give some context.

[mike-marcacci]

Hi folks! Indeed #2188 was my pet and is still something I would love to get out. I can't promise any kind of timeline, as it's a fairly significant undertaking... but I also think it's needed. @IvanGoncharov did have some reservations about overcomplicating a "reference implementation" but IMO it's just correctly using typescript. My goal would be to not change any functionality but simply communicate the functionality via TypeScript. That is, we wouldn't eliminate runtime type checking of returned values and rely only on TS; instead we would use TS to communicate the expectations of the runtime type checker, so that most mistakes are caught at compile time.

[yaacovCR]

Closing this issue as I believe the discussion can be tracked at #2188. Feel free to reopen as necessary.