Consider replacing null values with the schema default during validation

[marshall007]

I think it would make sense for .validate() to populate defaults for null values.

var Doc = thinky.createModel('docs', {
  id: String,
  price: {
    _type: Number,
    default: 0
  }
}, { enforce_type: 'strict' });

Works as expected when doc.price is undefined:

var doc = new Doc({ id: 'UUID' });
doc.validate(); // doc.price === 0

Doesn't work when doc.price is null:

var doc = new Doc({ id: 'UUID', price: null });
doc.validate(); // throws "Value for [price] must be a number."

[neumino]

Hum, I'm a bit torn on this.

The value null is a valid in the sense that it can be saved in the database.

So if you want a default method for a field, and where null is a valid value, you need to generate null when the field is null, which doesn't seem nice.

[neumino]

Oh, and I forgot @marshall007 the most important -- Thanks for all the feedkback AND the PRs!

[marshall007]

@neumino my pleasure! I see what you mean, it might throw people off if they've been using enforce_type: false|loose. Maybe it makes sense to only do this for strict since that's the only case where null isn't considered valid?

On the other hand, setting a default value would seem to imply that the property is "non-nullable", at least to me. I'm having trouble imagining a scenario where you'd want both a default value and for null to be considered valid.

[mstade]

Use undefined instead of null perhaps?

[marshall007]

@mstade undefined values already get populated with their defaults. This issue is aimed at treating null the same way.

[mstade]

Sure, but what I mean is why not just set your values to undefined if you want defaults? This seems a bit magical to me; null is a value after all.

[neumino]

Sorry folks, this PR slipped through my mails.

I don't really want to change the default because that could break people code (to some extend, I try not to break anything). Part of the refactoring of the schema was to also split options. I was thinking about adding methods like allowNull(bool), allowUndefined(bool), replaceNull(bool), replaceUndefined(bool), but I also somehow forgot to do that.

The reason why it would nice to replace null, is mostly because of other parties. You may use a library that uses null to mark an undefined field.

[marshall007]

I don't really want to change the default because that could break people...

Agreed, it shouldn't be the default, but I think it makes complete sense under enforce_type:strict.