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

feat(bindNodeCallback): remove resultSelector #3364

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
116 changes: 7 additions & 109 deletions spec/observables/bindNodeCallback-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,23 +60,6 @@ describe('bindNodeCallback', () => {
expect(results).to.deep.equal([42, 'done']);
});

it('should emit one value chosen by a selector', () => {
function callback(datum: number, cb: Function) {
cb(null, datum);
}
const boundCallback = bindNodeCallback(callback, (datum: any) => datum);
const results: Array<number | string> = [];

boundCallback(42)
.subscribe(x => {
results.push(x);
}, null, () => {
results.push('done');
});

expect(results).to.deep.equal([42, 'done']);
});

it('should raise error from callback', () => {
const error = new Error();

Expand All @@ -99,24 +82,6 @@ describe('bindNodeCallback', () => {
expect(results).to.deep.equal([error]);
});

it('should emit an error when the selector throws', () => {
function callback(cb: Function) {
cb(null, 42);
}

const expected = new Error('Yikes!');
const boundCallback = bindNodeCallback(callback, (err: any) => { throw expected; });

boundCallback()
.subscribe(() => {
throw new Error('should not next');
}, (err: any) => {
expect(err).to.equal(expected);
}, () => {
throw new Error('should not complete');
});
});

it('should not emit, throw or complete if immediately unsubscribed', (done: MochaDone) => {
const nextSpy = sinon.spy();
const throwSpy = sinon.spy();
Expand Down Expand Up @@ -149,7 +114,7 @@ describe('bindNodeCallback', () => {
cb(null);
}

const boundCallback = bindNodeCallback(callback, null, rxTestScheduler);
const boundCallback = bindNodeCallback(callback, rxTestScheduler);
const results: Array<number | string> = [];

boundCallback()
Expand All @@ -168,7 +133,7 @@ describe('bindNodeCallback', () => {
function callback(datum: number, cb: Function) {
cb(null, datum);
}
const boundCallback = bindNodeCallback<number>(callback, null, rxTestScheduler);
const boundCallback = bindNodeCallback<number>(callback, rxTestScheduler);
const results: Array<number | string> = [];

boundCallback(42)
Expand All @@ -187,7 +152,7 @@ describe('bindNodeCallback', () => {
function callback(this: { datum: number }, cb: Function) {
cb(null, this.datum);
}
const boundCallback = bindNodeCallback(callback, null, rxTestScheduler);
const boundCallback = bindNodeCallback(callback, rxTestScheduler);
const results: Array<number | string> = [];

boundCallback.call({datum: 42})
Expand All @@ -207,7 +172,7 @@ describe('bindNodeCallback', () => {
function callback(datum: number, cb: Function) {
throw expected;
}
const boundCallback = bindNodeCallback(callback, null, rxTestScheduler);
const boundCallback = bindNodeCallback(callback, rxTestScheduler);

boundCallback(42)
.subscribe(x => {
Expand All @@ -228,7 +193,7 @@ describe('bindNodeCallback', () => {
cb(error);
}

const boundCallback = bindNodeCallback(callback, null, rxTestScheduler);
const boundCallback = bindNodeCallback(callback, rxTestScheduler);
const results: Array<number | string> = [];

boundCallback()
Expand All @@ -244,57 +209,13 @@ describe('bindNodeCallback', () => {

expect(results).to.deep.equal([error]);
});

it('should error if selector throws', () => {
const expected = new Error('what? a selector? I don\'t think so');
function callback(datum: number, cb: Function) {
cb(null, datum);
}
function selector() {
throw expected;
}
const boundCallback = bindNodeCallback(callback, selector, rxTestScheduler);

boundCallback(42)
.subscribe((x: any) => {
throw new Error('should not next');
}, (err: any) => {
expect(err).to.equal(expected);
}, () => {
throw new Error('should not complete');
});

rxTestScheduler.flush();
});

it('should use a selector', () => {
function callback(datum: number, cb: Function) {
cb(null, datum);
}
function selector(x: number) {
return x + '!!!';
}
const boundCallback = bindNodeCallback(callback, selector, rxTestScheduler);
const results: Array<number | string> = [];

boundCallback(42)
.subscribe((x: string) => {
results.push(x);
}, null, () => {
results.push('done');
});

rxTestScheduler.flush();

expect(results).to.deep.equal(['42!!!', 'done']);
});
});

it('should pass multiple inner arguments as an array', () => {
function callback(datum: number, cb: Function) {
cb(null, datum, 1, 2, 3);
}
const boundCallback = bindNodeCallback<number[]>(callback, null, rxTestScheduler);
const boundCallback = bindNodeCallback<number[]>(callback, rxTestScheduler);
const results: Array<number[] | string> = [];

boundCallback(42)
Expand All @@ -309,36 +230,13 @@ describe('bindNodeCallback', () => {
expect(results).to.deep.equal([[42, 1, 2, 3], 'done']);
});

it('should pass multiple inner arguments to the selector if there is one', () => {
function callback(datum: number, cb: Function) {
cb(null, datum, 1, 2, 3);
}
function selector(a: number, b: number, c: number, d: number) {
expect([a, b, c, d]).to.deep.equal([42, 1, 2, 3]);
return a + b + c + d;
}
const boundCallback = bindNodeCallback(callback, selector, rxTestScheduler);
const results: Array<number | string> = [];

boundCallback(42)
.subscribe(x => {
results.push(x);
}, null, () => {
results.push('done');
});

rxTestScheduler.flush();

expect(results).to.deep.equal([48, 'done']);
});

it('should cache value for next subscription and not call callbackFunc again', () => {
let calls = 0;
function callback(datum: number, cb: Function) {
calls++;
cb(null, datum);
}
const boundCallback = bindNodeCallback<number>(callback, null, rxTestScheduler);
const boundCallback = bindNodeCallback<number>(callback, rxTestScheduler);
const results1: Array<number | string> = [];
const results2: Array<number | string> = [];

Expand Down
68 changes: 11 additions & 57 deletions src/internal/observable/bindNodeCallback.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,14 @@ import { Subscriber } from '../Subscriber';
import { Action } from '../scheduler/Action';

/* tslint:disable:max-line-length */
export function bindNodeCallback<R>(callbackFunc: (callback: (err: any, result: R) => any) => any, selector?: void, scheduler?: IScheduler): () => Observable<R>;
export function bindNodeCallback<T, R>(callbackFunc: (v1: T, callback: (err: any, result: R) => any) => any, selector?: void, scheduler?: IScheduler): (v1: T) => Observable<R>;
export function bindNodeCallback<T, T2, R>(callbackFunc: (v1: T, v2: T2, callback: (err: any, result: R) => any) => any, selector?: void, scheduler?: IScheduler): (v1: T, v2: T2) => Observable<R>;
export function bindNodeCallback<T, T2, T3, R>(callbackFunc: (v1: T, v2: T2, v3: T3, callback: (err: any, result: R) => any) => any, selector?: void, scheduler?: IScheduler): (v1: T, v2: T2, v3: T3) => Observable<R>;
export function bindNodeCallback<T, T2, T3, T4, R>(callbackFunc: (v1: T, v2: T2, v3: T3, v4: T4, callback: (err: any, result: R) => any) => any, selector?: void, scheduler?: IScheduler): (v1: T, v2: T2, v3: T3, v4: T4) => Observable<R>;
export function bindNodeCallback<T, T2, T3, T4, T5, R>(callbackFunc: (v1: T, v2: T2, v3: T3, v4: T4, v5: T5, callback: (err: any, result: R) => any) => any, selector?: void, scheduler?: IScheduler): (v1: T, v2: T2, v3: T3, v4: T4, v5: T5) => Observable<R>;
export function bindNodeCallback<T, T2, T3, T4, T5, T6, R>(callbackFunc: (v1: T, v2: T2, v3: T3, v4: T4, v5: T5, v6: T6, callback: (err: any, result: R) => any) => any, selector?: void, scheduler?: IScheduler): (v1: T, v2: T2, v3: T3, v4: T4, v5: T5, v6: T6) => Observable<R>;
export function bindNodeCallback<T>(callbackFunc: Function, selector?: void, scheduler?: IScheduler): (...args: any[]) => Observable<T>;
export function bindNodeCallback<T>(callbackFunc: Function, selector?: (...args: any[]) => T, scheduler?: IScheduler): (...args: any[]) => Observable<T>;
export function bindNodeCallback<R>(callbackFunc: (callback: (err: any, result: R) => any) => any, scheduler?: IScheduler): () => Observable<R>;
export function bindNodeCallback<T, R>(callbackFunc: (v1: T, callback: (err: any, result: R) => any) => any, scheduler?: IScheduler): (v1: T) => Observable<R>;
export function bindNodeCallback<T, T2, R>(callbackFunc: (v1: T, v2: T2, callback: (err: any, result: R) => any) => any, scheduler?: IScheduler): (v1: T, v2: T2) => Observable<R>;
export function bindNodeCallback<T, T2, T3, R>(callbackFunc: (v1: T, v2: T2, v3: T3, callback: (err: any, result: R) => any) => any, scheduler?: IScheduler): (v1: T, v2: T2, v3: T3) => Observable<R>;
export function bindNodeCallback<T, T2, T3, T4, R>(callbackFunc: (v1: T, v2: T2, v3: T3, v4: T4, callback: (err: any, result: R) => any) => any, scheduler?: IScheduler): (v1: T, v2: T2, v3: T3, v4: T4) => Observable<R>;
export function bindNodeCallback<T, T2, T3, T4, T5, R>(callbackFunc: (v1: T, v2: T2, v3: T3, v4: T4, v5: T5, callback: (err: any, result: R) => any) => any, scheduler?: IScheduler): (v1: T, v2: T2, v3: T3, v4: T4, v5: T5) => Observable<R>;
export function bindNodeCallback<T, T2, T3, T4, T5, T6, R>(callbackFunc: (v1: T, v2: T2, v3: T3, v4: T4, v5: T5, v6: T6, callback: (err: any, result: R) => any) => any, scheduler?: IScheduler): (v1: T, v2: T2, v3: T3, v4: T4, v5: T5, v6: T6) => Observable<R>;
export function bindNodeCallback<T>(callbackFunc: Function, scheduler?: IScheduler): (...args: any[]) => Observable<T>;
/* tslint:enable:max-line-length */

/**
Expand All @@ -39,11 +38,6 @@ export function bindNodeCallback<T>(callbackFunc: Function, selector?: (...args:
* second parameter. If there are more parameters (third and so on),
* Observable will emit an array with all arguments, except first error argument.
*
* Optionally `bindNodeCallback` accepts selector function, which allows you to
* make resulting Observable emit value computed by selector, instead of regular
* callback arguments. It works similarly to {@link bindCallback} selector, but
* Node.js-style error argument will never be passed to that function.
*
* Note that `func` will not be called at the same time output function is,
* but rather whenever resulting Observable is subscribed. By default call to
* `func` will happen synchronously after subscription, but that can be changed
Expand Down Expand Up @@ -77,7 +71,7 @@ export function bindNodeCallback<T>(callbackFunc: Function, selector?: (...args:
* switch to {@link bindCallback} instead.
*
* Note that even if error parameter is technically present in callback, but its value
* is falsy, it still won't appear in array emitted by Observable or in selector function.
* is falsy, it still won't appear in array emitted by Observable.
*
*
* @example <caption>Read a file from the filesystem and get the data as an Observable</caption>
Expand All @@ -99,20 +93,6 @@ export function bindNodeCallback<T>(callbackFunc: Function, selector?: (...args:
* console.log(value); // [5, "some string"]
* });
*
*
* @example <caption>Use with selector function</caption>
* someFunction((err, a, b) => {
* console.log(err); // undefined
* console.log(a); // "abc"
* console.log(b); // "DEF"
* });
* var boundSomeFunction = bindNodeCallback(someFunction, (a, b) => a + b);
* boundSomeFunction()
* .subscribe(value => {
* console.log(value); // "abcDEF"
* });
*
*
* @example <caption>Use on function calling callback in regular style</caption>
* someFunction(a => {
* console.log(a); // 5
Expand All @@ -130,8 +110,6 @@ export function bindNodeCallback<T>(callbackFunc: Function, selector?: (...args:
* @see {@link fromPromise}
*
* @param {function} func Function with a Node.js-style callback as the last parameter.
* @param {function} [selector] A function which takes the arguments from the
* callback and maps those to a value to emit on the output Observable.
* @param {Scheduler} [scheduler] The scheduler on which to schedule the
* callbacks.
* @return {function(...params: *): Observable} A function which returns the
Expand All @@ -140,15 +118,13 @@ export function bindNodeCallback<T>(callbackFunc: Function, selector?: (...args:
* @name bindNodeCallback
*/
export function bindNodeCallback<T>(callbackFunc: Function,
selector: Function | void = undefined,
scheduler?: IScheduler): (...args: any[]) => Observable<T> {
return function(this: any, ...args: any[]): Observable<T> {
const params: ParamsState<T> = {
subject: undefined,
args,
callbackFunc,
scheduler,
selector,
context: this,
};
return new Observable<T>(subscriber => {
Expand All @@ -165,19 +141,7 @@ export function bindNodeCallback<T>(callbackFunc: Function,
return;
}

let result: T;
if (selector) {
try {
result = selector(...innerArgs);
} catch (err) {
subject.error(err);
return;
}
} else {
result = innerArgs.length <= 1 ? innerArgs[0] : innerArgs;
}

subject.next(result);
subject.next(innerArgs.length <= 1 ? innerArgs[0] : innerArgs);
subject.complete();
};

Expand Down Expand Up @@ -206,13 +170,12 @@ interface ParamsState<T> {
args: any[];
scheduler: IScheduler;
subject: AsyncSubject<T>;
selector: Function | void;
context: any;
}

function dispatch<T>(this: Action<DispatchState<T>>, state: DispatchState<T>) {
const { params, subscriber, context } = state;
const { callbackFunc, args, scheduler, selector } = params;
const { callbackFunc, args, scheduler } = params;
let subject = params.subject;

if (!subject) {
Expand All @@ -222,15 +185,6 @@ function dispatch<T>(this: Action<DispatchState<T>>, state: DispatchState<T>) {
const err = innerArgs.shift();
if (err) {
this.add(scheduler.schedule<DispatchErrorArg<T>>(dispatchError, 0, { err, subject }));
} else if (selector) {
let result: T;
try {
result = selector(...innerArgs);
} catch (err) {
this.add(scheduler.schedule<DispatchErrorArg<T>>(dispatchError, 0, { err, subject }));
return;
}
this.add(scheduler.schedule<DispatchNextArg<T>>(dispatchNext, 0, { value: result, subject }));
} else {
const value = innerArgs.length <= 1 ? innerArgs[0] : innerArgs;
this.add(scheduler.schedule<DispatchNextArg<T>>(dispatchNext, 0, { value, subject }));
Expand Down