Switch from callbacks to events

[danvk]

dygraphs offers many callbacks right now. They all share the flaw that you can't set multiple functions, which makes writing tools like Dygraph.synchronizer harder.

The web solved this long ago by switching to the addEventListener model. jQuery and other libraries offer .on() and .off() as convenient wrappers around this.

dygraphs should offer something similar. There are the beginnings of such a system for plugins now, but it should be part of the core API.

There are many JS libraries which offer event triggering & registration. A few are:

• backbone.js
• bean
• EventEmitter

Usage would looks something like:

var g = new Dygraph(...);
g.on('point-click', function(e) {
  console.log(e.xvalue);
}).on('redraw', function(e) {
  console.log('chart redrew!');
});

[volkermauel]

I'd like to change the code to use events rather than callbacks.

• What should happen to legacy code with callbacks being set? Should backward compatibility be provided and the callback converted to an event?
• Should the events be fired synchronously or asynchronously? Should there be an option to to use to let the user decide when it is safe to use an asynchronous execution?

[mKlus]

You could consider using js-signals: http://millermedeiros.github.io/js-signals/

Advantages

Doesn't rely on strings for subscribing/publishing to a event type: Trying to dispatch or listen to an event type that doesn't exist throws errors instead of failing silently which helps to identify errors early. Code completion friendly. Arbitrary number of parameters to event handlers. Easy control of the event broadcaster and subscriber, avoiding that the wrong objects reacts to the event. Convenience methods that usually aren't present on other implementations of the observer pattern like: Enable/disable event dispatching per event type. Remove all event listeners attached to a specific event type. Option to automatically remove listener after first execution. Option to bind an execution context to the event handler, avoiding scope issues. Remove/detach anonymous listener. Etc... Favor composition over inheritance. Easy to identify which event types the object dispatch.

And also priority for listeners: The priority level of the event listener. Listeners with higher priority will be executed before listeners with lower priority. Listeners with same priority level will be executed at the same order as they were added.

[danvk]

Events should be added for the 2.0 release, where they'll co-exist with callbacks. Callbacks can be deprecated in a subsequent release and removed entirely from a 3.0 release.

I'd also like to take this opportunity to clean up a few warts:

• Each event listener should take a single argument, an Object stuffed with information relating to the event. This is more flexible than the positional parameters that the current callbacks take.
• this should be set to the Dygraph object in all event listeners. The Dygraph object should not be passed as a parameter.

The event registration system used by plugins is a step in this direction. It also defines behaviors for calling preventDefault on the event.

I don't have strong preferences about the event library. My mild preference would be to use something string-based, since that will be more familiar to most JS developers.

[reinert]

I was navigating on the issues and just faced with this one. I must say that this approach of firing events is definitely better than setting callbacks.

So right that I was doing exactly that in my dygraphs polymer wrapper (wip poor docs).

You can check here how nice it is to use this approach.

For the core lib I suggest firing the events with the dispatchEvent api.