Allow Grats to validate context values

[captbaritone]

The third argument to a resolver is always the request context. This can be anything the user configures, so it's generally hard to typecheck effectively. However, Grats knows each function/method that is a GraphQL field (thanks to the @gqlField annotations) so could be charged with ensuring all fields are typed in a way that matches the developer's expectations.

This would require some way for Grats users to inform users what type they are passing to their executor. I'll need to brainstorm what options make sense here:

1. Grats config file allows you to define a module path + export name
2. Some well defined global type (GratsContextValue?) that can be defined by the user in their code and Grats will look for
3. Some other TypeScript trick that I'm not thinking of?
4. Edit /** @gqlContext */ on the context type declaration.

We should also examine how existing TS GraphQL servers address this. I'm thinking specifically of TypeGraphQL and Pothos.

[mandx]

Another option is to simply enforce that the context argument (if present) on all resolvers have to reference the exact same type. I doesn't matter what type it is (Request/string/{}/etc) but they all have to be the same.

So this would be valid:

function resolverOne(_: Foo, args: {}, context: MyContextType) {}
function resolverTwo(_: Bar, args: {}, context: MyContextType) {}

but this would make Grats emit an error:

function resolverOne(_: Foo, args: {}, context: MyContextType) {}
function resolverTwo(_: Bar, args: {}, context: string) {}

because MyContextType is not string.

Similarly, this would be fine:

function resolverOne(_: Foo, args: {}, context: string) {}
function resolverTwo(_: Bar, args: {}, context: string) {}

This as well:

function resolverOne(_: Foo, args: {}, context: MyContextType) {}
function resolverTwo(_: Bar, args: {}) {} // this doesn't use the `context` arg

And maybe, this could be allowed as well

function resolverOne(_: Foo, args: {}, context: MyContextType) {}
function resolverTwo(_: Bar, args: {}, context: unknown) {}

Typing it as unknown would be a safe-ish escape hatch, if one can't really refer to a concrete type there, forcing the resolver implementation to unsafely cast context to something else, or dynamically inspect the value with instanceof or type guards etc.

IMO this is an elegant way for Grats to enforce the type of context, doesn't require a config or docblock tags, etc.

It still on the developer to ensure the type of the context passed to the executor is the same the resolvers expect, I doubt Grats can help with that...

[captbaritone]

Oh I like this! We can probably start here and follow-up with an explicit option if we find there’s some value in making it more explicit. I may pick a slightly more opinionated restriction: the ctx argument must be typed using a type that resolves to the same definition. That saves us from having to actually perform type checking (which is not fully supported by the TS library)

…

On Wed, Nov 1, 2023 at 8:00 PM Armando Pérez Marqués < ***@***.***> wrote: Another option is to simply enforce that the context argument (if present) on all resolvers have to reference the exact same type. I doesn't matter what type it is (Request/string/{}/etc) but they all *have* to be the same. So this would be valid: function resolverOne(_: Foo, args: {}, context: MyContextType) {}function resolverTwo(_: Bar, args: {}, context: MyContextType) {} but this would make Grats emit an error: function resolverOne(_: Foo, args: {}, context: MyContextType) {}function resolverTwo(_: Bar, args: {}, context: string) {} because MyContextType is not string. Similarly, this would be fine: function resolverOne(_: Foo, args: {}, context: string) {}function resolverTwo(_: Bar, args: {}, context: string) {} This as well: function resolverOne(_: Foo, args: {}, context: MyContextType) {}function resolverTwo(_: Bar, args: {}) {} // this doesn't use the `context` arg And maybe, this could be allowed as well function resolverOne(_: Foo, args: {}, context: MyContextType) {}function resolverTwo(_: Bar, args: {}, context: unknown) {} Typing it as unknown would be a safe-ish escape hatch, if one can't really refer to a concrete type there, forcing the resolver implementation to unsafely cast context to something else, or dynamically inspect the value with instanceof or type guards etc. IMO this is an elegant way for Grats to enforce the type of context, doesn't require a config or docblock tags, etc. — Reply to this email directly, view it on GitHub <#21 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABHXL5D2LBPII4KYJDQWD3YCMEDDAVCNFSM6AAAAAA62FYMDOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTOOBZHE4DMMRQGU> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[captbaritone]

A related topic to context variable typing is the ability to support positional arguments. I've written an issue for that in #23. In short, if Grats can explicitly know which arg is typed as the context value (for example via a /** @gqlContext */ tag), it could allow you to define your arguments as positional arguments. See the #23 for more context.

[captbaritone]

I've tried out the @gqlContext approach (without positional arguments) here: #24

[mandx]

Kinda torn on positional arguments... I can see how it might be more ergonomic for others; OTOH, since GraphQL field arguments are keyword arguments, IMO it's nice that the args mirrors that. Also, keeping the resolvers' signature as close as possible to what graphql-js expects allows for easy migration to Grats, which is good for adoption.

Or maybe it's just that the args: {/* ... */} syntax is most familiar to me, and my brain is just resisting change 😅

[mandx]

I may pick a slightly more opinionated restriction: the ctx argument must be typed using a type that resolves to the same definition. That saves us from having to actually perform type checking (which is not fully supported by the TS library)

Btw I agree, it has to (nominally) resolve to the same type, so for example:

function resolverOne(_: Foo, args: {}, context: { foo: string; bar: number }) {}
function resolverTwo(_: Bar, args: {}, context: { foo: string; bar: number }) {}

It's my understanding that for TS, those two context args don't have the same type, but rather two independent types that happen to be 100% compatible between them. Proper type checking is needed to ensure that the types actually are compatible. Also, in practice, devs will name the type and then refer to that by name (possibly with the @gqlContext added as well) so IMO it's Ok to reject these cases and provide actions ("put the type under a name and use the name instead") as part of the error message.