goinstant / goangular

AngularJS bindings for GoInstant. Build realtime, multi-user apps with AngularJS and GoInstant easily. https://developers.goinstant.com/v1/GoAngular/index.html
BSD 3-Clause "New" or "Revised" License
137 stars 30 forks source link

Model#$on confusing #57

Open colinmacdonald opened 10 years ago

colinmacdonald commented 10 years ago

Currently there are 5 events that can be listened for using model#on.

3 of these events are GoInstant key events, add, set, remove which can take an optionsObject and a listener:

model.$on('add', { local: true }, function() {})
model.$on('set', { bubble: true, listener: function() {} })
model.$on('remove', function() {})

The other 2 are ready and error which do not take an options object:

model.$on('ready', function() {})
model.$on('error', function(errObj) {})

These inconstancies could make the method confusing because how you call the method changes based on the eventName you passed.

Some alternative interface ideas:

Separate the key events from the local model events

model.$key.on // for 'add', 'set', 'remove' key events
model.$on // for model-only events ('ready', 'error')
// error and ready events given their only method that calls a listener
model.$error
model.$ready

// reserve model.$on for key events
model.$on('add', optsObj, listener);
colinmacdonald commented 10 years ago

cc: @ianlivingstone @mattcreager

colinmacdonald commented 10 years ago

error events should also be emitted when a $set, $add, etc. failure occurs, currently they do not.

colinmacdonald commented 10 years ago
colinmacdonald commented 10 years ago

I think replacing ready with sync could be confusing, sync sounds like it will trigger any time the local model is updated instead of just the initial sync. However, I do think we need an event like this to track any changes to the local model.

model.$on('syncd/changed')