fix(typings): set map function parameter for Observable.from as optional #1334
Conversation
without this change typescript returns TS2346 error when we use Observable.from with one parameter. I think that it will be nice if tests will be written in typescript because it will eliminate situations like this.
| @@ -20,7 +20,7 @@ export class FromObservable<T> extends Observable<T> { | |||
| super(null); | |||
| } | |||
|
|
|||
| static create<T>(ish: any, mapFnOrScheduler: Scheduler | ((x: any, y: number) => T), thisArg?: any, lastScheduler?: Scheduler): Observable<T> { | |||
| static create<T>(ish: any, mapFnOrScheduler?: Scheduler | ((x: any, y: number) => T), thisArg?: any, lastScheduler?: Scheduler): Observable<T> { | |||
There was a problem hiding this comment.
this makes sense for signature of generic from behavior. however in this case, does array-like observable need to check existence of mapFn in line 45? so far I remember arraylikeobservable's mapfn is required parameter. @justinwoo , is my understanding correct? or can we skip mapFn?
There was a problem hiding this comment.
oh, oops, mapFn should be optional. it's supposed to just use identity when not specified.
There was a problem hiding this comment.
Cool, thanks for clarification. So it seems PR's good :)
|
Merged with efa4dc3, thanks for catching this @mchmielarski (also @justinwoo for confirm this). For testing, we're currently trying to put some effort if it's feasible to migrate current test cases into typescript to have type validation tests as you mentioned. It's not there yet though, it's something we'd like to have. |
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
without this change typescript returns TS2346 error when we use Observable.from with one parameter.
I think that it will be nice if tests will be written in typescript because it will eliminate situations like this.