From b32a341700d72b7fcd6d22f377df4693dd574d85 Mon Sep 17 00:00:00 2001 From: Henry Heino <46334387+personalizedrefrigerator@users.noreply.github.com> Date: Thu, 28 Jul 2022 08:46:52 -0700 Subject: [PATCH] Chore: Migrate EventDispatcher to TypeScript, add tests (#6673) --- .eslintignore | 6 ++ .gitignore | 6 ++ packages/lib/DropboxApi.js | 2 +- packages/lib/EventDispatcher.js | 33 ------- packages/lib/EventDispatcher.test.ts | 137 +++++++++++++++++++++++++++ packages/lib/EventDispatcher.ts | 49 ++++++++++ 6 files changed, 199 insertions(+), 34 deletions(-) delete mode 100644 packages/lib/EventDispatcher.js create mode 100644 packages/lib/EventDispatcher.test.ts create mode 100644 packages/lib/EventDispatcher.ts diff --git a/.eslintignore b/.eslintignore index 7b8c90cb0..2b92f9ae0 100644 --- a/.eslintignore +++ b/.eslintignore @@ -1000,6 +1000,12 @@ packages/lib/ClipperServer.js.map packages/lib/CssUtils.d.ts packages/lib/CssUtils.js packages/lib/CssUtils.js.map +packages/lib/EventDispatcher.d.ts +packages/lib/EventDispatcher.js +packages/lib/EventDispatcher.js.map +packages/lib/EventDispatcher.test.d.ts +packages/lib/EventDispatcher.test.js +packages/lib/EventDispatcher.test.js.map packages/lib/HtmlToMd.d.ts packages/lib/HtmlToMd.js packages/lib/HtmlToMd.js.map diff --git a/.gitignore b/.gitignore index 277bb8fab..94c87402b 100644 --- a/.gitignore +++ b/.gitignore @@ -990,6 +990,12 @@ packages/lib/ClipperServer.js.map packages/lib/CssUtils.d.ts packages/lib/CssUtils.js packages/lib/CssUtils.js.map +packages/lib/EventDispatcher.d.ts +packages/lib/EventDispatcher.js +packages/lib/EventDispatcher.js.map +packages/lib/EventDispatcher.test.d.ts +packages/lib/EventDispatcher.test.js +packages/lib/EventDispatcher.test.js.map packages/lib/HtmlToMd.d.ts packages/lib/HtmlToMd.js packages/lib/HtmlToMd.js.map diff --git a/packages/lib/DropboxApi.js b/packages/lib/DropboxApi.js index 7ff89518a..80581b2e1 100644 --- a/packages/lib/DropboxApi.js +++ b/packages/lib/DropboxApi.js @@ -2,7 +2,7 @@ const Logger = require('./Logger').default; const shim = require('./shim').default; const JoplinError = require('./JoplinError').default; const time = require('./time').default; -const EventDispatcher = require('./EventDispatcher'); +const EventDispatcher = require('./EventDispatcher').default; class DropboxApi { constructor(options) { diff --git a/packages/lib/EventDispatcher.js b/packages/lib/EventDispatcher.js deleted file mode 100644 index 5fd795973..000000000 --- a/packages/lib/EventDispatcher.js +++ /dev/null @@ -1,33 +0,0 @@ -class EventDispatcher { - constructor() { - this.listeners_ = []; - } - - dispatch(eventName, event = null) { - if (!this.listeners_[eventName]) return; - - const ls = this.listeners_[eventName]; - for (let i = 0; i < ls.length; i++) { - ls[i](event); - } - } - - on(eventName, callback) { - if (!this.listeners_[eventName]) this.listeners_[eventName] = []; - this.listeners_[eventName].push(callback); - } - - off(eventName, callback) { - if (!this.listeners_[eventName]) return; - - const ls = this.listeners_[eventName]; - for (let i = 0; i < ls.length; i++) { - if (ls[i] === callback) { - ls.splice(i, 1); - return; - } - } - } -} - -module.exports = EventDispatcher; diff --git a/packages/lib/EventDispatcher.test.ts b/packages/lib/EventDispatcher.test.ts new file mode 100644 index 000000000..3e74f3ab2 --- /dev/null +++ b/packages/lib/EventDispatcher.test.ts @@ -0,0 +1,137 @@ +import EventDispatcher from './EventDispatcher'; + +enum TestKey { + FooEvent, + BarEvent, + BazEvent, +} + +describe('EventDispatcher', () => { + it('should trigger after adding a listener', () => { + const dispatcher = new EventDispatcher(); + let calledCount = 0; + dispatcher.on(TestKey.FooEvent, () => { + calledCount ++; + }); + + expect(calledCount).toBe(0); + dispatcher.dispatch(TestKey.FooEvent); + expect(calledCount).toBe(1); + }); + + it('should not trigger after removing a listener', () => { + const dispatcher = new EventDispatcher(); + let calledCount = 0; + const handle = dispatcher.on(TestKey.FooEvent, () => { + calledCount ++; + }); + + handle.remove(); + + expect(calledCount).toBe(0); + dispatcher.dispatch(TestKey.FooEvent); + expect(calledCount).toBe(0); + }); + + it('adding and removing listeners should not affect other listeners', () => { + const dispatcher = new EventDispatcher(); + + let fooCount = 0; + const fooListener = dispatcher.on(TestKey.FooEvent, () => { + fooCount ++; + }); + + let barCount = 0; + const barListener1 = dispatcher.on(TestKey.BarEvent, () => { + barCount ++; + }); + const barListener2 = dispatcher.on(TestKey.BarEvent, () => { + barCount += 3; + }); + const barListener3 = dispatcher.on(TestKey.BarEvent, () => { + barCount += 2; + }); + + dispatcher.dispatch(TestKey.BarEvent); + expect(barCount).toBe(6); + + dispatcher.dispatch(TestKey.FooEvent); + expect(barCount).toBe(6); + expect(fooCount).toBe(1); + + fooListener.remove(); + barListener2.remove(); + + // barListener2 shouldn't be fired + dispatcher.dispatch(TestKey.BarEvent); + expect(barCount).toBe(9); + + // The BazEvent shouldn't change fooCount or barCount + dispatcher.dispatch(TestKey.BazEvent); + expect(barCount).toBe(9); + expect(fooCount).toBe(1); + + // Removing a listener for the first time should return true (it removed the listener) + // and false all subsequent times + expect(barListener1.remove()).toBe(true); + expect(barListener1.remove()).toBe(false); + expect(barListener3.remove()).toBe(true); + }); + + it('should fire all un-removed listeners if removing a listener in a listener', () => { + const dispatcher = new EventDispatcher(); + + let count = 0; + const barListener = () => { + }; + const bazListener = () => { + count += 5; + }; + const fooListener = () => { + count ++; + dispatcher.off(TestKey.FooEvent, barListener); + }; + dispatcher.on(TestKey.FooEvent, barListener); + dispatcher.on(TestKey.FooEvent, fooListener); + dispatcher.on(TestKey.FooEvent, bazListener); + + // Removing a listener shouldn't cause other listeners to be skipped + dispatcher.dispatch(TestKey.FooEvent); + + expect(count).toBe(6); + }); + + it('should send correct data associated with events', () => { + const dispatcher = new EventDispatcher(); + + let lastResult = ''; + const resultListener = (result: string) => { + lastResult = result; + }; + + dispatcher.on(TestKey.BarEvent, resultListener); + + dispatcher.dispatch(TestKey.BazEvent, 'Testing...'); + expect(lastResult).toBe(''); + + dispatcher.dispatch(TestKey.BarEvent, 'Test.'); + dispatcher.off(TestKey.BarEvent, resultListener); + + dispatcher.dispatch(TestKey.BarEvent, 'Testing.'); + expect(lastResult).toBe('Test.'); + }); + + it('should work if imported using require(...).default', () => { + const Dispatcher = require('./EventDispatcher').default; + const dispatcher = new Dispatcher(); + + let pass = false; + dispatcher.on('Evnt', () => { + pass = true; + }); + + expect(pass).toBe(false); + dispatcher.dispatch('Evnt'); + expect(pass).toBe(true); + }); +}); diff --git a/packages/lib/EventDispatcher.ts b/packages/lib/EventDispatcher.ts new file mode 100644 index 000000000..22ce7b016 --- /dev/null +++ b/packages/lib/EventDispatcher.ts @@ -0,0 +1,49 @@ +type Listener = (data: Value)=> void; +type CallbackHandler = (data: EventType)=> void; + +// EventKeyType is used to distinguish events (e.g. a 'ClickEvent' vs a 'TouchEvent') +// while EventMessageType is the type of the data sent with an event (can be `void`) +export default class EventDispatcher { + // Partial marks all fields as optional. To initialize with an empty object, this is required. + // See https://stackoverflow.com/a/64526384 + private listeners: Partial>>>; + public constructor() { + this.listeners = {}; + } + + public dispatch(eventName: EventKeyType, event: EventMessageType = null) { + if (!this.listeners[eventName]) return; + + const ls = this.listeners[eventName]; + for (let i = 0; i < ls.length; i++) { + ls[i](event); + } + } + + public on(eventName: EventKeyType, callback: CallbackHandler) { + if (!this.listeners[eventName]) this.listeners[eventName] = []; + this.listeners[eventName].push(callback); + + return { + // Retuns false if the listener has already been removed, true otherwise. + remove: (): boolean => { + const originalListeners = this.listeners[eventName]; + this.off(eventName, callback); + + return originalListeners.length !== this.listeners[eventName].length; + }, + }; + } + + // Equivalent to calling .remove() on the object returned by .on + public off(eventName: EventKeyType, callback: CallbackHandler) { + if (!this.listeners[eventName]) return; + + // Replace the current list of listeners with a new, shortened list. + // This allows any iterators over this.listeners to continue iterating + // without skipping elements. + this.listeners[eventName] = this.listeners[eventName].filter( + otherCallback => otherCallback !== callback + ); + } +}