Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Poor typings in ExecutionArgs in @envelop/types #2085

Open
klippx opened this issue Nov 17, 2023 · 2 comments
Open

Poor typings in ExecutionArgs in @envelop/types #2085

klippx opened this issue Nov 17, 2023 · 2 comments
Assignees
Labels
kind/bug Something isn't working stage/1-reproduction A reproduction exists

Comments

@klippx
Copy link
Contributor

klippx commented Nov 17, 2023

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

  • Overriding the type def myself, but I dont think I can make typings file that overrides an internal type :-(
  • Laborious type casting in place everytime we access this
@EmrysMyrddin EmrysMyrddin added kind/bug Something isn't working stage/1-reproduction A reproduction exists labels Nov 17, 2023
@EmrysMyrddin
Copy link
Collaborator

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 EmrysMyrddin self-assigned this Nov 20, 2023
@EmrysMyrddin
Copy link
Collaborator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working stage/1-reproduction A reproduction exists
Projects
None yet
Development

No branches or pull requests

2 participants