Need a way to remove renderlets

[davisford]

I just realized that calling chart.renderlet() will build up an array of functions (even the same function), which will continue to spazz out if you end up rebuilding your crossfilter data multiple times, and again calling renderlet.

Example of what I mean:

var chart = dc.barChart('#barchart');

// rebuild charts with data
function rebuildCharts(data) {
  chart
    .dimension(/* dimension data */)
    .group(/* group data */)
    .renderlet(function (chart) {
        // do something with chart based on data
    })
  dc.renderAll();
}

rebuildCharts([1, 2, 3]);
// renderlet is called and data in closure bound to [1, 2, 3]
rebuildCharts([4, 5, 6]);
// renderlet is called and data in closure bound to [4, 5, 6]
// renderlet is called and data in closure bound to [1, 2, 3]

There is no way to remove or clear renderlets, and it doesn't check if the function is already registered.

I think there needs to be a function to clear a specific renderlet, or clear all renderlets. I only realized this now b/c the scope of the data I was referencing in my renderlet function from the closure was stale (b/c the previous renderlet closure had a reference to it -- so this is causing some bad news memory leaks for me the way I am using it).

I would propose a method on chart / base chart:

chart.clearRenderlet(fn); // clears that particular function
chart.clearRenderlets(); // just clears them all

Thoughts? @gordonwoodhull if you agree I'll send a PR. If you semi-agree, or have a different approach / suggestion, I'd love to hear it.

[gordonwoodhull]

I think the only reason renderlet isn't just an event, is that d3.dispatch didn't exist when Nick wrote dc.js.

d3.dispatch provides a consistent way to add, replace, delete, and trigger events. We wouldn't need any of the custom logic if we used that mechanism. It also provides for namespacing of events like jQuery does, so that it's easy to replace or remove a particular listener by name instead of having to keep the function pointer.

But that wouldn't be a backward-compatible change, so probably your approach is the best we can do now. I'll sit on this for a few days but will probably merge your PR for 2.0. Thanks!

[gordonwoodhull]

fixed by #833