refactor(ErrorObservable): clarify an error type E for ErrorObservable.

[tetsuharuohzeki]

Description:

• BREAKING CHANGE - ErrorObservable.create() (Obserbable.throw())
  takes a new type parameter E that express a error type instead on old
  one's T.
• Their functions return ErrorObservable<E>.
  • It is still the drived type of Observable<any>, it's not changed from
    old one.
  • But it contains the error typed as E now.

[coveralls]

Coverage remained the same at 97.217% when pulling 7ea0a2c on saneyuki:unused-type-param-ErrorObservable into 260d335 on ReactiveX:master.

[tetsuharuohzeki]

updated the commit message.

[coveralls]

Coverage remained the same at 97.217% when pulling 3a5770e on saneyuki:unused-type-param-ErrorObservable into 260d335 on ReactiveX:master.

[coveralls]

Coverage remained the same at 97.217% when pulling 62d0364 on saneyuki:unused-type-param-ErrorObservable into 0271fab on ReactiveX:master.

[mattpodwysocki]

@saneyuki wouldn't this break type inference such as if in our selector, we're depending on Observable<T> and we had conditional logic in our selector?

function selector(x: number) {
  if (x % 2 === 0) {
    return Observable.throw<number>('nope nope nope');
  }

  return Observable.of<number>(1);
}

[benlesh]

I think @mattpodwysocki is right here. @david-driscoll or @kwonoj can you confirm?

[tetsuharuohzeki]

@mattpodwysocki @Blesh

• ErrorObservable is defined as class ErrorObservable extends Observable<any> {} , so by your example, the return type of selector() is Observable<number> (and we need not specify type param T to Observable.throw()).
• At all, is it a right way that ErrorObservable is derived from Observable<any> so that make it more generic? Should we change from ErrorObservable to ErrorObservable<T>>

[coveralls]

Coverage remained the same at 97.217% when pulling d188b9b on saneyuki:unused-type-param-ErrorObservable into 89612b2 on ReactiveX:master.

[coveralls]

Coverage remained the same at 97.217% when pulling d188b9b on saneyuki:unused-type-param-ErrorObservable into 89612b2 on ReactiveX:master.

[kwonoj]

So this seems bit tricky.

First, I think generic of error observable itself is redundant, cause it does not being used to infer type of error. For example, in current interface Observable.throw(1) is inferred to
Observable<T>.throw: <{}>(error: any, scheduler?: Scheduler) => ErrorObservable

Since ErrorObservable itself does not have generic type and force set base observable type to any.

But also it is questionable if it's right to give type to ErrorObservable itself, since all of error being emit in Observable is currently any, do not have specific types so allowed to error any value as needed. It may considerable option to adapt generic type to ErrorObservable to allow type inferred (cases like selector need to return single type T).

Waiting @david-driscoll 's input as well.

[benlesh]

I think this should be: Observable.throw<T>(error: T) returning ErrorObservable<T>. Since we know the type that we're passing to it, it can be inferred.

@saneyuki, if you want to make that change in this PR that would be cool. Otherwise, I don't think we'll merge this PR, but rather make that change in another PR. :PR:

[tetsuharuohzeki]

@Blesh

I think this should be: Observable.throw(error: T) returning ErrorObservable. Since we know the type that we're passing to it, it can be inferred.

Okay. I'll change to it.

BTW, I think it would be better to use type parameter E (Observable.throw<E>, ErrorObservable<E>) instead of T, to express the generics type which we intend to accept an error type to. How do you think about it?

[tetsuharuohzeki]

@Blesh

I have updated this pull request. Your comment is like this, isn't it?

[coveralls]

Coverage remained the same at 97.593% when pulling b5fb259 on saneyuki:unused-type-param-ErrorObservable into 39214f2 on ReactiveX:master.

[coveralls]

Coverage remained the same at 97.217% when pulling 62d0364 on saneyuki:unused-type-param-ErrorObservable into 0271fab on ReactiveX:master.

[benlesh]

[lock]

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.