typed-graphql-builder / typed-graphql-builder

A fully type-safe graphql query builder, inspired by tql
https://typed-graphql-builder.spion.dev
MIT License
52 stars 10 forks source link

Inconsistent type-checking for input arguments #66

Closed michaelbridge closed 10 months ago

michaelbridge commented 10 months ago

In both cases, junk is an invalid argument, while id is valid. As you can see in the 2nd example, type-checking does not correctly identify junk as an invalid parameter. This issue also exists within more complicated JSON inputs, but this is the simplest reproduction.

Screen Shot 2023-12-15 at 4 12 05 PM

The generated type is as follows:

company<Args extends VariabledInput<{
    id: number,
}>, ...

I experimented with several solutions but have thus far been unsuccessful. The solution I thought would work did not -- this involves wrapping VariabledInput with Exact (per type-fest):

company<Args extends Exact<VariabledInput<{
    id: number,
}>, Args>, ...

Thoughts? Hints? Suggestions?

spion commented 10 months ago

Unfortunately not much I can do - Args extends VariabledInput<argsType> is necessary to be able to accept variables in any position of argsType, however, extends also implies "additional properties available".

An alternative design that I was playing with but never got a chance to make work

query<{myVar: string}>('QueryName', (q, vars) => [
  q.company({id: vars.myVar}, c => [c.display_name, c.investments(all)])
])

Runtime details: a proxy is passed where any get accessor always returns a variable (with the name of the accessor)

Pros

Cons

spion commented 10 months ago

Looks like I was wrong

continent<
    Args extends VariabledInput<{
      code: string
    }>,
    Sel extends Selection<Continent>
  >(
    args: Args & { [K in keyof Args]: K extends keyof { code: string } ? Args[K] : never },
    selectorFn: (s: Continent) => [...Sel]
  )

has the desired behavior. Haven't tried yet if it will work in all situations.

spion commented 10 months ago

PR: https://github.com/typed-graphql-builder/typed-graphql-builder/pull/67 - existing tests pass.