833172 - Need an easy way to listen to observer notifications

Status: RESOLVED INCOMPLETE

(Add-on SDK Graveyard :: General, defect)

Description

[Erik Vold [:erikvold] (please needinfo? me)]

Now that the observer service is being deprecated we need a easy way to do this:

hearThis('some-observer-notification', function() {/* stuff to do */});

the system/events module doesn't seem to provide this.

Comment 1

[Alexandre Poirot [:ochameau]]

Actually, you can already do that with its `on` method:
require("system-events").on(
  "some-observer-notification",
  function ({ subject, data, type }) {

  });
See unit tests:
https://github.com/mozilla/addon-sdk/pull/711

Comment 2

[Erik Vold [:erikvold] (please needinfo? me)]

I don't see that test case.  I was using code like that recently in the private-browsing/utils module and the tests failed because that listener was being gc'd too soon.

I events.on() usage is deceivingly similar to observer-service sadly..

Comment 3

[Alexandre Poirot [:ochameau]]

(In reply to Erik Vold [:erikvold] [:ztatic] from comment #2)
> I don't see that test case.

Here is one:
https://github.com/Gozala/addon-sdk/blob/041fac3b5e0eb8e40ef1d279aaa7f3a5395bbe6b/test/test-system-events.js#L154-L184


So do you want to morph this bug into changing default on/once behavior or improve doc or something else? Your response isn't very clear and we should close this bug somehow.

Comment 4

[Erik Vold [:erikvold] (please needinfo? me)]

(In reply to Alexandre Poirot (:ochameau) from comment #3)
> (In reply to Erik Vold [:erikvold] [:ztatic] from comment #2)
> > I don't see that test case.
> 
> Here is one:
> https://github.com/Gozala/addon-sdk/blob/
> 041fac3b5e0eb8e40ef1d279aaa7f3a5395bbe6b/test/test-system-events.js#L154-L184

This is testing `emit` not `on` afaict.

> So do you want to morph this bug into changing default on/once behavior or
> improve doc or something else? Your response isn't very clear and we should
> close this bug somehow.

I'd like to change how require('system/events').on works, but I'll let Irakli decide.

Comment 5

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Erik, It's not clear to me what this is about. As Alex pointed out there `event.on('something', handler)` should work. You're right in regard that
handler can be GC-ed, but that's not accidental, it's intentional and there are tests to ensure that too:
https://github.com/Gozala/addon-sdk/blob/041fac3b5e0eb8e40ef1d279aaa7f3a5395bbe6b/test/test-system-events.js#L70

If you don't want handler to be gc-ed you can either hold a reference to it for the time you wan't to keep it alive, or use third argument as shown here to use strong reference:
https://github.com/Gozala/addon-sdk/blob/041fac3b5e0eb8e40ef1d279aaa7f3a5395bbe6b/test/test-system-events.js#L76

Note that when using observer-service module you keep callback and anything it encloses over for the life time of the add-on, or you have to cleanup yourself.
This let's GC claim those things if they are not used. If you just need to keep
listener alive for the life time of the add-on that's pretty simple too:

function handler() { ... }
events.on('foo', handler)

This causes module to keep reference to your `handler` function for it's life time, which is very likely add-on's lifetime. So it would behave in equivalent way of deprecated observer-service.

For more advance usages you can take a look at `Disposable` that will keep unload handler alive as long as there are refs to the instance itself, which is great as
manual tear down becomes unnecessary.

If you have any specific suggestions please give more details.

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Erik, can you maybe write up a JEP for this instead?