vuejs / apollo

🚀 Apollo/GraphQL integration for VueJS
http://apollo.vuejs.org
MIT License
6.03k stars 522 forks source link

Type MutateWithRequiredVariables does not satisfy the constraint MutateWithOptionalVariables #948

Open geekox86 opened 4 years ago

geekox86 commented 4 years ago

Describe the bug

ERROR in /Users/geekox86/Documents/GitHub/OeMis/node_modules/@vue/apollo-composable/dist/useMutation.d.ts(45,340):
45:340 Type 'MutateWithRequiredVariables<TResult, TVariables>' does not satisfy the constraint 'MutateWithOptionalVariables<TResult, TVariables>'.
  Types of parameters 'variables' and 'variables' are incompatible.
    Type 'TVariables | undefined' is not assignable to type 'TVariables'.
      Type 'undefined' is not assignable to type 'TVariables'.
    43 |  * Use a mutation with variables, but without a default.
    44 |  */
  > 45 | export declare function useMutation<TResult = any, TVariables extends OperationVariables = OperationVariables>(document: DocumentNode | ReactiveFunction<DocumentNode>, options?: UseMutationOptionsNoVariables<TResult, undefined> | ReactiveFunction<UseMutationOptionsNoVariables<TResult, undefined>>): UseMutationReturn<TResult, TVariables, MutateWithRequiredVariables<TResult, TVariables>>;
       |                                                                                                                                                                                                                                                                                                                                                    ^
    46 | export {};
    47 | 
Version: typescript 3.8.3
Time: 2461ms

To Reproduce Steps to reproduce the behavior:

  1. Open node_modules/@vue/apollo-composable/dist/useMutation.d.ts
  2. Go to L45 C340
  3. See TypeScript error

Expected behavior No errors.

Versions vue: 2.6.11 @vue/composition-api: 0.5.0 @vue/apollo-composable: ^4.0.0-alpha.8 apollo-client: 2.6.8

Additional context I used JetBrains Rider to look into the error.

sylvaingi commented 4 years ago

I'm able to reproduce if TS (currently 3.8.3) is configured with the strict option enabled.

geekox86 commented 4 years ago

Yes, my TS is 3.8.3. Here is my package.json:

{
  "name": "oemis",
  "version": "0.1.0",
  "private": true,
  "scripts": {
    "serve": "vue-cli-service serve",
    "build": "vue-cli-service build"
  },
  "dependencies": {
    "@fortawesome/fontawesome-pro": "^5.13.0",
    "@vue/apollo-composable": "^4.0.0-alpha.8",
    "@vue/composition-api": "^0.5.0",
    "apollo-cache": "^1.3.4",
    "apollo-cache-inmemory": "^1.6.5",
    "apollo-client": "^2.6.8",
    "apollo-link": "^1.2.13",
    "apollo-link-batch-http": "^1.2.13",
    "apollo-link-scalars": "^0.1.6",
    "apollo-utilities": "^1.3.3",
    "core-js": "^3.6.4",
    "graphql": "^14.6.0",
    "graphql-anywhere": "^4.2.6",
    "graphql-tag": "^2.10.3",
    "moment": "^2.24.0",
    "roboto-fontface": "^0.10.0",
    "vue": "^2.6.11",
    "vue-router": "^3.1.6",
    "vuetify": "^2.2.21"
  },
  "devDependencies": {
    "@vue/cli-plugin-babel": "^4.3.1",
    "@vue/cli-plugin-router": "^4.3.1",
    "@vue/cli-plugin-typescript": "^4.3.1",
    "@vue/cli-service": "^4.3.1",
    "babel-plugin-graphql-tag": "^2.5.0",
    "sass": "^1.26.3",
    "sass-loader": "^8.0.2",
    "syncyarnlock": "^1.0.19",
    "typescript": "^3.8.3",
    "vue-cli-plugin-pug": "^1.0.7",
    "vue-cli-plugin-vuetify": "^2.0.5",
    "vue-template-compiler": "^2.6.11",
    "vuetify-loader": "^1.4.3"
  }
}
tobias-kuendig commented 4 years ago

I created a possible solution in #955.

Maybe someone can chime in and test the change. With the change incorporated, I got rid of all TS errors in my application. There might be other use-cases that are not covered, though.

chanlito commented 4 years ago

Facing this issue as well, anyone has a work around in the mean time?

tobias-kuendig commented 4 years ago

@chanlito Can you manually test this fix?

https://github.com/vuejs/vue-apollo/pull/955/files

You will have to patch this in node_modules/@vue/apollo-composable/dist/useMutation.d.ts:45

lucapircher commented 4 years ago

@tobias-kuendig Applying your proposed change in #955 solves the issue for me.

lucapircher commented 4 years ago

@chanlito I do not face the issue if I use alpha.7

bbugh commented 4 years ago

Hi! 👋 I'm the original author for the types of v4 in #895 and I believe the issue you're reporting was caused by a9d950156d1f8e89cf8ba3532936b6a09e0b776a.

~There's an underlying incorrect assumption on my part that is causing this issue, that I believe will be resolved after the PR for https://github.com/vuejs/vue-apollo/issues/961.~ This is not correct, see comment below.

bbugh commented 4 years ago

Ok, after digging into this more, I don't think the issue I linked is the problem, or even a problem. I thought all of these issues were related but it seems like it might not be.

It's quite difficult adding types to an API that was originally typed with any and didn't take type usage into account. 😓

I think that the real issue and the reason why this reported issue exists is that useQuery, useSubscription, and mutate all use the signature (variables, options) but useMutation uses just (options). mutate using a different method signature than the parent method it comes from makes it extra effort to type https://github.com/vuejs/vue-apollo/commit/a9d950156d1f8e89cf8ba3532936b6a09e0b776a was added to fix it, but it's sort of a hack. If mutate took the same options argument type that useMutation did, it wouldn't require these extra typing workarounds.

It might also be possible to resolve it with an inner type that selects the correct option:

diff --git a/packages/vue-apollo-composable/src/useMutation.ts b/packages/vue-apollo-composable/src/useMutation.ts
index 5825ae1..74d155f 100644
--- a/packages/vue-apollo-composable/src/useMutation.ts
+++ b/packages/vue-apollo-composable/src/useMutation.ts
@@ -40,8 +40,8 @@ type MutateResult<TResult> = Promise<FetchResult<TResult, Record<string, any>, R
 export type MutateWithOptionalVariables<TResult, TVariables> = (variables?: TVariables, overrideOptions?: MutateOverrideOptions) => MutateResult<TResult>
 export type MutateWithRequiredVariables<TResult, TVariables> = (variables: TVariables, overrideOptions?: MutateOverrideOptions) => MutateResult<TResult>

-export interface UseMutationReturn<TResult, TVariables, Mutate extends MutateWithOptionalVariables<TResult, TVariables> = MutateWithOptionalVariables<TResult, TVariables>> {
-  mutate: Mutate
+export interface UseMutationReturn<TResult, TVariables> {
+  mutate: TVariables extends undefined ? MutateWithOptionalVariables<TResult, TVariables> : MutateWithRequiredVariables<TResult, TVariables>,
   loading: Ref<boolean>
   error: Ref<Error>
   called: Ref<boolean>

I'm not sure how to proceed with this, and I don't have any more time to look at it right now.

bbugh commented 4 years ago

Ok, I think issue #961 was the problem. Sorry for all of the comments, I must look like a crazy person now. 🤪 I blame 2+ months of quarantine.

I have submitted a PR here: https://github.com/vuejs/vue-apollo/pull/962 If you're invested in this outcome, please check it out.