Investigate using TypeScript strict mode with Redwood

[dac09]

Things to investigate:

Resolvers
Following #5437 we make it hard to use Resolvers with TS strict mode, because for convenience of testing we set obj and args as optional (which is technically not correct).

When strict mode is ON, we can switch to using the complete definition of ResolverFn

Docs on:

• Prisma types in scenario tests
• DbAuth functions
• Src/lib/auth in db auth
• Resolver configuration changing with typegen
• Add docs on how to extend context

[orta]

Just bumped my app to canary and there still seems to be issues on cross calling resolvers which is kind of intrinsic to how GraphQL resolvers work, I'll explain via an example. This mutation returns a legit slug or empty string to imply you can't use it:

export const canUseHandle: MutationResolvers["canUseHandle"] = async ({ handle }) => {
  const invalid = leo.check(handle)
  if (invalid) return ""

  const handleSlug = slugify(handle)
  const exists = await db.user.findUnique({ where: { username: handleSlug } })
  if (exists) return ""

  return handleSlug
}

Which looks like this to the type system:

const canUseHandle: (args: RequireFields<MutationcanUseHandleArgs, "handle">, obj: {
    root: {};
    context: RedwoodGraphQLContext;
    info: GraphQLResolveInfo;
}) => Partial<...> | Promise<...>

The tricky bit is here Partial<...> - which means you cannot write

export const createNewUserOnAccount: MutationResolvers["createNewUserOnAccount"] = async (args) => {
  const username = await canUseHandle({ handle: args.input.username }, {} as any)
  if (!username) {
    return null
  }
  // ....

Because the type of canUseHandle is either a string or a Promise. E.g. not a function to a string or the promise but the T or Promise<T> directly. I would have expected that makeResolverTypeCallable would make those callable?

Looking at the codegen code - I think I would have expected to see a lot of ResolverFnModelName in the .d.ts files - but I think the config change isn't doing the work #5437

[dac09]

I'm also super stuck with this problem... I don't understand why TypeScript in strict mode doesn't let me await a Promise<Partial<X>>?

If you modify the ResolverFn to not return a partial, TS doesn't complain :S

[orta]

There's a very very slim possibility of a TS bug here, this simplified model of the issue doesn't have issues when doing await () => Promise<Partial<X>>.

Here's a very close copy which does not repro

Here's a full failing repro - changing export type ResolverTypeWrapper<T> = T 'fixes it'

---

Yeah, after digging in, I'm not convinced this is a bug - I think this behavior from TS is correct. Just because I declared that canUseHandle is an async function - that doesn't absolve it from being a string on it's parent object in the type system according to the codegen types. We'd either need a Promise-y only version of MutationResolvers["canUseHandle"] to use after the const - e.g.

const canUseHandle: MutationResolversFn["canUseHandle"] = async ({ handle }) => {
  return handle
}

[dac09]

We'd either need a Promise-y only version of MutationResolvers["canUseHandle"] to use after the const.

Think I follow what you mean... but what I still dont understand is why removing Partial makes it stop complaining? If you leave all the other types exactly the same, but just remove the Partial, there's no errors

[Tobbe]

I think there might be an issue with the type generation here.
If I'm reading the full failing repro correctly the type pretty much boils down to

type CanUseHandle = ({ handle }: { handle: string }) => Promise<Partial<Promise<string> | string>>

But I think it should be

type CanUseHandle = ({ handle }: { handle: string }) => Promise<Partial<string>> | Partial<string>

Having Partial<Promise<string>> makes no sense. It's like "Here's a promise, but it might not have a then method..."

[dac09]

Think you misread. It's not a Partial of a promise. It's a promise of a partial.

Oh I think I understand. Some other wrapper there not in the ResolverFn itself - gotcha, great find.

[dac09]

So looks like we have a fix based on @Tobbe's observation.

Since our types weren't complex enough, we made it a little more complex:

export type ResolverFn<TResult, TParent, TContext, TArgs> = (
  args: TArgs,
  obj?: { root: TParent; context: TContext; info: GraphQLResolveInfo }
) => TResult extends PromiseLike<TResult>
  ? Promise<Partial<Awaited<TResult>>>
  : Promise<Partial<TResult>> | Partial<TResult>

@orta thoughts?

[Tobbe]

@dac09 With the code you posted above you've added Partial<TResult>to the return value (at the very end, after |). Previously it was always a Promise. (Promise<Partial<TResult>>)

Looking at the actual RW source code it's really Promise<Partial<TResult>> | Partial<TResult>. So that's fine with the code above.

---

So, TResult is basically going to be Promise<T> | T
If it's just T it's fine to do Promise<Partial<T>> | Partial<T>
But in case it's Promise<T> we'll get Promise<Partial<Promise<T>>> | Partial<Promise<T>> which we don't want.

If we branch on it being a Promise we need pick T out of it, and then wrap that in a partial before sticking it back into a promise again. That'd look like this
Promise<Promise<Partial<T>>> | Promise<Partial<T>>

I think the end result would be this

export type ResolverFn<TResult, TParent, TContext, TArgs> = (
  args: TArgs,
  obj?: { root: TParent; context: TContext; info: GraphQLResolveInfo }
) => TResult extends PromiseLike<unknown>
  ? Promise<Promise<Partial<Awaited<TResult>>>> | Promise<Partial<Awaited<TResult>>>
  : Promise<Partial<TResult>> | Partial<TResult>

[orta]

Tricky, running either of these doesn't improve too much, the big two off the bar are:

• obj?: cannot be optional or you cannot use { root } in a resolver. If tests are putting this object as empty, they should be using an {} as any because the real prod code will always have it.

• null isn't handled in a way that matches how prisma works (and I assume how most people will write resolvers)

export const game: QueryResolvers["game"] = ({ id }) =>
  const query = id.length > 10 ? { id } : { slug: id }
  return db.game.findUnique({ where: query })
}

              Type 'Game | null' is not assignable to type 'Partial<Promise<Game>>'.
                Type 'null' is not assignable to type 'Partial<Promise<Game>>'.ts(2322)

Now fails because the only acceptable responses are Game | undefined - but prisma returns Game | null.

I think I'm increasingly feeling like the answer doesn't live inside configuring graphql-codegen with a more and more complex type but in creating per-service file type definition which are explicitly generating the exact types for the exports of that file, and maybe even allowing for the return types to explicitly be Prisma objects so that you don't have the Date vs string issue.

[dac09]

The problem you're seeing here, I believe is something with how typescript in strict mode interprets Promise<Game | null vs Promise<Game> | Promise<null> - which I think should be equivalent. If it isn't.... whats the difference?!

I'm waiting to hear back from the Prisma team - this seems to be the last unresolved issue on the API side - it happens exclusively on the findUnique ones because findUniques return Promise<TData | null> (see prisma reproduction here: https://github.com/dac09/prisma-strict-ts-repro)

creating per-service file type definition which are explicitly generating the exact types for the exports

The reason I prefer gql-codegen still, is because if, for example, you use "select" in your Prisma queries, without using the codegen types, we wouldn't know what fields are required. e.g.

// This will error out with gql-codegen, telling us we need to return atleast whatever the sdl defines bazingas as
const bazingas: QueryResolvers['bazingas'] = () => {
  return db.bazinga.findMany({select: {id: true}}
}

On similar note, I'm also not sure about whether Partial<TResult> is correct - will need to look into this more.
My suspicions were right, TResult refers to the sdl type, so we don't want to do a Partial of TResult

[dac09]

Hey @orta - I'm wondering if I could get your help on the last remaining issue?

To summarise the problem:

• Prisma's return types for findUnique is Promise<Model | null>
• vs graphql-codegen expected type is Promise<Model> | Promise<null>

This means that you can't just return from a db.model.findUnique call in a service/resolver, but instead you need to do

return db.model.findUnique(...) as Promise<Model> | Promise<null>

Here's a playground repro of the problem. I understand that the types aren't exactly the same, but in reality this really shouldn't be an issue because at runtime there's no difference between the two types.

For now I'm just going to document it, but if you had any tips or suggestions I'm all ears - I've bothered a lot of people about this, but can't seem to find a clear solution.

[ebramanti]

@dac09 Not sure if this helps for the findUnique but maybe there's a way to use mappers to override the type in the codegen? I saw this comment, pretty old so not sure if it exactly solves the issue: dotansimha/graphql-code-generator#1793 (comment)

[orta]

I asked in the discord, where we got some pretty good explanations about why structurally you can only really compare in one direction:

Which I think makes sense generally, what you accept for one of those - personally, I think switching graphql-codegen to use Promise<Model | null> makes sense because it is the more flexible of the two ways to describe that pattern of something or nothing