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

Initial implementation of nullable type #329

Closed
wants to merge 4 commits into from

Conversation

ohde
Copy link

@ohde ohde commented Sep 24, 2016

Addresses #282

nullabletype

Note: I had issues implementing HOC for nullable types, it was forcing loss of focus in inputs. Therefore, I used down and dirty Parent -> Child solution to implement nullable type.

@n1k0
Copy link
Collaborator

n1k0 commented Sep 27, 2016

I really appreciate the amount of work done here. Though, I'm still trying to figure out the benefits of json nullable types in general, and am challenging our willing to support them in this form lib. It really seems to me that they try to mimic the required jsonschema directive.

I suspect there's a bunch of tools relying on them (I've seen people mentioning .Net in issues), but I'm still wondering if we really want to support these very specific, exotic-to-us use cases; this library was never supposed to cover the entire scope of jsonschema, especially its corner cases.

The issue I have with landing code specific to certain existing tooling is that we (at Mozilla) don't use it, and we're not really prone to maintain something we don't use.

But maybe I'm missing something obvious, and there's a valid use case for nullable types in jsonschema, which can't be achieved with a required field?

On the other hand, I see value in providing a button to clear any entered value for non-required field, so we would drop it from the resulting formData. Would that be an acceptable solution to you, or would you need actual support for nullable types because you critically depend on them?

@leplatrem
Copy link
Contributor

We'd like to prevent the library from growing too much (if it's not too late already).

Would it be possible to add only the necessary hooks here and leave the NullableField implementation in your project?

What do you think ?

@stathismor stathismor mentioned this pull request Nov 23, 2016
8 tasks
@crumblix
Copy link
Collaborator

You might want to see issue #402 (comment)
and also #410

The last one is mine. I'm an advocate of turning "empty string" and "empty array" into undefined.
The string and array items are removed from the JSON data ... in the same way that they dissapear from the data for numbers and dates currently.

This allows them to pass or fail validation when setting the "required" field. I personally plan to treat them as NULL in my backend and effectively disallow empty string (no loss for me).

Just throwing this out there in case it meets your requirements without adding a new nullable type :)

The array part of the patch has been submitted as a PR:

#420

The string part is referenced in my issue here: #410
but hasn't (and won't) be submitted as a PR until Issue #402 is decided upon.

Hope that helps!

@glasserc
Copy link
Contributor

glasserc commented Mar 1, 2017

I agree with @n1k0 that I don't really see the value of having null or undefined instead of a missing field. But I see in this PR the essence of a structure that could be useful for implementing oneOf (cf. #302, #417) or type: [] (as in #282). I think that if we could come up with a way to support "pick one" types on individual fields, that would be worth merging. Then, if we extend "custom fields" to let a user plug in some kind of "nullable field" like the one in the PR, I think that would cover the use case of #282 (assuming it wasn't already covered by #442).

@n1k0
Copy link
Collaborator

n1k0 commented Mar 12, 2017

No activity since last September, I'm closing this. Feel anyone free to reopen if you're planning to work on this.

@n1k0 n1k0 closed this Mar 12, 2017
@asindhu
Copy link

asindhu commented Mar 13, 2018

I've been looking for a solution to this as I do have a use case for it. It may be too specific to be worth it but the short of it is that at least for our use case, there is a behavior difference between a value not being present in an API payload and a value that is being explicitly cleared or unset.

The details: I have an object with several integer properties that I want to update via an API call. For an update operation, all properties are optional; anything that is not provided as part of the API payload remains unchanged. Of course, a property set to a certain integer in the payload would change the value. In addition, though, I need to be able to 'unset' or clear any of these properties, so I need a way of expressing this via JSON. In my opinion, using undefined for this is asking for trouble because checking the difference between a property in a JS object not being present at all and being set to undefined is error-prone in code. I'd much rather be able to set any given property to null instead.

So the behavior is something like this:

Payload {"iops_max":43} sets the value of iops_max to 43 and all other properties remain unchanged.

Payload {"iops_max":undefined} doesn't change anything. It would be the same as sending an empty object.

Payload {"iops_max":null} explicitly clears the value that is currently set.

So, I'd like my JSON schema to have type: ["integer", "null"] and accept both an update to a new integer value, and an intention to unset/clear. I appreciate that the difficult part here is having a UX that expresses this in an intuitive way... as others have said I would expect it could be done as a clear or 'x' control on the form field.

As far as I can tell this use case can't be addressed with the required field of JSON schema.

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