Skip to content

Instantly share code, notes, and snippets.

@Gozala
Last active December 16, 2015 07:29
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save Gozala/5399293 to your computer and use it in GitHub Desktop.
Save Gozala/5399293 to your computer and use it in GitHub Desktop.

This will keep listeners alive until add-on is unloaded, which is exactly a problem! In a lot of cases that's unnecessary overhead, in a lot of cases both your listener and object can be GC-ed a lot faster than add-on is unloaded. For example see this comment: https://github.com/mozilla/addon-sdk/pull/944/files#r3817523

What's even more important in a lot of cases such API introduced circular references that will cause both wrapper and listener to stay alive with a rest of the object graph they refer to.

Here is a very common case:

let Foo = Class({
  initialize: funciton(element) {
     this.doSomething = this.doSomething.bind(this);
     on("some-event", this.doSomething);
  },
  destroy: function() {
     off("some-event", this.doSomething);
  }
})

In this very common pattern, you introduced circular ref between Foo instance and bound this.doSomething which means only chance for GC to claim these is either add-on unload or manually call of destroy.

On top of this observers will still be fired once after add-on is unloaded, and you'll have to clear up listeners manually.

On the other hand consider this alternative:

function Foo(element) {
   function handler() {
      doSomethingWith(element)
   }
   on("some-event", handler);
   refs(this).add(handler);
}

In this case, which I'd argue is a lot simpler, you just associate handler to the foo instance and that's it. As soon as Foo instance is no longer referenced (which can be a lot earlier that add-on unload) will be GC-ed with all the event handlers associated with it. In addition you no longer have to manually destroy or remove listeners. This also means GC can schedule it's mark and sweep operations without stressing JS runtime too much.

Another option is to do almost the same as in original example, as a matter of fact do even less:

let Foo = Class({
  initialize: funciton(element) {
     this.doSomething = this.doSomething.bind(this);
     on("some-event", this.doSomething);
  }
})

This will still work because listener will be alive as long as foo instance is. As an additional benefit nothis needs to be done on destory.

Yet another option to make this pattern somewhat even less complicated is to extend system events such that target of listener can be passed as third argument ensuring that listener will be alive as long as target is:

function Foo(element) {
  on("some-event", function() {
    doSomethingWith(element)
  }, this)
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment