From bcd5b930f740b7534a5530e54f8a77fc6a4cc336 Mon Sep 17 00:00:00 2001 From: Mateusz Podlasin Date: Thu, 2 Feb 2017 17:33:20 +0100 Subject: [PATCH] fix(bindCallback): properly set context of input function Set context of input function to context of output function, so that it is possible to control context at call time and underlying observable is not available in input function --- spec/observables/bindCallback-spec.ts | 38 +++++++++++++++++++++++ src/observable/BoundCallbackObservable.ts | 30 +++++++++++++----- 2 files changed, 60 insertions(+), 8 deletions(-) diff --git a/spec/observables/bindCallback-spec.ts b/spec/observables/bindCallback-spec.ts index 23d0f567f8..cae44820a0 100644 --- a/spec/observables/bindCallback-spec.ts +++ b/spec/observables/bindCallback-spec.ts @@ -25,6 +25,24 @@ describe('Observable.bindCallback', () => { expect(results).to.deep.equal([42, 'done']); }); + it('should set callback function context to context of returned function', () => { + function callback(cb) { + cb(this.datum); + } + + const boundCallback = Observable.bindCallback(callback); + const results = []; + + boundCallback.apply({datum: 5}) + .subscribe( + (x: number) => results.push(x), + null, + () => results.push('done') + ); + + expect(results).to.deep.equal([5, 'done']); + }); + it('should emit one value chosen by a selector', () => { function callback(datum, cb) { cb(datum); @@ -105,6 +123,26 @@ describe('Observable.bindCallback', () => { expect(results).to.deep.equal([42, 'done']); }); + it('should set callback function context to context of returned function', () => { + function callback(cb) { + cb(this.datum); + } + + const boundCallback = Observable.bindCallback(callback, null, rxTestScheduler); + const results = []; + + boundCallback.apply({datum: 5}) + .subscribe( + (x: number) => results.push(x), + null, + () => results.push('done') + ); + + rxTestScheduler.flush(); + + expect(results).to.deep.equal([5, 'done']); + }); + it('should error if callback throws', () => { const expected = new Error('haha no callback for you'); function callback(datum, cb) { diff --git a/src/observable/BoundCallbackObservable.ts b/src/observable/BoundCallbackObservable.ts index 3f11a01867..bdcc87a178 100644 --- a/src/observable/BoundCallbackObservable.ts +++ b/src/observable/BoundCallbackObservable.ts @@ -43,17 +43,30 @@ export class BoundCallbackObservable extends Observable { * `bindCallback` is not an operator because its input and output are not * Observables. The input is a function `func` with some parameters, but the * last parameter must be a callback function that `func` calls when it is - * done. The output of `bindCallback` is a function that takes the same + * done. + * + * The output of `bindCallback` is a function that takes the same * parameters as `func`, except the last one (the callback). When the output * function is called with arguments, it will return an Observable where the * results will be delivered to. * + * If `func` depends on some context (`this` property), that context will be set + * to the same context that returned function has at call time. In particular, if `func` + * is usually called as method of some object, in order to preserve proper behaviour, + * it is recommended to set context of output function to that object as well, + * provided `func` is not already bound. + * * @example Convert jQuery's getJSON to an Observable API * // Suppose we have jQuery.getJSON('/my/url', callback) * var getJSONAsObservable = Rx.Observable.bindCallback(jQuery.getJSON); * var result = getJSONAsObservable('/my/url'); * result.subscribe(x => console.log(x), e => console.error(e)); * + * @example Use bindCallback on object method + * const boundMethod = Rx.Observable.bindCallback(someObject.methodWithCallback); + * boundMethod.call(someObject) // make sure methodWithCallback has access to someObject + * .subscribe(subscriber); + * * @see {@link bindNodeCallback} * @see {@link from} * @see {@link fromPromise} @@ -72,14 +85,15 @@ export class BoundCallbackObservable extends Observable { static create(func: Function, selector: Function | void = undefined, scheduler?: IScheduler): (...args: any[]) => Observable { - return (...args: any[]): Observable => { - return new BoundCallbackObservable(func, selector, args, scheduler); + return function(this: any, ...args: any[]): Observable { + return new BoundCallbackObservable(func, selector, args, this, scheduler); }; } constructor(private callbackFunc: Function, private selector: Function, private args: any[], + private context: any, private scheduler: IScheduler) { super(); } @@ -112,20 +126,20 @@ export class BoundCallbackObservable extends Observable { // use named function instance to avoid closure. (handler).source = this; - const result = tryCatch(callbackFunc).apply(this, args.concat(handler)); + const result = tryCatch(callbackFunc).apply(this.context, args.concat(handler)); if (result === errorObject) { subject.error(errorObject.e); } } return subject.subscribe(subscriber); } else { - return scheduler.schedule(BoundCallbackObservable.dispatch, 0, { source: this, subscriber }); + return scheduler.schedule(BoundCallbackObservable.dispatch, 0, { source: this, subscriber, context: this.context }); } } - static dispatch(state: { source: BoundCallbackObservable, subscriber: Subscriber }) { + static dispatch(state: { source: BoundCallbackObservable, subscriber: Subscriber, context: any }) { const self = (this); - const { source, subscriber } = state; + const { source, subscriber, context } = state; const { callbackFunc, args, scheduler } = source; let subject = source.subject; @@ -150,7 +164,7 @@ export class BoundCallbackObservable extends Observable { // use named function to pass values in without closure (handler).source = source; - const result = tryCatch(callbackFunc).apply(this, args.concat(handler)); + const result = tryCatch(callbackFunc).apply(context, args.concat(handler)); if (result === errorObject) { subject.error(errorObject.e); }