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

first operator does not accept default value only signature with null-value predicate #2447

Closed
kwonoj opened this issue Mar 8, 2017 · 7 comments
Labels
help wanted Issues we wouldn't mind assistance with. TS Issues and PRs related purely to TypeScript issues

Comments

@kwonoj
Copy link
Member

kwonoj commented Mar 8, 2017

RxJS version:
latest

Code to reproduce:
Rx.Observable.range(0, 10).first(null, null, true);

Expected behavior:

Actual behavior:

error TS2345: Argument of type 'null' is not assignable to parameter of type '(value: number, index: number, source: Observable) => boolean'.

Additional information:
This seems bit weird since default signature allows optional values for predicate / resultselector (

export function first<T, R>(this: Observable<T>, predicate?: (value: T, index: number, source: Observable<T>) => boolean,
) and test case exists (
expectObservable(e1.first(null, null, 'a')).toBe(expected);
) - not sure why it's being caused. Maybe signature ordering causes this.

@kwonoj kwonoj added TS Issues and PRs related purely to TypeScript issues help wanted Issues we wouldn't mind assistance with. labels Mar 8, 2017
@martinsik
Copy link
Contributor

I tested this with rxjs@5.2.0 and typescript@2.2.1 and it compiled without any problem. What Typescript version are you using?

@jsayol
Copy link
Contributor

jsayol commented Mar 11, 2017

null is indeed not assignable to that type, even if it's optional, when strictNullChecks is enabled. That's probably your case so you should call it like this:

Rx.Observable.range(0, 10).first(undefined, undefined, true);

@kwonoj
Copy link
Member Author

kwonoj commented Mar 11, 2017

Yes, I am aware this can be avoided. Still type definition need to be looked into.

@seveves
Copy link

seveves commented Mar 29, 2017

Does something like this look valid? I could create a PR then ...

export function first<T, R>(this: Observable<T>, predicate?: (value: T, index: number, source: Observable<T>) => boolean | undefined | null,
                            resultSelector?: ((value: T, index: number) => R) | void | undefined | null,
                            defaultValue?: R): Observable<T | R> {
  return this.lift(new FirstOperator(predicate, resultSelector, defaultValue, this));
}

@martinsik
Copy link
Contributor

I'm looking at this again after more than a year and I think this should be the expected behavior. Even when the method is defined as optional it shouldn't accept null in my opinion. If I want to skip any of the parameters I should use undefined.

@benlesh
Copy link
Member

benlesh commented Sep 2, 2020

This is no longer an issue.

@benlesh benlesh closed this as completed Sep 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Issues we wouldn't mind assistance with. TS Issues and PRs related purely to TypeScript issues
Projects
None yet
Development

No branches or pull requests

5 participants