n1ru4l / envelop

Envelop is a lightweight library allowing developers to easily develop, share, collaborate and extend their GraphQL execution layer. Envelop is the missing GraphQL plugin system.
https://envelop.dev
MIT License
771 stars 117 forks source link

Poor typings in `ExecutionArgs` in `@envelop/types` #2085

Open klippx opened 7 months ago

klippx commented 7 months ago

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

I enabled @typescript-eslint/no-unsafe-member-access (part of plugin:@typescript-eslint/recommended-type-checked) and I get into trouble now for Unsafe member access .definitions on an `any` value.

This is due to the typings in ExecutionArgs in @envelop/types v5.0.0 which look like this:

export interface ExecutionArgs {
    schema: any;
    document: any;
    rootValue?: any;
    contextValue?: any;
    variableValues?: any;
    operationName?: any;
    fieldResolver?: any;
    typeResolver?: any;
    subscribeFieldResolver?: any;
}

Example:

import { type Plugin } from 'graphql-yoga';
export const useErrorHandling = (): Plugin => {
  return {
    onExecute: (payload) => {
      const operationName = payload.args.operationName as unknown as string; // manual type casting...
      const operation = payload.args.document.definitions[0]
                                           // ^-- Unsafe member access .definitions on an `any` value

Is unsafely accessing definitions on document: any.

Describe the solution you'd like

Isn't there anything we can do to give the user more hints that we have some sort of clue what these properties are? It is set by the framework isn't it so we should know that it is at least e.g.

export interface ExecutionArgs {
    schema: Schema; // Whatever the correct type is
    document: Document; // Whatever this type is, and that it is likely including `operation` which is e.g `query` | `mutation`
    rootValue?: RootValue; // Whatever the correct type is
    contextValue?: Record<string, unknown>; // Should be possible to get from Context generic
    variableValues?: Record<string, unknown>;
    operationName?: string;
    fieldResolver?: SomeFn;
    typeResolver?: AnotherFn;
    subscribeFieldResolver?: ThirdFn;
}

Describe alternatives you've considered

EmrysMyrddin commented 7 months ago

Hi, yes the typing is not really great there... We have to check why we define this type in Envelop and doesn't rely on the type defined by graphql.

EmrysMyrddin commented 7 months ago

For your information, the typing is a custom one here because we wanted to stay agnostic on the actual engine or schema building libraries you are using.

We are searching for an happy path for you to customize this types and avoid the any types everywhere. We are exploring the possibility to use module declarations, that would let you override the types we use, or some kind of generics to let you define the actual types to use.

I will keep you update here once we will take action on this.

The workaround during this time is, as you pointed out, to type cast each time you use it... Sorry about that, we should come with a better solution soon.