refactor(api): update INotifyMixin to clean up listeners

thi-ng · Nov 5, 2019 · ed2be64 · ed2be64
1 parent 4007351
commit ed2be64
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 12 deletions.
diff --git a/packages/api/src/mixins/inotify.ts b/packages/api/src/mixins/inotify.ts
@@ -31,9 +31,7 @@ export const inotify_dispatch = (listeners: any[][], e: Event) => {
 export const INotifyMixin = mixin(<INotify>{
     addListener(this: _INotify, id: string, fn: Listener, scope?: any) {
         let l = (this._listeners = this._listeners || {})[id];
-        if (!l) {
-            l = this._listeners[id] = [];
-        }
+        !l && (l = this._listeners[id] = []);
         if (this.__listener(l, fn, scope) === -1) {
             l.push([fn, scope]);
             return true;
@@ -42,23 +40,26 @@ export const INotifyMixin = mixin(<INotify>{
     },
 
     removeListener(this: _INotify, id: string, fn: Listener, scope?: any) {
-        if (!this._listeners) return false;
-        const l = this._listeners[id];
+        let listeners: IObjectOf<[Listener, any][]>;
+        if (!(listeners = this._listeners)) return false;
+        const l = listeners[id];
         if (l) {
             const idx = this.__listener(l, fn, scope);
             if (idx !== -1) {
                 l.splice(idx, 1);
+                !l.length && delete listeners[id];
                 return true;
             }
         }
         return false;
     },
 
     notify(this: _INotify, e: Event) {
-        if (!this._listeners) return;
+        let listeners: IObjectOf<[Listener, any][]>;
+        if (!(listeners = this._listeners)) return false;
         e.target === undefined && (e.target = this);
-        inotify_dispatch(this._listeners[<string>e.id], e);
-        inotify_dispatch(this._listeners[EVENT_ALL], e);
+        inotify_dispatch(listeners[<string>e.id], e);
+        inotify_dispatch(listeners[EVENT_ALL], e);
     },
 
     __listener(listeners: [Listener, any][], f: Listener, scope: any) {

diff --git a/packages/api/test/mixins.ts b/packages/api/test/mixins.ts
@@ -26,14 +26,18 @@ describe("mixins", () => {
         const foo = new Foo();
         const l = (e: Event) => (res[e.id] = e.value);
         const lall = (e: Event) => (res[EVENT_ALL] = e.value);
-        assert.doesNotThrow(() => foo.addListener("x", l));
-        assert.doesNotThrow(() => foo.addListener(EVENT_ALL, lall));
+        assert.ok(foo.addListener("x", l));
+        assert.ok(foo.addListener(EVENT_ALL, lall));
         foo.notify({ id: "x", value: 1 });
         assert.deepEqual(res, { x: 1, [EVENT_ALL]: 1 });
-        assert.doesNotThrow(() => foo.removeListener("x", l));
+        assert.ok(foo.removeListener("x", l));
+        assert.ok((<any>foo)._listeners.x === undefined);
+        assert.ok(!foo.removeListener("x", l));
         foo.notify({ id: "x", value: 2 });
         assert.deepEqual(res, { x: 1, [EVENT_ALL]: 2 });
-        assert.doesNotThrow(() => foo.removeListener(EVENT_ALL, lall));
+        assert.ok(foo.removeListener(EVENT_ALL, lall));
+        assert.ok((<any>foo)._listeners[EVENT_ALL] === undefined);
+        assert.deepEqual((<any>foo)._listeners, {});
         foo.notify({ id: "x", value: 3 });
         assert.deepEqual(res, { x: 1, [EVENT_ALL]: 2 });
     });