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

Proposal: remove model.validate #96

Closed
sylvainpolletvillard opened this issue Aug 22, 2018 · 7 comments
Closed

Proposal: remove model.validate #96

sylvainpolletvillard opened this issue Aug 22, 2018 · 7 comments
Labels

Comments

@sylvainpolletvillard
Copy link
Owner

sylvainpolletvillard commented Aug 22, 2018

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
Copy link
Contributor

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
Copy link
Owner Author

sylvainpolletvillard commented Aug 24, 2018

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
Copy link
Contributor

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

@sylvainpolletvillard
Copy link
Owner Author

sylvainpolletvillard commented Aug 26, 2018

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
Copy link

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

@sylvainpolletvillard
Copy link
Owner Author

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
Copy link
Owner Author

Shipped in v4 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants