Asserting proper return types for operators inherited from Observable to allow subclassing it in TypeScript

[slikts]

Subclassing Observable and overriding the lift() method (as shown in the docs) causes TypeScript compiler errors, since the inherited methods that return a lift() call still keep the same return type as at the time of their definition. This is what is happening currently:

class Observable {
  map() { return this.lift() }
  lift() { return new Observable() }
}
class Subclass extends Observable {
  lift() { return new Subclass() }
  foo() {}
}
const o = new Subclass().map()
o.foo() // tsc Error: Property 'foo' does not exist on type 'Observable'.

A simple solution would be to parametrize the methods with their receiver and then assert the return type:

map<T extends Observable>(this: T) { return <T>this.lift() }

This would make adding custom operators by subclassing Observable practicable in TypeScript while having minimal downsides.

Related issues:

• Polymorphic this and Generics microsoft/TypeScript#6223
• Need something more robust than polymorphic this typing for RxJS microsoft/TypeScript#5845

[kwonoj]

I'm closing this issue as dupe of #1876 (and #1829 for reference) There are long running discussion to support class inheritance with preserving types of custom class, but there aren't clean solution with current version of compiler support and design of rx itself. There are few places need to be considered to correctly support this, like static creation method supports ctor types with preserving custom generic as well as prototype operator.

If you have working prototype resolves known issues, please do not hesitate to bring it as PR. It'll definitly solve one of huge pain point we have currently.

[felixfbecker]

What's the purpose of having a generic method that uses the generic parameter only as its return value? It's equivalent to just casting the return.

Reference: https://github.com/DefinitelyTyped/DefinitelyTyped#common-mistakes

getMeAT<T>(): T: If a type parameter does not appear in the types of any parameters, you don't really have a generic function, you just have a disguised type assertion. Prefer to use a real type assertion, e.g. getMeAT() as number. Example where a type parameter is acceptable: function id<T>(value: T): T;. Example where it is not acceptable: function parseJson<T>(json: string): T;. Exception: new Map<string, number>() is OK.

[slikts]

What's the purpose of having a generic method that uses the generic parameter only as its return value? It's equivalent to just casting the return.

The map() method is declared as a separate function, not as a class member, so the this type is not available in it.

... there aren't clean solution with current version of compiler support and design of rx itself.

I mean, this issue is about a potential solution that I hoped to discuss. The issue you linked to does not talk about instance methods, or about the approach of using type assertions for the return type in inherited methods.

[slikts]

Here's a more complete example using Observable as a base:

import { MapOperator } from 'rxjs/operator/map'
import { Operator } from 'rxjs/Operator'

class Observable<T> {
  protected source: Observable<any>
  protected operator: Operator<any, T>

  map<T, R, K extends Observable<R>>(
    this: K,
    project: (value: T, index: number) => R,
    thisArg?: any,
  ): K {
    return <K>this.lift(new MapOperator(project, thisArg))
  }

  lift<R>(operator: Operator<T, R>): Observable<R> {
    const observable = new Observable<R>()
    observable.source = this
    observable.operator = operator
    return observable
  }
}

class Subclass<T> extends Observable<T> {
  lift<R>(operator: Operator<T, R>): Subclass<R> {
    const observable = new Subclass<R>()
    observable.source = this
    observable.operator = operator
    return observable
  }

  bar() {}
}

const foo = new Subclass().map(n => n) // Subclass<{}>

[kwonoj]

@slikts your code snippet doesn't work as it breaks projection function doesn't carry forward generic type of Observable, as well as it doesn't support mapping from Observable<T> to Observable<R> as well.

Referenced issue doesn't specifically mention about all the way we've failed to make it into, but while those discussions are progressing we have noticed most cases works in some usecases, it wasn't able to be applicable with way current codebase designed. So for this case, I'd like to strongly suggest to bring up some PR to discussion progress further.

[kwonoj]

To clarify, closing this issue DOES NOT MEAN this topic is not supported or will not be progressed further. It's simply to make discussion continue in single issue originally opened, to enable easier tracking of history. Also reason to suggest to create PR for discussion is, as you could observe in referenced issue some concept implementation looks working as expected, but after that we've noticed applying it into Rx codebase was not possible due to various reasons. Creating actual PR will address those concerns faster, will make discussion progresses quicker than discussing with non actual Rx code snippets.

[slikts]

@slikts your code snippet doesn't work as it breaks projection function doesn't carry forward generic type of Observable, as well as it doesn't support mapping from Observable to Observable as well.

Thanks for explaining; I was not looking at it carefully enough.

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