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

Typesafe serialization and deserialization of scalars #5949

Closed
mastorm opened this issue May 6, 2021 · 8 comments
Closed

Typesafe serialization and deserialization of scalars #5949

mastorm opened this issue May 6, 2021 · 8 comments

Comments

@mastorm
Copy link

mastorm commented May 6, 2021

Is your feature request related to a problem? Please describe.

We use a lot of custom scalars in our schema. Following are three small examples:

"""The `Date` scalar represents an ISO-8601 compliant date type."""
scalar Date
"""The `DateTime` scalar represents an ISO-8601 compliant date time type."""
scalar DateTime
"""The WeekYear scalar represent an ISO Week compliant week year type. yyyy-Www"""
scalar WeekYear

Currently, the only option we have been able to find is to use the ScalarsMap to map them to strings respectively:

scalars:
     Date: string
     DateTime: string
     WeekYear: string

I am aware of the fact that you can map scalars to custom types aside from string too, but serializing to Date obviously doesnt work since graphql-codegen has no concept(that im aware of) that allows specifying how a specific scalar is deserialized.
That leaves me with what we have currently, which is telling graphql-codegen that the scalar should be mapped to string and then deserialize them at every occurence in my code which results in noisy and repetitive code for both mutations and queries.

Describe the solution you'd like
I would like to have some way to specify scalar types on the client safely. Here is my general idea:

  1. Add a new config value scalarMappers
  2. The scalarMappers value points to a folder containing all the scalarMappers
  3. ScalarMappers need to adhere to this interface:
interface ScalarMapper<T> {
    resultingTypeName(): string
    deserializeValue(value: any): T
    serializeValue(value: T): string
}
  1. Users could then declare typesafe parsers for scalar values:
import { parseISO } from "date-fns";

class DateTimeMapper implements ScalarMapper<Date> {
  resultingTypeName(): string {
    return 'Date';
  }
  deserializeValue(value: any): Date {
    if(typeof value !== 'string') {
      throw new Error('Could not deserialize value');
    }

    return parseISO(value);
  }
  serializeValue(value: Date): string {
    return value.toISOString();
  }
}

Describe alternatives you've considered
The current workaround we use is outlined in the first bullet point. Another option i considered was writing a custom plugin, but from the docs it seemed like that would require me to fork the typescript plugin since i want to modify the output of that. If theres a way to modify the output of the typescript plugin, let me know please. I sadly wasnt able to find one.

@mastorm
Copy link
Author

mastorm commented May 6, 2021

Another sidenote: if we agree that this is in-scope for the library and if we agree on a design for it, i would be willing to contribute this too.

@balazssoltesz
Copy link

Set in the config the following:

scalars:
     Date: MyDate
     DateTime: MyDateTime
     WeekYear: MyWeekYear

Then you can add a global.d.ts in your project, where u can define your custom types. This types will be accessible in your project without any import.
Check typescript documentation: https://www.typescriptlang.org/docs/handbook/declaration-files/templates/global-d-ts.html

@Urigo
Copy link
Collaborator

Urigo commented May 12, 2021

@mastorm this is in interesting area we have been thinking about.
Also the fact that we maintain graphql-scalars might help create an overall better experience using the two libraries together.
I think we can turn this issue into an RFC that would be great if you'll be the champion of!

Some possibly related issue from GraphQL Scalars:
Urigo/graphql-scalars#419
Urigo/graphql-scalars#651
Urigo/graphql-scalars#757

@dotansimha
Copy link
Owner

As @balazssoltesz suggested, you can use scalars config flag to set the types of your scalars.
If you wish to have that managed differently for input/output types, you can track this: #2684

@hachibeeDI
Copy link

hachibeeDI commented Jun 29, 2021

If I understand correctly, the thing @mastorm pointing is serialization/deserialization and type safety.
We can declare scalars map like DateTime: MyDateTime and it looks working fine in compile time but runtime because there is no serialization process from string to MyDateTime.

To work around, I'm using Branded Type:

// externs.ts

type Brand<T, B extends string> = T &
  {
    readonly [k in B]: symbol;
  };

export type MyDatetime = Brand<string, '__MyDatetime__'>;

/*
const dt: MyDatetime = '2011-10-05T14:48:00.000Z';  // error!: Type 'string' is not assignable to type 'MyDatetime'.
*/

and declare scalars:

      scalars:
        DateTime:  "externs#MyDateTime"

then write a serializer from MyDateTime to DateTime you would like to use (i.e. luxon).

Serializer issue still remains but type safety could be assured.

@ardatan
Copy link
Collaborator

ardatan commented Jun 29, 2021

Codegen doesn't aim this kind of deserialization. You need to implement it as part of client library.

@pganster
Copy link

pganster commented Nov 9, 2021

I'm facing the same issue. My only idea is to serialize & deserialize date fields wherever they are read from / written to in the code which leads to a lot of boilerplate code.

@ardatan you wrote

Codegen doesn't aim this kind of deserialization. You need to implement it as part of client library.

What do you mean with 'as part of the client library'? How could I write a central de-serializer that codegen uses for Date or Datetime scalars? As far as I know, I can only supply types with the scalars: config. Or is the only way to serialize / deserialize the scalar fields whenever they are accessed?

@ClayBenson94
Copy link

Facing this issue as well. Have had to write boilerplate scalar conversion code to handle this. It feels like a natural extension of the scalars map. Being able to define not just the type that a scalar should map to, but how to create that type from the scalar seems like it'd be a huge value add to the feature!

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

No branches or pull requests

8 participants