-
Notifications
You must be signed in to change notification settings - Fork 34
KDData: Proxy based realtime update object for kd.js #176
Conversation
lib/core/object.coffee
Outdated
registerKDObjectInstance: -> KD.registerInstance @ | ||
setDataAt: (path, value) -> | ||
@data = data | ||
return @data |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no data
at this point, i am guessing this is a copy & paste issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
forgot to remove that, good catch 👍
lib/core/data.coffee
Outdated
this.__proxy__ = new Proxy data, proxyHandler this | ||
this.__proxy__[key] = val for key, val of this when key isnt 'constructor' | ||
|
||
return this.__proxy__ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the first time i've seen something else being returned from a constructor, so not sure if this is gonna cause any problems. For example will the returned object from new kd.Data
be extended from kd.EventEmitter
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am guessing the answer is yes, because of the line base.emit 'update', [ path ]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a bit trivial there, when you create a new KDData
instance with provided data
you're actually getting a Proxy
object which has KDEventEmitter
functionality on it. Which provides us to use it as a native data object to use with in KDView
s.
lib/core/data.coffee
Outdated
@@ -0,0 +1,39 @@ | |||
KDEventEmitter = require './eventemitter' | |||
|
|||
module.exports = class KDData extends KDEventEmitter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd love to have more docs, and possibly tests covering this class, because I don't even understand the need of a Proxy
object here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Proxy
object is required to achieve observe functionality over KDData
I don't want to introduce specific setters to use on a KDData
instance, instead we can use these instances as a native objects/arrays.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💯
lib/core/object.coffee
Outdated
setOptions:(@options = {})-> | ||
setOption: (option, value) -> | ||
@options[option] = value | ||
return @options |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return value changes here, we must make sure this doesn't break anything
I've re-shaped the KDData to provide the data on instance as is, only difference will be the __proxy__ field under the data which we also need to use it with KDView.setData to listen update events from.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💯
On Travis only Chrome, Firefox and PhantomJS tests will run.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
KDData
returns aProxy
object wrapped withKDEventEmitter
which hasget
andset
traps for sendingupdate
events for changed fields. If used as a data to anyKDView
it can work fine with realtime updates and pistachio templates.A simple time example would be like;
To make things simpler it takes only one argument which is the data itself, that can be an
Array
or anObject
.Todo
Proxy
requirement for not supported browsersKDData.getEmitter
to return correct emitter if it was used to wrap aKDObject