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

Observable.from(iterable) generates Typescript compiler error #2306

Closed
Jason3S opened this Issue Jan 22, 2017 · 25 comments

Comments

Projects
None yet
10 participants
@Jason3S

Jason3S commented Jan 22, 2017

RxJS version: 5.0.3

Code to reproduce:

function* generateDoubles(seed: number) {
  let i = seed;
  while (true) {
    yield i;
    i = 2 * i; // double it
  }
}

const iterator = generateDoubles(3);
const result = Rx.Observable.from<number>(iterator).take(10);
result.subscribe((x: number) => console.log(x));

Expected behavior:
Since this is the example given on reactivex.io
It would be expected to compile in Typescript without an error.

Actual behavior:

error TS2345: Argument of type 'IterableIterator<number>' is not assignable to parameter of type 'ObservableInput<number>'.
  Type 'IterableIterator<number>' is not assignable to type 'PromiseLike<number>'.
    Property 'then' is missing in type 'IterableIterator<number>'.

Additional information:
This seems to be an issue with the definition of ObservableInput<T>:

export interface Subscribable<T> {
    subscribe(observerOrNext?: PartialObserver<T> | ((value: T) => void), error?: (error: any) => void, complete?: () => void): AnonymousSubscription;
}
export declare type SubscribableOrPromise<T> = Subscribable<T> | PromiseLike<T>;
export declare type ObservableInput<T> = SubscribableOrPromise<T> | ArrayLike<T>;

The iterable type is missing.

A fix might be to add:

export interface IterableLike<T> {
    [Symbol.iterator](): Iterator<T> | IterableIterator<T>;
}

// and fix ObservableInput<T>
export declare type ObservableInput<T> = SubscribableOrPromise<T> | ArrayLike<T> | IterableLike<T>;

tsconfig.json

{
	"compilerOptions": {
		"target": "es2015",
		"module": "commonjs",
		"moduleResolution": "node",
		"sourceMap": true,
		"alwaysStrict": true,
        "declaration": true,
		"noImplicitAny": true,
		"noImplicitThis": true,
		"strictNullChecks": true,
		"outDir": "dist",
		"lib": ["dom", "dom.iterable", "es2015", "es2015.iterable", "scripthost"]
	},
	"include": [
		"src"
	]
}
@kwonoj

This comment has been minimized.

Member

kwonoj commented Jan 22, 2017

Yes, seems type definition need to be updated.

@sublimator

This comment has been minimized.

sublimator commented Jan 29, 2017

Yes please :)

@david-driscoll

This comment has been minimized.

Collaborator

david-driscoll commented Jan 29, 2017

It's a catch 22 at the moment.

If we enable iterables then anyone that doesn't consume iterables will have a hard time...

Let me see if I can't figure out a good way to deal with this, that is opt in with augmentation.

@sublimator

This comment has been minimized.

sublimator commented Jan 29, 2017

@david-driscoll

This comment has been minimized.

Collaborator

david-driscoll commented Jan 29, 2017

We can augment interfaces, but it doesn't appear there is any way to augment type unions. Which kind of makes sense with how augmentation works, it's kind of crappy though.

We had Iterable<T> in there, during the early days, but we removed it because of problems with users that don't have it enabled at the compiler level. This was before the compiler could choose a custom combination of libs (which came with 2.0 iirc). See #1582 for the code that removed it.

Now that we have lib:[] we might be able to bring it back.

Risks involve:

  • Anyone that wants to use RxJS must turn on es2015.iterable
  • If they don't target ES6 (perhaps ES5, needs testing) they will get weird runtime errors if they attempt to use Iterable.

Ts2.2 is bringing in downlevel support for generators to ES3, via Microsoft/TypeScript#12346 . So once we get to TS 2.2 this seems like an appropriate change, until then I'm not really sure.

@sublimator

This comment has been minimized.

sublimator commented Jan 29, 2017

@Jason3S

This comment has been minimized.

Jason3S commented Jan 29, 2017

Sadly I am not seeing a clean way to do this other than to create a new file Rx.es2015.ts

The code already works with Iterators, so the goal is to communicate to the Typescript compiler that Iterators are ok.

Here is a solution.

  1. It requires the end user to do: import * as Rx from 'rxjs/Rx.es2015'
  2. It extends the function signature of the Observable.from to include Iterator<T> and IterableIterator<T>.

This will work, but:

  1. It introduces a new import end point that will need to be supported.
  2. There are now 3 places to maintain the type signature for the create function.

Rx.es2015.ts

export * from './Rx';
import './add/observable/from.es2015';

./add/observable/from.ts

import { Observable } from '../../Observable';
import { from as staticFrom } from '../../observable/from';
import { IScheduler } from '../../Scheduler';

Observable.from = staticFrom as Observable.CreateObservableFrom;

declare module '../../Observable' {
  namespace Observable {
    export interface CreateObservableFrom {
      <T>(ish: ObservableInput<T>, scheduler?: IScheduler): Observable<T>;
      <T, R>(ish: ArrayLike<T>, scheduler?: IScheduler): Observable<R>;
      <T>(ish: ObservableInput<T>, scheduler?: IScheduler): Observable<T>;
    }

    export let from: CreateObservableFrom;
  }
}

./add/observable/from.es2015.ts

import { Observable } from '../../Observable';
import { IScheduler } from '../../Scheduler';

declare module '../../Observable' {
  namespace Observable {
    export interface CreateObservableFrom {
      <T>(ish: IterableIterator<T>, scheduler?: IScheduler): Observable<T>;
      <T>(ish: Iterator<T>, scheduler?: IScheduler): Observable<T>;
    }
  }
}
@david-driscoll

This comment has been minimized.

Collaborator

david-driscoll commented Jan 29, 2017

It's even easier really, we only need to augment ObservableInput<T> to support iterator and it will flow through to subscribeToResult.

The problem still stands we need two different versions of ObservableInput<T> without causing a breaking change for all users, and even worse, allowing them to use code that may not be supported by the runtime they are running on.

It's been easy enough for where I use to iterators to just cast the observable to the correct type, and cast the iterator to <any>. It's not type safe, but I haven't been able to find a good workaround that doesn't make a lot of work.

@Jason3S

This comment has been minimized.

Jason3S commented Jan 30, 2017

Because ObservableInput<T> is a type and not a interface, I don't think you can change it in a way that will work with ES5. This is because Iterator<T> and IterableIterator<T> do not exist.

Note this issue is about the type interface for FromObservable.create. There is not a runtime issue. The way the code detects iterables at runtime will work for both ES5 and ES2015,
typeof ish[$$iterator] === 'function' :

  static create<T>(ish: ObservableInput<T>, scheduler?: IScheduler): Observable<T> {
    if (ish != null) {
      if (typeof ish[$$observable] === 'function') {
        if (ish instanceof Observable && !scheduler) {
          return ish;
        }
        return new FromObservable<T>(ish, scheduler);
      } else if (isArray(ish)) {
        return new ArrayObservable<T>(ish, scheduler);
      } else if (isPromise(ish)) {
        return new PromiseObservable<T>(ish, scheduler);
      } else if (typeof ish[$$iterator] === 'function' || typeof ish === 'string') {
        return new IteratorObservable<T>(ish, scheduler);
      } else if (isArrayLike(ish)) {
        return new ArrayLikeObservable(ish, scheduler);
      }
    }
@Jason3S

This comment has been minimized.

Jason3S commented Jan 30, 2017

Instead of making things complicated for a temporary solution, my vote is to wait for TS 2.2 and then add IterableLike<T> to OvservableInput<T>.

This is the workaround I use for Sets:

function observableFromSet<T>(source: Set<T>): Rx.Observable<T> {
    return Rx.Observable.from(source as any);
}

or a more generic workaround:

interface IterableLike<T> {
    [Symbol.iterator]: () => Iterator<T> | IterableIterator<T>;
};

function observableFromIterable<T>(source: IterableLike<T>): Rx.Observable<T> {
    return Rx.Observable.from(source as any);
}

// example usage:
const s = new Set([1, 1, 2, 1, 1, 2, 3]);
const o = observableFromIterable(s);
@sublimator

This comment has been minimized.

sublimator commented Feb 1, 2017

@felixfbecker

This comment has been minimized.

Contributor

felixfbecker commented Apr 9, 2017

Can we just have an additional operator fromIterable() maybe?

@felixfbecker

This comment has been minimized.

Contributor

felixfbecker commented May 6, 2017

@david-driscoll any update on this? TS 2.3 is out now, and if that doesn't allow this, I would prefer to have an explicit operator like we have fromPromise(). This bug bites me every day and forces me to cast to any everywhere in my app, removing the whole point of type safety.

@felixfbecker

This comment has been minimized.

Contributor

felixfbecker commented May 6, 2017

As an example what a major issue this is, I just wanted to refactor a project to use WHATWG URL objects instead of strings, and after the change there was no compile error because an iterator that previously emitted strings now emitted URLs, but was casted to any. A comparison operator that compared strings by value before now compared objects by reference and resulted in a silent bug.

@HipsterZipster

This comment has been minimized.

HipsterZipster commented Jul 30, 2017

What's the current status on this issue? It's really a bummer having to cast back to <any>.
Is there any interest in adding a fromIterable() operator? Is there any better way to resolve this now that we're up to TypeScript 2.4? @david-driscoll @felixfbecker

@jbcpollak

This comment has been minimized.

jbcpollak commented Oct 29, 2017

I am still running into this with the latest TS and RxJS but I get a slightly different error:

error TS2345: Argument of type 'IterableIterator<MyObject>' is not assignable to parameter of type 'ObservableInput<MyObject>'.
  Type 'IterableIterator<MyObject>' is not assignable to type 'ArrayLike<MyObject>'.
    Property 'length' is missing in type 'IterableIterator<MyObject>'.

It would be great to have a fix.

@felixfbecker

This comment has been minimized.

Contributor

felixfbecker commented Oct 29, 2017

@jbcpollak you need to write your own helper function for now like this:

export function observableFromIterable<T>(iterable: Iterable<T>): Observable<T> {
    return Observable.from(iterable as any)
}
@jbcpollak

This comment has been minimized.

jbcpollak commented Oct 29, 2017

Thanks I did that! I was just hoping for a permanent solution

@pablocarreraest

This comment has been minimized.

pablocarreraest commented Feb 3, 2018

Hi @david-driscoll I make this at #1497 but when I use that function at yield die the execution (don't crush just don't happen anything) can you give me some guide what can that be... thanks noble man if you can help me

@benlesh

This comment has been minimized.

Member

benlesh commented Feb 5, 2018

I'm wondering if we should just throw our hands up with from and pass any to it. The type checking here is pretty painful, and then we have to check at run time anyhow.

@david-driscoll

This comment has been minimized.

Collaborator

david-driscoll commented Feb 5, 2018

Now that we have Iterable<T> used elsewhere in the code base, and the fact that TypeScript with iterables have been out for a while, I think we can add Iterable<T> to ObservableInput<T> and then this problem goes away.

@bloadvenro

This comment has been minimized.

bloadvenro commented Mar 16, 2018

My workaround for now is Observable.from((fibonacciGenerator() as any) as Array<number>).

@david-driscoll

This comment has been minimized.

Collaborator

david-driscoll commented Apr 25, 2018

@Jason3S can you confirm this is now working in rxjs6? We made the Iterable<> type flow through.

@Jason3S

This comment has been minimized.

Jason3S commented May 5, 2018

This seems to work just fine with RxJs 6.

@Jason3S Jason3S closed this May 5, 2018

@lock

This comment has been minimized.

lock bot commented Jun 5, 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 5, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.