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

optional parameters in mutation #852

Closed
seeden opened this issue Oct 28, 2016 · 13 comments
Closed

optional parameters in mutation #852

seeden opened this issue Oct 28, 2016 · 13 comments
Assignees
Labels

Comments

@seeden
Copy link
Contributor

seeden commented Oct 28, 2016

I would like to use some parameters in mutations as optional. But I will get next error when I skip second "title" parameter: The inline argument title is expected as a variable but was not provided.

Here is the gql example

  mutation ($id: ID!, $title: String, $language: String) {
    quiz: update(id: $id, title: $title, language: $language) {
      id
    }
  }

As you can see only id is required parameter. But when I will call next mutation I can see described error.

await update({
      variables: {
         id: 1,
         language: 'en',
      },
    });

The truth is that I expected default "undefined" value for title.

@seeden
Copy link
Contributor Author

seeden commented Oct 28, 2016

I will try to explain why I want to have this feature. I have "Main" component which can use mutation. This component has two routes. Each of them can update a different fields but when one of them wanted to save data it call mutation in the parent component via "onSubmit" callback.

@seeden seeden changed the title optional parameters in mutation optional parameters in mutation | feature Oct 28, 2016
@helfer
Copy link
Contributor

helfer commented Oct 28, 2016

@seeden Thanks for reporting it. I think that counts as a bug, because we want to support the full GraphQL spec.

@helfer
Copy link
Contributor

helfer commented Oct 28, 2016

@seeden Could you submit a PR with a failing test? I think it should be easy to do and would help us a lot!

@stubailo
Copy link
Contributor

Yeah this is a bug where Apollo Client caching expects variables to exist in the variables dictionary: https://github.com/apollostack/apollo-client/blob/56c3cf9033f953bab07bd7d725497f4962120aec/src/data/storeUtils.ts#L62

Should be a one-line fix to replace those with null in the store. BTW this error is duplicated in graphql-anywhere: https://github.com/apollostack/graphql-anywhere/blob/5335986c60786d62734f701385648a7da59ecff1/src/storeUtils.ts#L56

We should factor out storeUtils and put them in graphql-anywhere only.

@seeden
Copy link
Contributor Author

seeden commented Oct 29, 2016

@stubailo can you use undefined instead of null? Because null value is valid database value and I will use it on the server. When I get undefined I can skip it in my "update" query. I am using temporary fix for my mutation with undefined values.

      variables: {
        title: undefined,
        language: undefined,
        ...attributes,
      },

@seeden seeden changed the title optional parameters in mutation | feature optional parameters in mutation Oct 29, 2016
@stubailo
Copy link
Contributor

Does GraphQL on the server treat undefined differently than null?

@seeden
Copy link
Contributor Author

seeden commented Oct 29, 2016

Yes. When I send undefined I will get undefined and when I send null I will get null. If the variable is undefined I will ignore this variable on the server but I will use it with null value. I am not sure maybe you can ignore undefined variable on the client and graphql will be ok because I will not get any undefined variables

@stubailo
Copy link
Contributor

Sounds good. If GraphQL servers treat undefined and null differently, we should as well!

@helfer
Copy link
Contributor

helfer commented Oct 30, 2016

@stubailo @seeden currently there's no difference between null and undefined

@seeden
Copy link
Contributor Author

seeden commented Oct 30, 2016

Thanks @helfer for information. Working with null will be changed as you can see in the merged thread graphql/graphql-spec#83

@helfer
Copy link
Contributor

helfer commented Oct 31, 2016

@seeden which is why I said "currently" 😉 We'll wait until the changes are made in graphql-js, and then update Apollo Client accordingly.

@seeden
Copy link
Contributor Author

seeden commented Oct 31, 2016

ok than we can use default "null" values as @stubailo mentioned because graphql has support for optional parameters in mutation

@lorensr
Copy link
Contributor

lorensr commented Dec 5, 2016

null landed in graphql-js in 0.8.0: https://github.com/graphql/graphql-js/releases/tag/v0.8.0

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

5 participants