Skip to content

Commit

Permalink
feat(bindNodeCallback): remove resultSelector
Browse files Browse the repository at this point in the history
- Removes the optional result selector to simplify the API

BREAKING CHANGE: resultSelector removed, use `map` instead: `bindNodeCallback(fn1, fn2)()` becomes `bindNodeCallback(fn1)().pipe(map(fn2))`
  • Loading branch information
benlesh committed Mar 2, 2018
1 parent 0a3a04a commit 26e6e5c
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 166 deletions.
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

0 comments on commit 26e6e5c

Please sign in to comment.