nuxt-modules / apollo

Nuxt.js module to use Vue-Apollo. The Apollo integration for GraphQL.
https://apollo.nuxtjs.org
MIT License
929 stars 194 forks source link

`clientId` is not working in `useAsyncQuery` when passed as the third argument #562

Closed AnotiaWang closed 4 months ago

AnotiaWang commented 8 months ago

Environment

Describe the bug

When passing clientId as the third param to useAsyncQuery, clientId is not actually set before performing the request.

Expected behaviour

clientId should be properly set

Reproduction

Just pass clientId as the third param when calling useAsyncQuery.

const clientId = ''
const result = await useAsyncQuery<API.UploadImagesQuery>(
  UploadImagesDocument,
  { /** custom variables here */ },
  clientId,
)

Additional context

I had a look at the code. In prep function, only the first and second member of args is checked, maybe this is the problem.

https://github.com/nuxt-modules/apollo/blob/042c15f21b9bbe449c6763d19520d21d44e51c70/src/runtime/composables.ts#L44C98-L44C98

Logs

No response

TimvdEijnden commented 7 months ago

This is fixed in #571. Please test again with 5.0.0-alpha-9.

AnotiaWang commented 7 months ago

I tested with the new version, but noticed a new issue: When performing a query that requires login, the request failed because the library didn't send the request with Authorization header, therefore the request was rejected by my backend.

Diizzayy commented 7 months ago

@AnotiaWang Would you be able to provide details or an example of how you've configured the authorization? I'd be happy to have a closer look to guide you accordingly or see if there's an issue which needs to be resolved.

PS. You can find the auth documentation here.

AnotiaWang commented 7 months ago

@AnotiaWang Would you be able to provide details or an example of how you've configured the authorization? I'd be happy to have a closer look to guide you accordingly or see if there's an issue which needs to be resolved.

PS. You can find the auth documentation here.

@Diizzayy I use localStorage mode, and useQuery can return the correct data, while `useAsyncQuery cannot.

image

(Btw, I think it can be better if the usage of useAsyncQuery keeps consistent with useQuery. I use graphql-codegen, and I'm not sure how to disable cache using useAsyncQuery, since the cache option is in the first parameter, which is occupied by the generated document)

Arlanthir commented 5 months ago

I'm on 5.0.0-alpha.11 and I'm still experiencing this. Looking at the code in https://github.com/nuxt-modules/apollo/blob/v5/src/runtime/composables.ts:

Overloaded function signatures (letter-named for later explanations):

A: export function useAsyncQuery <T> (opts: TAsyncQuery<T>): AsyncData<T, Error>
B: export function useAsyncQuery <T> (query: TQuery<T>, clientId?: string): AsyncData<T, Error>
C: export function useAsyncQuery <T> (query: TQuery<T>, variables?: TVariables<T>, clientId?: string, context?: DefaultContext): AsyncData<T, Error>

prep:

const context = args?.[0]?.context || (typeof args?.[1] === 'string' && args?.[3]) || undefined
let clientId = args?.[0]?.clientId || (typeof args?.[1] === 'string' && args?.[2]) || undefined

It seems to me it has the following issues:

  1. It's trying to grab context from the 4th parameter when param 1 is a string, which matches function signature B instead of C.
  2. It's trying to grab clientId from the third parameter of function signature B (which doesn't exist).
  3. It's not trying to grab clientId from function signature C.

If I'm correct, it should be more like:

const context = args?.[0]?.context || (typeof args?.[1] === 'object' && args?.[3]) || undefined
let clientId = args?.[0]?.clientId || (typeof args?.[1] === 'string' && args?.[1]) || (typeof args?.[1] === 'object' && args?.[2]) || undefined

Or a refactor of the above such as:

const context = args?.[0]?.context || (typeof args?.[1] === 'object' && args?.[3]) || undefined
let clientId
switch (typeof args?.[1]) {
  case 'string':
    clientId = args?.[1]
    break
  case 'object':
    clientId = args?.[2]
    break
  case 'undefined':
  default:
    clientId = args?.[0]?.clientId
}

Apologies if I'm missing something here or I wasn't clear.

plexus77 commented 4 months ago

Can confirm this is a fix @Arlanthir thankyou.