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

arg with null should turn to defaultValue or stay null? #210

Closed
jardakotesovec opened this issue Oct 19, 2015 · 11 comments
Closed

arg with null should turn to defaultValue or stay null? #210

jardakotesovec opened this issue Oct 19, 2015 · 11 comments

Comments

@jardakotesovec
Copy link

Seems that graphql-js treats args that are undefined (not present in json) and which are null (null value in json) the same way. Which is generally fine.

But I have situation where I have nullable field with defaultValue and I expected that if I explicitly send name: null I will get args.name = null and not the defaultValue, which is graphql-js behavior.

What do you think?

@gyzerok
Copy link

gyzerok commented Oct 20, 2015

@jardakotesovec You set defaultValue when value is not presented - undefined. In your case value is presented. Its normal behaviour.

@jardakotesovec
Copy link
Author

Just to make sure its clear. Here is example.

Lets say I have nullable string called name, with defaultValue:jarda

In graphql-js, if I send name: null, I get name = jarda.
Which was not expected behavior. I expected getting name = null in that situation.

@gyzerok
Copy link

gyzerok commented Oct 20, 2015

@jardakotesovec oh, sorry, I've got it wrong. It seems to me, that you are right and it may be considered as a bug.

@leebyron
Copy link
Contributor

This is expected behavior for GraphQL. The library currently does not distinguish between undefined and null values for input. So both are replaced with a default value.

One nice thing about this from a server development point of view is that if you provide a default value for an argument, you can be guaranteed to always get a non-null value for that argument.

Is the behavior you're describing something that you have a legitimate use case for, or was this just something that caused confusion?

@jardakotesovec
Copy link
Author

I was using connection argument saved which was boolean with defaultValue true. Because by default I wanted to return saved images and I wanted to have option to return all images, so I thought that if I send saved: null, I will use this null to disable this filter.

I guess this could be solved easily by ENUM.. (to have 3 different states, with defaultValue).

And I agree with you that this behavior is less error prone as its easy to mix up undefined/null values on client and that guarantee of having defaultValue on server is nice.

Thanks for explanation!

@gyzerok
Copy link

gyzerok commented Oct 21, 2015

@leebyron The problem here is that React and GraphQL treats null differently. In React defaults only applies to null.

@jardakotesovec
Copy link
Author

I have run into this issue again with bit different use case. So reopening for further discussion. I wonder how other deal with it.

Be able differentiate (as react) between undefined and null is useful anytime you consider null as actual value.

Lets say I have updateUser mutation with input name(string) and age (number). I want to be able to update name and age independently (update only if value is present), because I update these things in different place and time in app.

Therefore I update only input that is not undefined (= sent as null or not sent at all). But how about if I need to remove that age. I don't want to set age to zero, I want to say that we don't know age information and set it to null. At this moment I can't distinguish if age is not supposed to be set or should be set to null.

Its simplified example, but I think situations when make sense to set something to null (usually for objects than numbers) is not that uncommon.

@clintwood
Copy link

I have similar issue to @jardakotesovec. I realize GraphQL is language agnostic so how does one deal with null vs undefined in JavaScript where null is in fact an object type. I have cases where I'd like a defaultValue of null on the response to client HTTP request to explicitly indicate to the client that there is no value for that field as opposed to it just not being defined (JSON.stringify will output null fields but drops undefined from output).

At the moment, for GraphQLString at least, I use an empty string '' as a defaultValue where I'd prefer to use null. This is difficult for numeric types (perhaps using Number.NaN which JSON.stringify translates to null may be an option but have not tried it as a defaultValue with GraphQLInt).

@seanchas
Copy link

Perhaps one can provide additional boolean arguments like removeAge or removeName. By default they're set to false. Not very clean and elegant solution, but it can work.

@mattkrick
Copy link
Contributor

My solution is to create 2 separate input types, 1 for creating & 1 for updating. A little more boilerplate, but super robust. If there's a cleaner way to do it, I'd love to hear it!

// Create a base object
const inputFields = {
  id: {type: GraphQLID,},
  name: {type: GraphQLString},
  age: {type: GraphQLInt},
}

export const NewThing =  new GraphQLInputObjectType({
      name: 'NewThing',
      fields: () => makeRequired(inputFields, ['name', 'age'])
});

export const UpdatedThing =  new GraphQLInputObjectType({
      name: 'UpdatedThing',
      fields: () => makeRequired(inputFields, ['id'])
});

export const makeRequired = (fields, requiredFieldNames) => {
  const newFields = Object.assign({}, fields);
  requiredFieldNames.forEach(name => {
    return newFields[name] = Object.assign({}, newFields[name], {type: new GraphQLNonNull(newFields[name].type)})
  });
  return newFields;
};

@leebyron
Copy link
Contributor

leebyron commented Apr 7, 2016

Closing this issue in favor of graphql/graphql-spec#83 which tracks a potential path forward (though there are still open questions and concerns there)

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

No branches or pull requests

6 participants