sylvainpolletvillard / ObjectModel

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

Error on instanciating a model using ES6 classes #80

Closed klues closed 6 years ago

klues commented 6 years ago

I'm using ObjectModel v2.6.3 with the following models:

class GridData extends Model({
    id: String,
    label: [String],
    rowCount: Number,
    gridElements: Model.Array(GridElement)
})

class GridElement extends Model({
    id: String,
    width: Number,
    height: Number,
    x: [Number],
    y: [Number],
    speakText: [String],
    label: [String]
})

now if I try to instantiate a GridData element like this:

let json = ... //json containing all needed properties for GridData and an array of GridElements
var gridData = new GridData(JSON.parse(json));

I get the following error on console: object-model.js:180 Uncaught TypeError: Class constructor GridElement cannot be invoked without 'new'. I've inspected the compiled bundle I'm using and found out that I could fix the error with this commit: https://github.com/asterics/AsTeRICS-Ergo-Grid/commit/935560c4021c7664adfeba7a48692ad03118b0c6

I did the same changes in this PR: https://github.com/sylvainpolletvillard/ObjectModel/pull/79 I don't know if it makes sense or if it breaks something, but at least in my case it seems to fix the error.

sylvainpolletvillard commented 6 years ago

Yes, v3 has many fixes around ES6 classes extensions that have not been reported to v2. The truth is that I added ES6 classes instanciation to v2 docs too early, without exhaustive testing. I should have wait for v3, since it is an ES6 feature. Let's try to fix that.

You were on the right track with this PR, but there is more work to do to pass all the v3 tests. I'll do that today, thanks for the investigation and report.

By the way, if you are using v2 to get IE support and at the same time use ES6 classes, that means you are using Babel to transpile the classes to ES5. What's funny is that the requirement "Class cannot be invoked without new" is added by Babel to follow the ES6 spec, but is actually not required in ES5. So maybe it works in loose mode

Anyway, I'll try to report the fixes, stay tuned

klues commented 6 years ago

What's funny is that the requirement "Class cannot be invoked without new" is added by Babel to follow the ES6 spec, but is actually not required in ES5. So maybe it works in loose mode

Yeah, right, I'm using babel. However I follow an approach that allows to use the transpiled ES5 code only in older browsers and run the native ES6 code in newer ones. Therefore the loose mode will not help for me, because in new browsers the actual class definitions are used. See this site explaining the approach, if you are interested: https://philipwalton.com/articles/deploying-es2015-code-in-production-today/

sylvainpolletvillard commented 6 years ago

Ah yes I heard about this approach, it's probably the best you can do while waiting for IE to die for good.

sylvainpolletvillard commented 6 years ago

I released v2.6.4, all v3 tests related to es6 classes now pass on v2. Can't test with Babel on IE though, because it depends on your version and configuration. Let me know if you still have errors.

klues commented 6 years ago

Tested 2.6.4 in my project on IE11 and all works fine now 👍 Thanks for the fix!