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

Cannot read property 'length' of undefined #28

Closed
vbarbarosh opened this issue Sep 3, 2016 · 3 comments
Closed

Cannot read property 'length' of undefined #28

vbarbarosh opened this issue Sep 3, 2016 · 3 comments

Comments

@vbarbarosh
Copy link

vbarbarosh commented Sep 3, 2016

$ node --version
v6.5.0
$ cat node_modules/objectmodel/package.json | grep version
"version": "2.2.0"
// Default is [1, 0, 0, 1, 0, 0]
var Matrix = Model.Array(Number).assert(v => v.length == 6);

Matrix.prototype.toString = function () {
    var [a, b, c, d, e, f] = this;
    return `matrix(${a} ${b} ${c} ${d} ${e} ${f})`;
};

var Shape = Model({
    matrix: Matrix,
    path: String
}).defaults({matrix: new Matrix([1, 0, 0, 1, 0, 0])});

new Shape({path: ''}); // OK

var Picture2 = Model({
    matrix: [Matrix],
    shapes: Model.Array(Shape)
}).defaults({shapes: []});

new Picture2(); // Error: Cannot read property 'length' of undefined
@sylvainpolletvillard
Copy link
Owner

Here is what is happening:

  • matrix property is optional so interpreted as [Matrix, undefined]
  • with the changes from Automatic model casting with union types #24, every part of a union type is tested to check if suitable
  • matrix value is undefined for Picture2
  • undefined is tested against the Matrix model definition
  • your assertion v > v.length == 6 throws this exception when v is undefined

So an easy fix is to change your assertion to .assert(v => v && v.length == 6)

The same thing happens when you do this:

var Matrix = Model.Array(Number).assert(v => v.length == 6);
new Matrix();

Now we can think of some possible improvements to prevent this kind of errors. For example, always reject falsy values on array/object models without testing assertions. I need to think about it and see what is the most predictive behaviour.

@sylvainpolletvillard
Copy link
Owner

sylvainpolletvillard commented Sep 4, 2016

Okay, I have a much simpler proposal: use try/catch on assertions. If an assertion throws an exception, it means obviously that the assertion failed. There is a little performance penalty but the development experience is much improved and it fits well with my goal to make ObjectModel as transparent as possible.

This will be fixed in v2.3, which will also improve error messages on assertion failures:
chrome_2016-09-04_18-38-44

@sylvainpolletvillard
Copy link
Owner

v2.3 is out 👍

# 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