sylvainpolletvillard / ObjectModel

Strong Dynamically Typed Object Modeling for JavaScript
http://objectmodel.js.org
MIT License
467 stars 30 forks source link

Proposal: remove model.validate #96

Closed sylvainpolletvillard closed 5 years ago

sylvainpolletvillard commented 6 years ago

Argument: there is no common usecase where someone would want to validate a model instance with autocast explicitely disabled. So model.test could be enough if we add a custom error collector as second argument.

Open to discussion, please comment if you need an { autocast: false } option. Use reactions to vote

gotjoshua commented 6 years ago

I am using validate, but if the functionality can be offered via an optional argument to test(), then it seems fine.

I do want to be able to test/validate an OM without throwing errors. ( related to #97 )

sylvainpolletvillard commented 6 years ago

the upgrade plan could be:

1) MyModel.test(x) => no change 2) MyModel.validate(x, errorCollector) => MyModel.test(x, errorCollector) 3) MyModel.validate(x) => new MyModel(x)

1) return true if valid, no errors thrown, autocast enabled 2) return true if valid, pass errors to custom collector, autocast enabled 3) validate, throw errors to console or model's own error collector

optional: if ppl really want to validate a model instance with autocast explicitely disabled, we could add a second { autocast: false } argument to Model constructors. But I prefer not to, because it no longer matches the original object constructor API, and is not future-friendly (maybe some future models would have several arguments in their constructors)

gotjoshua commented 6 years ago

Can you explain a bit more what autocast does (and when - and on what lines of your code base it is most relevant)?

sylvainpolletvillard commented 6 years ago

Autocasting (also referred as duck typing in the docs) happens when checking the model definition for an instance. This is the cast function in the sources: https://github.com/sylvainpolletvillard/ObjectModel/blob/master/src/object-model.js#L203

To explain it with an example:

const Person = ObjectModel({
    name: String,
    age: [Number]
});

const Lovers = ObjectModel({
    husband: Person,
    wife: Person
});

const joe = { name: "Joe", age: 42 };
const ann = new Person({
    name: joe.name + "'s wife",
    age: joe.age - 5
});

const couple = Lovers({
   husband: joe,  // object autocasted (duck typed)
   wife: ann // object model
});

couple.husband instanceof Person === true // object has been autocasted to Person

The joe object has been compared to the Person model definition, and it was valid. So the library autocasted this object to the Person model, that is to say, it invokes the Person constructor on this object and replace the property value with the newly created model instance.

This is really useful when you start to add methods or computed properties on your models. Usually, developers have to manually cast the data they receive to their suitable classes. With ObjectModel, you can use the type-checking phase to do this for you.

Autocasts works for object models properties, but also for Array/Map/Set models items when inserted, and for FunctionModel arguments and return value.

That's why I say ObjectModel is actually much more than just type checking. Once you have a mechanism to deeply traverse an object, and a declared definition to match with the developer's intention, you can bring all kind of interesting features to the table.

ryan-mahoney commented 5 years ago

Any ETA on this? Would love to have Model.test(someInstance, errors) :)

sylvainpolletvillard commented 5 years ago

Since it's a breaking API change, it must be shipped in the next major version (v4). V4 roadmap is not yet determined, there is much more stuff I would like to put in. Don't expect it any time soon.

Note that this is just API cleanup, not a new feature. You can already do MyModel.validate(x, errors => { console.log(errors) })

sylvainpolletvillard commented 5 years ago

Shipped in v4 👍