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

TypeScript requiring variables based on types in v4 has incorrect behavior #961

Open
bbugh opened this issue Apr 21, 2020 · 1 comment
Open

Comments

@bbugh
Copy link
Contributor

bbugh commented Apr 21, 2020

Describe the bug

While recently working with the author of the vue-apollo-composable generator for graphql-code-generator I discovered that I made a bad assumption while bolting-on the types for v4 in #895.

That assumption was that you can use the types to determine if variables are required or not. This is wrong, because all of the GraphQL variables may be all optional, which means that requiring variables is incorrect TypeScript behavior.

The unfortunate side effect of this is that people using the raw composition API will not get type check warnings if they forget to specify variables, though it will correctly type check the variables if it is specified. That's a bummer, but it can be resolved something like the graphql-code-generator to create wrapper hooks that ensure that the hook functions expect the correct inputs.

Fixing this issue would also have a benefit of simplifying the Frankenstein of types, and fix a few issues that I listed at the bottom.

I will submit a PR for this ASAP. PR fix #962 created

To Reproduce
Given this query with optional arguments:

query projects($text: String) {
  projects(text: $text) {
    id
  }
}

And this vue-apollo-composable code:

  return VueApolloComposable.useQuery<ProjectsQuery, ProjectsQueryVariables>(
    ProjectsDocument,
    variables,
    options
  );

Variables is actually optional here, but the typing makes it required.

Expected behavior

At all times in the API, we must allow variables to be undefined, regardless of how it is typed. This will just have to be a shortcoming of the TypeScript typing.

Versions
@vue/apollo-composable: 4.0.0-alpha.8

Additional context

I believe this would resolve these issues:

@bbugh bbugh changed the title TypeScript requiring variables based on types in v4 is incorrect behavior (and it's my fault) TypeScript requiring variables based on types in v4 has incorrect behavior Apr 21, 2020
@cerinoligutom
Copy link
Contributor

cerinoligutom commented Apr 23, 2020

+1

Edit:
Just started using this, the logs are so annoying to look at right now.

Edit 2:
I cannot build the app bootstrapped with vue cli.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants