Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Automatic model casting with union types #24

Closed
vbarbarosh opened this issue Aug 30, 2016 · 3 comments
Closed

Automatic model casting with union types #24

vbarbarosh opened this issue Aug 30, 2016 · 3 comments

Comments

@vbarbarosh
Copy link

After assignment a plain object to a dynamic property that property becomes typeless (in the example below I expect to get User instance but got an Object):

var Model = require('objectmodel');

var User = new Model({username: String, email: String})
    .defaults({username: 'foo', email: 'foo@foo'});
var Article = new Model({title: String, user: [User]})
    .defaults({title: 'foo', user: new User()});

var a = new Article();
console.log(a.user instanceof User); // true
// a.user.email = 1; // TypeError: expecting email to be String, got Number 1
a.user = {username: 'foo', email: 'foo'};
console.log(a.user instanceof User); // false
a.user.email = 1; // OK
a.user.foobarbaz = 123; // OK
console.log(a.user);
@sylvainpolletvillard
Copy link
Owner

Thanks for the report.

The problem comes from User being optional, which is the equivalent of a union type [User, undefined] and makes automatic model casting ambiguous here. I'll try to make automatic model casting working with union types as well, and keep you updated.

@sylvainpolletvillard sylvainpolletvillard changed the title Assign to a dynamic property Automatic model casting with union types Aug 31, 2016
@sylvainpolletvillard
Copy link
Owner

Current proposal as exposed in #25

if(isArray(defNode)){
    var suitableModels = defNode.filter(function(part){
        return part instanceof Model && obj && !(obj instanceof part) && part.test(obj)
    })
    if(suitableModels.length === 1){
        return suitableModels[0](obj); // automatic cast to the suitable model
    } else if(suitableModels.length > 1){
        console.warn(`Ambiguous model for value ${obj}, could be ${suitableModels.join(" or ")}`);
    }
    return obj;
}

When the property is defined by an union type containing several models, the library checks for each model if the object does validate its definition. If there is only one model that does the match, it is pretty straightforward and we can return an instance of this model. When there are several suitable models, the libary does nothing and return the object as it is, with a console warning "Ambiguous model for value X, could be Model A or B or C...."

I let this open for discussion and tests for a few days, then it will be released as v2.2

@sylvainpolletvillard
Copy link
Owner

A few changes to the proposal:

  • when the value is already a model instance, return the value unchanged and stop testing other parts of the union type
  • describe the internals of the value in the warning message when ambiguous

This issue is fixed in v2.2

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

No branches or pull requests

2 participants