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

toObservable() is not compatible with Rx (observer is not partial) #245

Closed
felixfbecker opened this issue Dec 5, 2018 · 3 comments
Closed

Comments

@felixfbecker
Copy link

IxJS version:

Code to reproduce:

import {from} from 'rxjs'
import {AsyncIterableX, toObservable} from 'ix/asynciterable'

from(toObservable(AsyncIterableX.from([1,2,3])))

Expected behavior:
Should compile

Actual behavior:
Fails with

[ts]
Type 'Observable<T>' is not assignable to type 'Subscribable<T>'.
  Types of property 'subscribe' are incompatible.
    Type '(observer: Observer<T>) => Subscription' is not assignable to type '{ (observer?: NextObserver<T> | ErrorObserver<T> | CompletionObserver<T> | undefined): Unsubscribable; (next?: ((value: T) => void) | undefined, error?: ((error: any) => void) | undefined, complete?: (() => void) | undefined): Unsubscribable; }'.
      Types of parameters 'observer' and 'next' are incompatible.
        Type '((value: T) => void) | undefined' is not assignable to type 'Observer<T>'.

This is because Observable as defined in Ix takes an Observer where all of next, error and complete have to be provided, instead of being optional.

Additional information:

@trxcllnt
Copy link
Member

trxcllnt commented Dec 5, 2018

Ah yes this has been bothering me too. Ix also doesn't add the Symbol.observable property to the returned Observable, which I've been working around in JS this way:

import symbolObservable from 'symbol-observable';
function asyncIteratorToObservable(x) {
    const obs = AsyncIterable.as(x).toObservable();
    obs[symbolObservable] = () => obs;
    return Observable.from(obs);
}

trxcllnt added a commit that referenced this issue Jan 3, 2019
* update batch specs for jest

* add symbol-observable, fix observable typings/issue #245

* add symbol-observable to closure

* fix symbol-observable for real now

* polyfill Symbol.observable and add tests for RxJS compat

* cleanup, clarify Symbol.observable comment

* require symbol/observable from rx because jest
@arseneyr
Copy link

I think this fix broke compatibility in the other direction (creating an AsyncIterable from a rxjs Observable):

import { AsyncIterable } from "ix";
import { from } from "rxjs";

AsyncIterable.from(from([1, 2, 3]));

works fine with ix@2.3.5 but fails on ix@2.4.3 with

[ts]
Argument of type 'Observable<number>' is not assignable to parameter of type 'AsyncIterableInput<{}>'.
  Property '[Symbol.observable]' is missing in type 'Observable<number>' but required in type 'Observable<{}>'. [2345]

This might be related to ReactiveX/rxjs#3890 where [Symbol.observable] doesn't make it into the typings for rxjs observables.

Note that if you work around this problem with

declare module "rxjs/internal/Observable" {
  interface Observable<T> {
    [Symbol.observable]: () => Observable<T>;
  }
}

it still fails with

[ts]
Argument of type 'Observable<number>' is not assignable to parameter of type 'AsyncIterableInput<number>'.
  Type 'import("./node_modules/rxjs/internal/Observable").Observable<number>' is not assignable to type 'import(".node_modules/ix/observer").Observable<number>'.
    Types of property 'subscribe' are incompatible.
      Type '{ (observer?: NextObserver<number> | ErrorObserver<number> | CompletionObserver<number> | undefined): Subscription; (next?: ((value: number) => void) | undefined, error?: ((error: any) => void) | undefined, complete?: (() => void) | undefined): Subscription; }' is not assignable to type '(observerOrNext?: NextObserver<number> | ErrorObserver<number> | CompletionObserver<number> | ((value: number) => void) | undefined, error?: ((err: any) => void) | undefined, complete?: (() => void) | undefined) => Subscription'.
        Types of parameters 'observer' and 'observerOrNext' are incompatible.
          Type 'NextObserver<number> | ErrorObserver<number> | CompletionObserver<number> | ((value: number) => void) | undefined' is not assignable to type 'NextObserver<number> | ErrorObserver<number> | CompletionObserver<number> | undefined'.
            Type '(value: number) => void' is not assignable to type 'NextObserver<number> | ErrorObserver<number> | CompletionObserver<number> | undefined'.
              Property 'complete' is missing in type '(value: number) => void' but required in type 'CompletionObserver<number>'. [2345]

@trxcllnt
Copy link
Member

Dang, lemme see what I can do about that. Thanks for flagging this @arseneyr!

trxcllnt added a commit that referenced this issue Jan 11, 2019
* fix(toObservable): Fix rxjs typings and symbol-observable interop

fixes #245

* ci(ci): fix npm version to 6.5.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants