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

fix(ts): allow undefined for optional variables #962

Merged
merged 1 commit into from
Jul 27, 2020

Conversation

bbugh
Copy link
Contributor

@bbugh bbugh commented Apr 21, 2020

This fixes #961, fixes #948, fixes #998, and closes #955. Also should fix the issue raised in dotansimha/graphql-code-generator#3679.

The problem was that I added typing in #895 with the expectation that if TVariables was specified, then variables should be required. This is not the case, since queries, subscriptions, and mutations can have variables that are all optional. This is written up in #961.

This has the positive benefit of simplifying the Frankenstein's Monster that the types in useMutation were becoming.

@beeplin
Copy link

beeplin commented May 1, 2020

Also get the #948 problem. Hope it solves that.

@NickBolles
Copy link

@Akryum could we get this merged? I've tested it and it seems to work well.

@seanaye
Copy link

seanaye commented Jun 12, 2020

This doesn't seem to be getting merged, so how would one install this PR? I tried yarn add vuejs/vue-apollo#962/head but this is a monorepo and it gives me all sorts of errors when I try to compile

@tobias-kuendig
Copy link

@seanaye A temporary workaround is to add skipLibCheck: true to your tsconfig.json. But be aware that this will disable all type checking for anything in node_modules.

@bbugh
Copy link
Contributor Author

bbugh commented Jul 1, 2020

@Akryum what can we do to help get this merged? Vue 3 is rolling along but all vue-apollo TypeScript users are currently blocked from upgrading to v4 by this.

@Akryum Akryum merged commit 7495987 into vuejs:v4 Jul 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants