sylvainpolletvillard / ObjectModel

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

Is there a way to set a default prototype? #104

Closed sylvainpolletvillard closed 5 years ago

sylvainpolletvillard commented 5 years ago

Is there a way to set a default prototype? The following doesn't seem to work.

const {EventEmitter} = require('events')
const Crypto = ObjectModel({
  name: String
}).defaultTo(EventEmitter.prototype)

I'm setting the prototype as follows but it would be nice to have a default.

Object.setPrototypeOf(
  Crypto({
    name: 'Tezos'
  })
  , EventEmitter.prototype
)

Originally posted by @RichAyotte in https://github.com/sylvainpolletvillard/ObjectModel/issues/93#issuecomment-456624754

RichAyotte commented 5 years ago

It looks like base is hardcoded to Object, if I'm reading this correctly.

https://github.com/sylvainpolletvillard/ObjectModel/blob/1c23c0fb600b94d082afd2b1e1f9c7dae8ee4858/src/object-model.js#L422

Is setting the prototype a feature that you'd consider?

sylvainpolletvillard commented 5 years ago

So, to make things clear and avoid confusion:

So the best option I think for your usecase is to use the extend method: const Crypto = ObjectModel({ name: String }).extend(EventEmitter)

Basically it is equivalent to Object.assign(Crypto.prototype, EventEmitter.prototype) but let you override the model prototype afterwards if needed.

RichAyotte commented 5 years ago

That definitely works. Not sure how I missed the extend method. The prototype remains Model but mixed in with EventEmitter prototype which is fine. Thank you for the detailed and quick response.

sylvainpolletvillard commented 5 years ago

No problem, glad to help

RichAyotte commented 5 years ago

@sylvainpolletvillard I spoke too fast. As soon as I tried to listen for an event, I got a TypeError. Here's the MWE.

'use strict'

const {EventEmitter} = require('events')
const {ObjectModel} = require('objectmodel')

const Bar = ObjectModel({
    name: String
}).extend(EventEmitter)

const bar = Bar({
    name: 'Foo'
})

bar.on('data', console.log)
bar.emit('data', 'hello world')

Error:

/node_modules/objectmodel/dist/object-model.js:102
        isModelInstance = i => i && is(Model, getProto(i).constructor),
                                                         ^

TypeError: Cannot read property 'constructor' of null
    at isModelInstance (/node_modules/objectmodel/dist/object-model.js:102:52)
    at cast (/node_modules/objectmodel/dist/object-model.js:256:69)
    at getProxy (/node_modules/objectmodel/dist/object-model.js:288:36)
    at newPath (/node_modules/objectmodel/dist/object-model.js:332:38)
    at controlMutation (/node_modules/objectmodel/dist/object-model.js:226:5)
    at Object.set (/node_modules/objectmodel/dist/object-model.js:331:13)
    at _addListener (events.js:217:29)
    at Proxy.addListener (events.js:271:10)
    at Object.proxifyFn [as apply] (/node_modules/objectmodel/dist/object-model.js:292:26)
    at Object.<anonymous> (/src/ee-test.js:24:5)

Maybe you can shed some light on the importance of this check? https://github.com/sylvainpolletvillard/ObjectModel/blob/1c23c0fb600b94d082afd2b1e1f9c7dae8ee4858/src/object-model.js#L50

Would the following be a safe fix or would it potentially cause more problems?

isModelInstance = i => i && getProto(i) && is(Model, getProto(i).constructor),
sylvainpolletvillard commented 5 years ago

Ah, right, this lib creates objects with no prototypes (https://github.com/Gozala/events/blob/master/events.js#L86 )

Your suggested fix looks perfect, let me add some tests and release a new patch version

sylvainpolletvillard commented 5 years ago

Just released v3.7.7, can you confirm it fixes your issue ?

RichAyotte commented 5 years ago

v3.7.7 works well. Thanks again!