From 0fd99f3fa2db66cbd7bdd4bda297335f0dff1817 Mon Sep 17 00:00:00 2001 From: Christian Johns Date: Sat, 9 Jul 2016 09:31:40 -0700 Subject: [PATCH 1/4] Support EventEmitters in fromEvent extra Add support for stream generation from EventEmitters. fromEvent will accept as a first argument either a DOMElement or an EventEmitter. When providing fromEvent with an EventEmitter, the third argument is always ignored. Internally, include an additional Producer type within fromEvent to support type checking. Related to staltz/xstream#65. --- src/extra/fromEvent.ts | 65 ++++++++++++++++++++++++----- tests/extra/fromEvent.ts | 88 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 141 insertions(+), 12 deletions(-) diff --git a/src/extra/fromEvent.ts b/src/extra/fromEvent.ts index 346292e..2478d68 100644 --- a/src/extra/fromEvent.ts +++ b/src/extra/fromEvent.ts @@ -1,4 +1,6 @@ -import {Stream, InternalProducer, InternalListener} from '../core'; +/// +import { EventEmitter } from 'events'; +import { Stream, InternalProducer, InternalListener } from '../core'; export class DOMEventProducer implements InternalProducer { public type = 'fromEvent'; @@ -22,6 +24,25 @@ export class DOMEventProducer implements InternalProducer { } } +export class NodeEventProducer implements InternalProducer { + public type = 'fromEvent'; + private listener: Function; + + constructor(private node: EventEmitter, private eventName: string) { } + + _start(out: InternalListener) { + this.listener = (e: any) => out._n(e); + const {node, eventName} = this; + node.addListener(eventName, this.listener); + } + + _stop() { + const {node, eventName, listener} = this; + node.removeListener(eventName, listener); + this.listener = null; + } +} + /** * Creates a stream based on DOM events of type `eventType` from the target * node. @@ -29,11 +50,11 @@ export class DOMEventProducer implements InternalProducer { * Marble diagram: * * ```text - * fromEvent(node, eventType) + * fromEvent( element, eventType ) * ---ev--ev----ev--------------- * ``` * - * Example: + * Examples: * * ```js * import fromEvent from 'xstream/extra/fromEvent' @@ -45,7 +66,7 @@ export class DOMEventProducer implements InternalProducer { * next: i => console.log(i), * error: err => console.error(err), * complete: () => console.log('completed') - * }) + * }); * ``` * * ```text @@ -54,15 +75,37 @@ export class DOMEventProducer implements InternalProducer { * > 'Button clicked!' * ``` * - * @param {EventTarget} node The element we want to listen to. - * @param {string} eventType The type of events we want to listen to. - * @param {boolean} useCapture An optional boolean that indicates that events of + * ```js + * import fromEvent from 'xstream/extra/fromEvent' + * import {EventEmitter} from 'events'; + * + * const MyEmitter = new EventEmitter(); + * const stream = fromEvent( MyEmitter, 'foo' ); + * + * stream.addListener({ + * next: i => console.log(i), + * error: err => console.error(err), + * complete: () => console.log('completed') + * }); + * + * MyEmitter.emit( 'foo', 'bar' ); + * ``` + * + * ```text + * > 'bar' + * ``` + * + * @param {EventTarget|EventEmitter} element The element upon which to listen. + * @param {string} eventName The name of the event for which to listen. + * @param {boolean?} useCapture An optional boolean that indicates that events of * this type will be dispatched to the registered listener before being * dispatched to any EventTarget beneath it in the DOM tree. Defaults to false. * @return {Stream} */ -export default function fromEvent(node: EventTarget, - eventType: string, - useCapture: boolean = false): Stream { - return new Stream(new DOMEventProducer(node, eventType, useCapture)); +export default function fromEvent( element: EventTarget | EventEmitter, eventName: string, useCapture: boolean = false): Stream { + if ( element instanceof EventEmitter ) { + return new Stream(new NodeEventProducer( element, eventName )); + } else { + return new Stream(new DOMEventProducer( element, eventName, useCapture )); + } } diff --git a/tests/extra/fromEvent.ts b/tests/extra/fromEvent.ts index 7f57f40..e731ecd 100644 --- a/tests/extra/fromEvent.ts +++ b/tests/extra/fromEvent.ts @@ -1,5 +1,6 @@ /// /// +import {EventEmitter} from 'events'; import xs from '../../src/index'; import fromEvent from '../../src/extra/fromEvent'; import * as assert from 'assert'; @@ -39,7 +40,39 @@ class FakeEventTarget implements EventTarget { } }; -describe('fromEvent (extra)', () => { +class FakeEventEmitter extends EventEmitter { + public handler: Function; + public event: string; + public removedEvent: string; + + constructor() { + super(); + } + + emit( eventName: string, ...args: any[] ): any { + if (typeof this.handler !== 'function') { + return; + } + this.handler.apply(void 0, args ); + return true; + } + + addListener(e: string, handler: Function): FakeEventEmitter { + this.event = e; + this.handler = handler; + return this; + } + + removeListener(e: string, handler: Function): FakeEventEmitter { + this.removedEvent = e; + + this.handler = this.event = void 0; + return this; + } + +}; + +describe('fromEvent (extra) - DOMEvent', () => { it('should call addEventListener with expected parameters', () => { const target = new FakeEventTarget(); const stream = fromEvent(target, 'test', true); @@ -103,3 +136,56 @@ describe('fromEvent (extra)', () => { target.emit(2); }); }); + +describe('fromEvent (extra) - EventEmitter', () => { + it('should call addListener with expected parameters', () => { + const target = new FakeEventEmitter(); + const stream = fromEvent(target, 'test'); + + stream.addListener({next: noop, error: noop, complete: noop}); + + assert.strictEqual('test', target.event); + }); + + it('should propagate events', (done) => { + const target = new FakeEventEmitter(); + const stream = fromEvent(target, 'test').take(3); + + let expected = [1, 2, 3]; + + stream.addListener({ + next: (x: any) => { + assert.strictEqual(x, expected.shift()); + }, + error: (err: any) => done(err), + complete: () => { + assert.strictEqual(expected.length, 0); + done(); + } + }); + + target.emit( 'test', 1 ); + target.emit( 'test', 2 ); + target.emit( 'test', 3 ); + target.emit( 'test', 4 ); + }); + + it('should call removeListener with expected parameters', (done) => { + const target = new FakeEventEmitter(); + const stream = fromEvent(target, 'test'); + + stream.take(1).addListener({ + next: (x) => {}, + error: (err: any) => done(err), + complete() { + setTimeout(() => { + assert.strictEqual('test', target.removedEvent); + done(); + }, 5); + } + }); + + target.emit( 'test', 1 ); + target.emit( 'test', 2 ); + }); +}); From 575408d855b672b2f24193f49eba8d13c45008f7 Mon Sep 17 00:00:00 2001 From: Christian Johns Date: Sat, 9 Jul 2016 09:42:37 -0700 Subject: [PATCH 2/4] Revert indent change Revert indentation change to remain consistent with project style. --- src/extra/fromEvent.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/extra/fromEvent.ts b/src/extra/fromEvent.ts index 2478d68..791a828 100644 --- a/src/extra/fromEvent.ts +++ b/src/extra/fromEvent.ts @@ -102,7 +102,9 @@ export class NodeEventProducer implements InternalProducer { * dispatched to any EventTarget beneath it in the DOM tree. Defaults to false. * @return {Stream} */ -export default function fromEvent( element: EventTarget | EventEmitter, eventName: string, useCapture: boolean = false): Stream { +export default function fromEvent( element: EventTarget | EventEmitter, + eventName: string, + useCapture: boolean = false): Stream { if ( element instanceof EventEmitter ) { return new Stream(new NodeEventProducer( element, eventName )); } else { From 7f1147912f68f9b2afb9708f30ac6730261fd9c7 Mon Sep 17 00:00:00 2001 From: Christian Johns Date: Sun, 10 Jul 2016 21:28:31 -0700 Subject: [PATCH 3/4] Refactor guards; Conform test and source style Introduce style changes to enforce project source consistency. Adjust type checking within the fromEvent function. Specifically, duck type the first argument rather than rely on `instanceof` or `typeof` guards when deciding which producer type to instantiate. * Remove whitespace to align with project style requirements. * Remove destructuring assignment in producer class methods. * Update documentation to include both uses of the extra. * Remove whitespaces and semicolons from example markdown. * Refactor producer instantiation guards to implement duck-typing rather than `instanceof`/`typeof` checking. * Remove unused import from tests * Reorder assertion equality arguments. * Rename parameter in fromEvent documentation (`eventType`>`eventName`) --- src/extra/fromEvent.ts | 51 ++++++++++++++++++++-------------------- tests/extra/fromEvent.ts | 17 +++++++------- 2 files changed, 34 insertions(+), 34 deletions(-) diff --git a/src/extra/fromEvent.ts b/src/extra/fromEvent.ts index 791a828..6534ab5 100644 --- a/src/extra/fromEvent.ts +++ b/src/extra/fromEvent.ts @@ -1,6 +1,6 @@ /// -import { EventEmitter } from 'events'; -import { Stream, InternalProducer, InternalListener } from '../core'; +import {EventEmitter} from 'events'; +import {Stream, InternalProducer, InternalListener} from '../core'; export class DOMEventProducer implements InternalProducer { public type = 'fromEvent'; @@ -13,13 +13,11 @@ export class DOMEventProducer implements InternalProducer { _start(out: InternalListener) { this.listener = (e) => out._n(e); - const {node, eventType, useCapture} = this; - node.addEventListener(eventType, this.listener, useCapture); + this.node.addEventListener(this.eventType, this.listener, this.useCapture); } _stop() { - const {node, eventType, listener, useCapture} = this; - node.removeEventListener(eventType, listener, useCapture); + this.node.removeEventListener(this.eventType, this.listener, this.useCapture); this.listener = null; } } @@ -32,25 +30,28 @@ export class NodeEventProducer implements InternalProducer { _start(out: InternalListener) { this.listener = (e: any) => out._n(e); - const {node, eventName} = this; - node.addListener(eventName, this.listener); + this.node.addListener(this.eventName, this.listener); } _stop() { - const {node, eventName, listener} = this; - node.removeListener(eventName, listener); + this.node.removeListener(this.eventName, this.listener); this.listener = null; } } +function isEmitter(element: any): boolean { + return element.addListener; +} + /** - * Creates a stream based on DOM events of type `eventType` from the target - * node. + * Creates a stream based on either: + * - DOM events with the name `eventName` from a provided target node + * - Events with the name `eventName` from a provided NodeJS EventEmitter * * Marble diagram: * * ```text - * fromEvent( element, eventType ) + * fromEvent(element, eventName) * ---ev--ev----ev--------------- * ``` * @@ -66,7 +67,7 @@ export class NodeEventProducer implements InternalProducer { * next: i => console.log(i), * error: err => console.error(err), * complete: () => console.log('completed') - * }); + * }) * ``` * * ```text @@ -77,18 +78,18 @@ export class NodeEventProducer implements InternalProducer { * * ```js * import fromEvent from 'xstream/extra/fromEvent' - * import {EventEmitter} from 'events'; + * import {EventEmitter} from 'events' * - * const MyEmitter = new EventEmitter(); - * const stream = fromEvent( MyEmitter, 'foo' ); + * const MyEmitter = new EventEmitter() + * const stream = fromEvent(MyEmitter, 'foo') * * stream.addListener({ * next: i => console.log(i), * error: err => console.error(err), * complete: () => console.log('completed') - * }); + * }) * - * MyEmitter.emit( 'foo', 'bar' ); + * MyEmitter.emit('foo', 'bar') * ``` * * ```text @@ -102,12 +103,12 @@ export class NodeEventProducer implements InternalProducer { * dispatched to any EventTarget beneath it in the DOM tree. Defaults to false. * @return {Stream} */ -export default function fromEvent( element: EventTarget | EventEmitter, - eventName: string, - useCapture: boolean = false): Stream { - if ( element instanceof EventEmitter ) { - return new Stream(new NodeEventProducer( element, eventName )); +export default function fromEvent(element: EventTarget | EventEmitter, + eventName: string, + useCapture: boolean = false): Stream { + if (isEmitter(element)) { + return new Stream(new NodeEventProducer( element, eventName)); } else { - return new Stream(new DOMEventProducer( element, eventName, useCapture )); + return new Stream(new DOMEventProducer( element, eventName, useCapture)); } } diff --git a/tests/extra/fromEvent.ts b/tests/extra/fromEvent.ts index e731ecd..e1625bf 100644 --- a/tests/extra/fromEvent.ts +++ b/tests/extra/fromEvent.ts @@ -1,7 +1,6 @@ /// /// import {EventEmitter} from 'events'; -import xs from '../../src/index'; import fromEvent from '../../src/extra/fromEvent'; import * as assert from 'assert'; function noop() {}; @@ -79,8 +78,8 @@ describe('fromEvent (extra) - DOMEvent', () => { stream.addListener({next: noop, error: noop, complete: noop}); - assert.strictEqual('test', target.event); - assert.strictEqual(true, target.capture); + assert.strictEqual(target.event, 'test'); + assert.strictEqual(target.capture, true); }); it('should call addEventListener with expected parameters', () => { @@ -89,8 +88,8 @@ describe('fromEvent (extra) - DOMEvent', () => { stream.addListener({next: noop, error: noop, complete: noop}); - assert.strictEqual('test', target.event); - assert.strictEqual(false, target.capture); + assert.strictEqual(target.event, 'test'); + assert.strictEqual(target.capture, false); }); it('should propagate events', (done) => { @@ -125,8 +124,8 @@ describe('fromEvent (extra) - DOMEvent', () => { error: (err: any) => done(err), complete() { setTimeout(() => { - assert.strictEqual('test', target.removedEvent); - assert.strictEqual(true, target.removedCapture); + assert.strictEqual(target.removedEvent, 'test'); + assert.strictEqual(target.removedCapture, true); done(); }, 5); } @@ -144,7 +143,7 @@ describe('fromEvent (extra) - EventEmitter', () => { stream.addListener({next: noop, error: noop, complete: noop}); - assert.strictEqual('test', target.event); + assert.strictEqual(target.event, 'test'); }); it('should propagate events', (done) => { @@ -179,7 +178,7 @@ describe('fromEvent (extra) - EventEmitter', () => { error: (err: any) => done(err), complete() { setTimeout(() => { - assert.strictEqual('test', target.removedEvent); + assert.strictEqual(target.removedEvent, 'test'); done(); }, 5); } From c8eea7059177773d10aa3db46f9abccea76ff0d7 Mon Sep 17 00:00:00 2001 From: Christian Johns Date: Mon, 11 Jul 2016 08:27:32 -0700 Subject: [PATCH 4/4] Expand emitter guard Check for both `addListener` and `emit` methods on an element supplied to fromEvent. This ensures that the function will not attempt to invoke the `addListener` method on an xstream stream instance with incorrect arguments. Instead, this scenario will throw a TypeError: 'this.node.addEventListener is not a function' --- src/extra/fromEvent.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/extra/fromEvent.ts b/src/extra/fromEvent.ts index 6534ab5..a74c6a5 100644 --- a/src/extra/fromEvent.ts +++ b/src/extra/fromEvent.ts @@ -40,7 +40,7 @@ export class NodeEventProducer implements InternalProducer { } function isEmitter(element: any): boolean { - return element.addListener; + return element.emit && element.addListener; } /**