Skip to content
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.

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

TS: Use both input and output types in scalars parse/serialize #3397

Closed
wants to merge 1 commit into from

Conversation

honzajerabek
Copy link

Hi, currently the parseValue and serialize functions in GraphQLScalarTypeConfig are receiving unknown type in the value arg, but the type is known - it's the inverted type of the function to each other.

...
serialize: (val: TInternal) => TExternal
parseValue: (val: TExternal) => TInternal
...

With current unknown typing there are TS errors when trying to have it strongly typed.

This minimal custom scalar

export const Date = new GraphQLScalarType({
  name: 'Date',
  serialize: (value: Date): number => value.valueOf(),
  parseValue: (value: number): Date => new Date(value),
});

causes these errors:

serialize:

TS2322: Type '(value: Date) => number' is not assignable to type 'GraphQLScalarSerializer<number>'.   Types of parameters 'value' and 'outputValue' are incompatible.     Type 'unknown' is not assignable to type 'Date'

parseValue

TS2322: Type '(value: number) => Date' is not assignable to type 'GraphQLScalarValueParser<Date>'.   Types of parameters 'value' and 'inputValue' are incompatible.     Type 'unknown' is not assignable to type 'number'. 

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Dec 1, 2021

CLA Signed

The committers are authorized under a signed CLA.

@honzajerabek
Copy link
Author

Let me pls know if this makes sense at all, I'll then fix tests and everything

@IvanGoncharov
Copy link
Member

IvanGoncharov commented Dec 2, 2021

@honzajerabek Thanks for reporting.
It's unknown on purpose since you need to do validation inside both functions.
parseValue should validate query variable and serialize should validate return of resolvers.
graphql-js do all necessary validation for all types except for custom scalars so for custom scalars, you need to do all validation yourself that's why we need to have unknown there.

@honzajerabek
Copy link
Author

Okay thanks for explanation

@yaacovCR
Copy link
Contributor

yaacovCR commented Jan 8, 2022

See #3451 (comment) for some follow-on thoughts, perhaps the serializer could be made type-safe....

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants