Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(bindCallback): properly set context of input function #2319

Merged
merged 1 commit into from
Feb 2, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions spec/observables/bindCallback-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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) {
Expand Down
30 changes: 22 additions & 8 deletions src/observable/BoundCallbackObservable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,17 +43,30 @@ export class BoundCallbackObservable<T> extends Observable<T> {
* `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 <caption>Convert jQuery's getJSON to an Observable API</caption>
* // 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 <caption>Use bindCallback on object method</caption>
* 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}
Expand All @@ -72,14 +85,15 @@ export class BoundCallbackObservable<T> extends Observable<T> {
static create<T>(func: Function,
selector: Function | void = undefined,
scheduler?: IScheduler): (...args: any[]) => Observable<T> {
return (...args: any[]): Observable<T> => {
return new BoundCallbackObservable<T>(func, <any>selector, args, scheduler);
return function(this: any, ...args: any[]): Observable<T> {
return new BoundCallbackObservable<T>(func, <any>selector, args, this, scheduler);
};
}

constructor(private callbackFunc: Function,
private selector: Function,
private args: any[],
private context: any,
private scheduler: IScheduler) {
super();
}
Expand Down Expand Up @@ -112,20 +126,20 @@ export class BoundCallbackObservable<T> extends Observable<T> {
// use named function instance to avoid closure.
(<any>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<T>(state: { source: BoundCallbackObservable<T>, subscriber: Subscriber<T> }) {
static dispatch<T>(state: { source: BoundCallbackObservable<T>, subscriber: Subscriber<T>, context: any }) {
const self = (<Subscription><any>this);
const { source, subscriber } = state;
const { source, subscriber, context } = state;
const { callbackFunc, args, scheduler } = source;
let subject = source.subject;

Expand All @@ -150,7 +164,7 @@ export class BoundCallbackObservable<T> extends Observable<T> {
// use named function to pass values in without closure
(<any>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);
}
Expand Down