tv4 icon indicating copy to clipboard operation
tv4 copied to clipboard

Handle undefined properties

Open noirbizarre opened this issue 10 years ago • 9 comments

This pull-request allow to validate objects with properties set to undefined if the properties are not required.

It seems coherent with this:

JSON.stringify({nullProp: null, undefinedProp: undefined})
>> {"nullProp":null}"

So with this schema:

var schema = {
    properties: {
        key: {type: 'string'}
    }
};

we should have:

tv4.validate({key: undefined}, schema);
>> true
tv4.validate({key: null}, schema);
>> false

noirbizarre avatar Dec 27 '14 05:12 noirbizarre

Hi! Any comment on this one ? Is it possible to merge it ?

noirbizarre avatar Jun 23 '15 07:06 noirbizarre

Sorry for the delay.

I like this, but it may represent a subtle change in behaviour. I'm trying to figure out whether it will screw up anybody's existing validation.

The spec's understandably silent on undefined values, so I think it might be safe, though. This is probably good to go in. (@Bartvds, any objection?)

geraintluff avatar Jun 23 '15 14:06 geraintluff

No worries, it was only to make sure you've seen it ;)

I agree, it introduce a very subtle change, but as you said, it was not specified. I think this is because undefined is a JavaScript only concept and does not exists in JSON (which explain the JSON.stringify behavior), so it's not defined in JSON Schema (which is consistent).

noirbizarre avatar Jun 23 '15 17:06 noirbizarre

+1 Any chance we merge this in?

klyngbaek avatar Sep 25 '15 09:09 klyngbaek

This is actually the only thing keeping me from using this module :)

klyngbaek avatar Sep 25 '15 09:09 klyngbaek

Hi! Any news on this one ?

noirbizarre avatar Oct 12 '15 09:10 noirbizarre

I just made a PR for this same issue without noticing this one. Seems like this is a fix that a lot of people need!

polpo avatar Oct 23 '15 17:10 polpo

Yes, this is the third PR on the topic.

I think we are just waiting for @Bartvds to confirm that it can be merged.

noirbizarre avatar Oct 24 '15 16:10 noirbizarre

Any progress on the merge of PR?

drom avatar Apr 28 '17 02:04 drom