Open captbaritone opened 9 months ago
Is there an escape hatch for this right now if you want to have custom serializer/deserializer function?
The only escape hatch I can think of would be monkey patching the GraphQLSchema instance created by Grats.
An observation: Probably however we solve this, it should work the same as resolveType
on abstract types, which is another similar missing feature in Grats today.
My intuition for this after using Grats for a couple days is that it should just be similar to how resolvers are defined for a plain object type.
/** @gqlScalar */
type DateTime = Date
/** @gqlScalar */
function serialize(date: DateTime){
return date.toISO8601()
}
/** @gqlScalar */
function parse(value: unknown): DateTime {
try {
return new Date(value);
} catch (e) {
throw new Error("Not a valid date string")
}
}
I really appreciate hearing what felt intuitive given your existing experience with Grats! That's very helpful. I love that this approach does not require any additional syntax, however there are a few things I don't love about this approach:
@gqlScalar
tag now means different things in different contexts. That would break from our existing rules where each tag maps directly to the GraphQL construct it will create.Update: Another thing that just occurred to me. In theory one could define a custom scalar who's JavaScript representation was itself a function. So, determining which of the /** @gqlScalar */
s refer to the scalar and which refer to the conversion functions might be tricky to do cleanly.
I'll try to enumerate other options as I think of them as well as their pros and cons. I don't (yet?) see a solution that feels really good.
What if defining a scalar was done by annotating an exported object literal that defined the parse/serialize methods?
/** @gqlScalar **/
export const Date = {
serialize(date: DateTime): string {
return date.toISO8601()
},
parse(value: string): DateTime {
return new Date(value);
}
}
The docblock tag does not actually go on the definition of the type that will be returned by fields or accepted as args.
Here’s another thought. Perhaps we keep the mental model that GQL tags map 1 to 1 with SDL constructs.
Defining a JavaScript executor, of course, requires more than this, as we are seeing here. Perhaps these additional capabilities should be defined via a more traditional api.
For example our generated artifact could expose a getSchema
function instead of exporting the schema directly. This function would required you to pass the serialize/parse functions for all custom scalars. So using the Grats schema might look like:
import {getSchema} from "./schema";
const schema = getSchema({
scalars: {
Date: {
serialize(date: DateTime): string {
return date.toString();
},
parse(input: unknown): DateTime {
return parseDateTime(input);
}
}
}
});
Grats could take care of generating types such that you would be required to provide these functions.
Concrete example of the code we could generate:
import { MyScalar as MyScalarRuntimeType } from "./path/to/scalar";
import { GraphQLSchema, GraphQLObjectType, GraphQLScalarType } from "graphql";
export type SchemaConfig = {
scalars: {
MyScalar: {
serialize(input: MyScalarRuntimeType): unknown;
parseValue(input: unknown): MyScalarRuntimeType;
};
};
};
export function getSchema(config: SchemaConfig) {
const MyScalarType: GraphQLScalarType = new GraphQLScalarType({
description: "A description",
name: "MyScalar",
serialize: config.scalars.MyScalar.serialize,
parseValue: config.scalars.MyScalar.parseValue
});
const QueryType: GraphQLObjectType = new GraphQLObjectType({
name: "Query",
fields() {
return {
mammal: {
name: "mammal",
type: MyScalarType
}
};
}
});
return new GraphQLSchema({
query: QueryType,
types: [MyScalarType, QueryType]
});
}
I like that a lot - Downside seems to be that we lose co-location?
It also seems like we're going to expose something like getSchema for other things in the future anyway right?
I'm proceeding with the getSchema
approach. One interesting issue is that the types exposed by graphql-js for serialize/parseValue are needlessly vague. What's worse, it seems to be intentional:
https://github.com/graphql/graphql-js/issues/3451#issuecomment-1008114612
Probably here we'll want to break with our design principle of preferring to use our dependencies types where possible and present more sensible types. This seems especially reasonable given the comment:
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, [...]
Which indicates they expect libraries at our abstraction to be more opinionated than they are.
I've documented the current work-around here: https://github.com/captbaritone/grats/commit/561a9142234ee976c7bfc0a91f5fdcd4e8875c03
Looking at the types in graphql-js
a bit more fully to see how our types should differ from theirs:
Here's basically what graphql-js has, and why:
type Scalar<TInternal, TExternal> = {
// Unknown becuase... resolvers might not return the right thing
serialize(outputValue: unknown): TExternal,
// Unknown because... called on user-passed variables values
parseValue(inputValue: unknown): TInternal,
parseLiteral(valueNode: ValueNode, variables?: Maybe<ObjMap<unknown>>): TInternal
}
I think Grats can do two things differently.
TInternal
for the input of .serialize
, since Grats can ensure the resolvers will return the expected type. (Assuming TypeScript type safety)TExternal
type param, since it only appears in one place in the whole program.
parseValue
after the input has been validated?).My proposal for Grat's type:
type GratsScalar<TInternal> = {
// Unknown becuase... resolvers might not return the right thing
serialize(outputValue: TInternal): any, // You can serialize as anything you want
// Unknown because... called on user-passed variables values.
parseValue(inputValue: unknown): TInternal,
parseLiteral(valueNode: ValueNode, variables?: Maybe<ObjMap<unknown>>): TInternal
}
We should look at
graphql-scalars
for compatibility. And Pothos' concept of Input and Output for scalars.We should also see how Strawberry and Juniper address this.
Specifically, we need a way to let users define these bits of a custom scalar: https://github.com/graphql/graphql-js/blob/688ee2f2a2f938e80bcf3808bc435d3d87a3ff3e/src/type/definition.ts#L548-L551