(vue) useQuery loses typing of variables

[arkandias]

Describe the bug

I'm using GraphQL codegen for typing. Here is an MWE (I also made a repo to show the issue -- see the reproduction link):

const AllFilmsQuery = graphql(/* GraphQL */ `
  query AllFilmsWithVariablesQuery($first: Int!) {
    allFilms(first: $first) {
      edges {
        node {
          id
        }
      }
    }
  }
`)

// These variables should raise a type error:
useQuery({
  query: AllFilmsQuery,
  variables: { first: null } // wrong type
})
useQuery({
  query: AllFilmsQuery,
  variables: {  } // missing field
})
useQuery({
  query: AllFilmsQuery,
  variables: { foo: 'bar' } // extra field
})

The problem comes from this commit. If you revert

type MaybeRefObj<T> = T extends {} ? {
    [K in keyof T]: MaybeRef<T[K]>;
} : T;

to

type MaybeRefObj<T extends {}> = { [K in keyof T]: MaybeRef<T[K]> };

then the type of the variables is correctly inferred, and the above examples produce a type error.

Not sure if one can revert the aforementioned commit without breaking something else... (I can investigate if need be.)

Reproduction

https://github.com/arkandias/urql-vue-variables-typing-issue

Urql version

"dependencies": {
"@urql/vue": "^1.4.2",
"graphql": "^16.10.0",
"vue": "^3.5.13"
},
"devDependencies": {
"@graphql-codegen/cli": "^5.0.3",
"@graphql-codegen/client-preset": "^4.5.1",
"@graphql-typed-document-node/core": "^3.2.0",
"@vitejs/plugin-vue": "^5.2.1",
"@vue/tsconfig": "^0.7.0",
"typescript": "~5.6.3",
"vite": "^6.0.5",
"vue-tsc": "^2.2.0"
}

Validations

• I can confirm that this is a bug report, and not a feature request, RFC, question, or discussion, for which GitHub Discussions should be used
• Read the docs.
• Follow our Code of Conduct

[arkandias]

It seems to me that there are multiple issues with the typing (apologies if I'm misunderstanding). Notably, unwrapDeeply has type (input: T) => T, which it should not (just like reactive<T> returns an UnwrapNestedRefs<T>). The type check passes because of multiple type assertions which are sometimes inconsistent.

Before fixing this issue, it should be made clear exactly what kind of reactive variable we can pass to useQuery et al.
The doc is quite vague about it: "All inputs that are passed to useQuery may also be reactive state."

What is implemented now is the following:

• a type MaybeRef<T> = T | (() => T) | Ref<T>
• a method unwrap (maybeRef: MaybeRef<T>) => T
• a method unwrapDeeply (input: T) => T

(As a side note, the first two should be called respectively MaybeRefOrGetter and toValue since these are the names of their implementations in Vue 3.3+)

The current implementation of unwrapDeeply(input) does the following:

• first it unwraps input if it's a MaybeRef,
• if the result is an array, it applies recursively to all it's elements,
• if the result is an object, it applies recursively to all it's properties.

For example,

unwrapDeeply({
  foo: ref('toto'),
  bar: [0, () => 'a', ref(true)],
  baz: () => ({
    a: ref(1),
    b: null,
  }),
});

returns

{
  foo: 'toto',
  bar: [0, 'a', true],
  baz: {
    a: 1,
    b: null,
  },
}

Is this really what we need? I'm not sure about unwrapping arrays for example (reactive unwraps refs deeply, but not in arrays).
Actually I'm not sure that a deep unwrapping is necessary at all (if need be the user can deeply unwrap variables with reactive before passing it to useQuery -- deep getters should be computed values), so I'd argue that a simple shallow unwrapping would be enough (as a conveniency, because, again, the end user can always wrap variables in reactive and it would work out of the box).
But I am probably unaware of many use cases...

Also, is there a use case for having a reactive query field? Edit: I'd argue that

& GraphQLRequestParams<Data, MaybeRefObj<Variables>>

is enough, but again I am probably missing some use cases.

Anyway, there seems to be something to discuss here.

[arkandias]

A side question about this type (in vue/core):

type GraphQLRequestParams<
  Data = any,
  Variables extends AnyVariables = AnyVariables,
> =
  | ({
      query: string | Data;
    } & (Variables extends void
      ? {
          variables?: Variables;
        }
      : Variables extends {
            [P in keyof Variables]: Exclude<Variables[P], null | void>;
          }
        ? Variables extends {
            [P in keyof Variables]: never;
          }
          ? {
              variables?: Variables;
            }
          : {
              variables: Variables;
            }
        : {
            variables?: Variables;
          }))
  | {
      query: string | Data;
      variables: Variables;
    };

As I understand it (and after some tests), it makes the variables field optional if and only if at least one field of Variables is void | null. Is this really the expected behavior? Shouldn't it be if all fields of Variables are nullish?
(@kitten I see you wrote this code, could you point to some explanation ? Sorry if this is a silly question!)

Edit: Not sure if this is the expected behavior, but this type

export type GraphQLRequestParams<
  Data = any,
  Variables extends AnyVariables = AnyVariables,
> =
  | ({
      query: DocumentInput<Data, Variables>;
    } & (Variables extends void
      ? {
          variables?: Variables;
        }
      : Variables extends {
            [P in keyof Variables]: undefined extends Variables[P]
              ? unknown
              : null extends Variables[P]
                ? unknown
                : never;
          }
        ? {
            variables?: Variables;
          }
        : {
            variables: Variables;
          }))
  | {
      query: DocumentInput<Data, Variables>;
      variables: Variables;
    };

would make variables optional if and only if each property of variables can be assigned either undefined (in particular if it is optional) or null.

[DevilTea]

Bump in to the same problem.😵

[arkandias]

I updated my PR to solve this issue. I implemented a simple unwrapping of variables and query.
Some feedback would be very appreciated.