Extending Observable - static of method

[benlesh]

While talking with @jayphelps about ActionsObservable from redux-observable/redux-observable, it's come up once or twice that someone would need to create an ActionsObservable from an array or set of values...

Currently, this means you have to manually override static of in the ActionsObservable if you want to get an ActionsObservable as a result.

I think we could possibly change the static of method to do something like this:

static of(...values) {
  const observable = new this();
  const scheduler = undefined;
  if (values[values.length - 1] instanceof Scheduler) {
    scheduler = values.pop();
  }
  observable.source = new ArrayObservable(values, scheduler);
  return observable;
}

This would mean that extended classes like ActionsObservable would be able to benefit from of without needing to override it.

Risks

1. I'm not sure what this would mean for things like Subjects yet. I'd have to dig in, but I can't imagine why someone would create Subject with Subject.of.
2. I suspect TypeScript will hate this... because there's not really a great way to determine the returned type. We do know it should always at least be an Observable, though.
3. I have only ran into one use case for this so far, and that's ActionsObservable.

Workaround

The current workaround if you needed this sort of thing is to simply implement static of yourself, (which is basically the same as above):

static of(...values) {
  const observable = new MyObservableType();
  const scheduler = undefined;
  if (values[values.length - 1] instanceof Scheduler) {
    scheduler = values.pop();
  }
  observable.source = new ArrayObservable(values, scheduler);
  return observable;
}

It's perhaps just a little gross to have to do this?

[jayphelps]

@Blesh I think you actually mean new this()

static of(...values) {
  const observable = new this();
  const scheduler = undefined;
  if (values[values.length - 1] instanceof Scheduler) {
    scheduler = values.pop();
  }
  observable.source = new ArrayObservable(values, scheduler);
  return observable;
}

My initial testing suggests that TypeScript is perfectly fine with this approach. It knows what type this is. Demo

[kwonoj]

What if implicitly declare variable in demo such as

const action$ = ActionObservable.of()? seems static of's return type are not explicitly reflecting caller if it's inherited class or not.

[jayphelps]

@kwonoj sorry, I don't follow?

TS seems perfectly okay with const action$ = ActionsObservable.of();

Demo

[kwonoj]

I refer this , implicit variable

class ActionsObservable extends Observable {
    test(): void {
        console.log('meh');
    }
}

const action$ = ActionsObservable.of();

action$.test(); //Typescript can't infer method in inherited class

[jayphelps]

shit...

Yep, known issue microsoft/TypeScript#5863

[benlesh]

Another thing about this approach is it would add a layer of indirection for the basic Observable.of(1, 2, 3) use case. There would be two subscriptions involved instead of just one.

That would incur a definite performance hit.

[jayphelps]

Using generics we could provide an ugly ass workaround....

Demo

class Observable {
    static of<T extends Observable>(): Observable & T {
        return new this() as Observable & T;
    }
}

class ActionsObservable extends Observable {
    test(): void {
        console.log('meh');
    }
}

const action$ = ActionsObservable.of<ActionsObservable>();

action$.test();

If we did such a thing, prolly would want to be a signature overload, so people can just omit the generic type for the default base type.

I don't love this...but given how many people don't use TS we can't let it always limit our decisions. Particularly because they'll very likely support polymorphism on this eventually.

The perf hit is the biggest argument against IMO. We'll need to run the test suite and see how much of a hit it ends up actually being.

We could be overly clever and only go down this path when needed...

static of(...values) {
  // etc...
  const source = new ArrayObservable(values, scheduler);

  if (this === undefined || this === Observable) {
    return source;
  } else {
    const observable = new this();
    observable.source = source;
    return observable;
  }
}

The real code is more complex than that. this might be Observable|ArrayObservable|EmptyObservable|etc

I'm not sure where I land on all of this. I really want this ability..just not confident this is the best approach atm.

[trxcllnt]

I've also used the new this(...) pattern for static creation methods in rxjs-gestures, so add one more use-case.

[benlesh]

I don't love this...but given how many people don't use TS we can't let it always limit our decisions.

I wouldn't let TS limit our decisions unless it was going to result in something horribly unergonomic for the large number of Angular 2 users that are using RxJS 5.

It seems like it's a known issue in TypeScript and they're working to resolve it. So I don't think we need any crazy TypeScript workarounds. No reason to junk up code any more than we have for typings issues. We probably just want to document it for the TypeScript users and reference that issue.

[ceymard]

Hello, I posted a solution to this approach that works with typescript >= 2.0 on microsoft/TypeScript#5863 if you're interested

[ceymard]

interface IObservableClass<T extends Observable> {
  new (...a: any[]): T
  // ... + other static methods/members of Observable
}

class Observable {
  // The `this:` is the important part here
    static of<T extends Observable>(this: IObservableClass<T>): T {
        return new this();
    }
}

class ActionsObservable extends Observable {
    test(): void {
        console.log('meh');
    }
}

// voilà, no more type specification !
const action$ = ActionsObservable.of();
// acion$ is an ActionsObservable, test() is now known.
action$.test();

[jayphelps]

@ceymard very cool. I worry about the interop story if we upgrade to TS 2.0 too early. I imagine users of 1.x would be SOL.

[kwonoj]

@jayphelps I expect adapt 2.0 probably considerable after official public release + ng2's compiler upgrade schedule, as same as introducing 1.8 for module augmentation.

[kwonoj]

I tried this myself and it seems there isn't a perfect solution for type definition to support this.

Below is code snippet shows it,

export interface ObservableCtor<X, U extends Observable<X>> {
  new(): U;
}

class Observable<T> {
  static of<R, U extends Observable<R>>(this: ObservableCtor<R, U>, ...args: Array<R>): U {
    return new this();
  }

  static ofBaseType<R, U extends Observable<R>>(this: ObservableCtor<R, U>, ...args: Array<R>): Observable<R> {
    return new this();
  }
}

class ActionsObservable<T> extends Observable<T> {}

let c1 = Observable.of(42); //Observable<any>
let c2 = ActionsObservable.of(42); //ActionsObservable<any>

let c3: ActionsObservable<number> = ActionsObservable.of(42); //forcing, supplying correct type 
let c4 = ActionsObservable.of<number, ActionsObservable<number>>(42); //forcing, supplying correct type 

let b1 = Observable.ofBaseType(42); //Observable<number>
let b2 = ActionsObservable.ofBaseType(42); //Observable<number>

This is because TS currently does not support higher order generic constraint, so there isn't a way to force U to have correct generic type R. Generic constraint Observable<R> in U only affect U is the type of Observable, since constraint type doesn't affect instantiation of U.

note : on TS 2.0.7, 2.1.4 both.

[benlesh]

Closing as stale.

[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.