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

Return type of .ignoreElements() and .empty() should be Observable<never> #2640

Closed
felixfbecker opened this issue Jun 4, 2017 · 4 comments
Closed

Comments

@felixfbecker
Copy link
Contributor

RxJS version: 5.4.0

Code to reproduce:

/** Returns an Observable of files */
function fetchFiles(): Observable<string> {
  return Observable.of('foo', 'bar', 'baz')
}

const files = new Set<string>()

/** Returns an Observable that emits no items and completes when the task is done */
function updateFiles() {
  return fetchFiles()
    .do(file => {
      files.add(file)
    })
    .ignoreElements()
}

Expected behavior:
Inferred return type of updateFiles should be Observable<never>. The Observable never emits items, so the items don't have a type, similar to how a function that always throws has no return type - it never emits/returns.

Actual behavior:
Emitted return type is Observable<string>

@felixfbecker
Copy link
Contributor Author

felixfbecker commented Jun 4, 2017

Observable.empty() should too. Consider this example:

const obs = condition ? Observable.empty() : Observable.of(1, 2, 3)

obs becomes either an Observable that emits numbers, or an Observable that emits no items. If the return type of Observable.empty() was Observable<never>, TS would correctly infer this type as Observable<number> (since in a union with never, the never disappears). But because the return type is Observable<T>, and T cannot be inferred here, TS infers the type of obs as Observable<{}> (the empty interface, no mention of number).

@felixfbecker felixfbecker changed the title Return type of .ignoreElements() should be Observable<never> Return type of .ignoreElements() and .empty() should be Observable<never> Jun 5, 2017
@staltz
Copy link
Member

staltz commented Jun 14, 2017

Hi @felixfbecker.
Would a.concat(b) type check correctly in TypeScript if a: Observable<never> and b: Observable<number>? That's the only concern I have, otherwise I agree with you in doing this change.

@felixfbecker
Copy link
Contributor Author

Yep. As said, in a union with never, the never disappears:

declare let a: Observable<never>;
declare let b: Observable<number>;

const c = a.concat(b); // Observable<number>

jayphelps added a commit to jayphelps/rxjs that referenced this issue Jun 22, 2017
…() now return Observable<never>

fixes ReactiveX#2640

BREAKING CHANGE: Previously, `Observable.never()` `Observable.empty()`
and the `.ignoreElements()` operator all returned `Observable<T>` which
was incorrect since they actually never emit anything. Now they all
return `Observable<never>` (`never` was added in TS 2.0 as a special
type)
jayphelps added a commit to jayphelps/rxjs that referenced this issue Jun 22, 2017
…() now return Observable<never>

fixes ReactiveX#2640

BREAKING CHANGE: Previously, `Observable.never()` `Observable.empty()`
and the `.ignoreElements()` operator all returned `Observable<T>` which
was incorrect since they actually never emit anything. Now they all
return `Observable<never>` (`never` was added in TS 2.0 as a special
type)
jayphelps added a commit to jayphelps/rxjs that referenced this issue Jun 22, 2017
…() now return Observable<never>

fixes ReactiveX#2640

BREAKING CHANGE: Previously, `Observable.never()` `Observable.empty()`
and the `.ignoreElements()` operator all returned `Observable<T>` which
was incorrect since they actually never emit anything. Now they all
return `Observable<never>` (`never` was added in TS 2.0 as a special
type)
jayphelps added a commit to jayphelps/rxjs that referenced this issue Jun 22, 2017
…() now return Observable<never>

fixes ReactiveX#2640

BREAKING CHANGE: Previously, `Observable.never()` `Observable.empty()`
and the `.ignoreElements()` operator all returned `Observable<T>` which
was incorrect since they actually never emit anything. Now they all
return `Observable<never>` (`never` was added in TS 2.0 as a special
type)
jayphelps added a commit to jayphelps/rxjs that referenced this issue Jun 22, 2017
…() now return Observable<never>

fixes ReactiveX#2640

BREAKING CHANGE: Previously, `Observable.never()` `Observable.empty()`
and the `.ignoreElements()` operator all returned `Observable<T>` which
was incorrect since they actually never emit anything. Now they all
return `Observable<never>` (`never` was added in TS 2.0 as a special
type)
david-driscoll pushed a commit that referenced this issue Jun 26, 2017
…() now return Observable<never> (#2691)

fixes #2640

BREAKING CHANGE: Previously, `Observable.never()` `Observable.empty()`
and the `.ignoreElements()` operator all returned `Observable<T>` which
was incorrect since they actually never emit anything. Now they all
return `Observable<never>` (`never` was added in TS 2.0 as a special
type)
claudiordgz added a commit to claudiordgz/rxjs that referenced this issue Mar 8, 2018
Per ReactiveX#2640 ignoreElements should resolve to Observable<never>, ReactiveX#3340 asks for this explicitly, there
was also another issue with tests2png that I fix here

Closes ReactiveX#3340
claudiordgz added a commit to claudiordgz/rxjs that referenced this issue Mar 8, 2018
Previous change tried to add the correct return type but ended up adding
Observable<Observable<never>> which is not what was requested

ReactiveX#3340 and ReactiveX#2640
benlesh pushed a commit that referenced this issue Mar 15, 2018
#3410)

* refactor(ignoreElements): ignoreElements now returns Observable<never>

Per #2640 ignoreElements should resolve to Observable<never>, #3340 asks for this explicitly, there
was also another issue with tests2png that I fix here

Closes #3340, #3410

* refactor(ignoreElements): Wrong return type on type of ignoreElements

Previous change tried to add the correct return type but ended up adding
Observable<Observable<never>> which is not what was requested

#3340 and #2640
@lock
Copy link

lock bot commented Jun 6, 2018

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.

@lock lock bot locked as resolved and limited conversation to collaborators Jun 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants