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

Subclassing Observable - Observable SubClasses extend Observable rather than MyObservable #1124

Closed
niklasfasching opened this issue Jan 3, 2016 · 10 comments

Comments

@niklasfasching
Copy link

I want to subclass Observable and extend it.

The documentation on this shows how to subclass observable and extend it with custom operators, however I run into problems whenever an operator / static method of MyObservable returns an instance of a different class.

All other Observable classes (e.g. ArrayObservable, ConnectableObservable) extend the original Observable class rather than MyObservable, meaning my custom operators will not be available.

class MyObservable extends Observable {
  lift(operator) {
    const observable = new MyObservable(); //<-- important part here
    observable.source = this;
    observable.operator = operator;
    return observable;
  }

  customOperator() {
    /* do things and return an Observable */ 
  }
}

// customOperator is not defined as the observable instance is not a subclass of MyObservable but Observable
MyObservable.of(42).customOperator() 

// returns a ConnectableObservable, meaning customOperator is again not available.
myObservable.publish().customOperator()

How is subclassing meant to be used?

Overwriting the methods to wrap the returned Observable in an instance of MyObservable feels dirty.

class MyObservable extends Observable {
  lift(operator) {
    const observable = new MyObservable();
    observable.source = this;
    observable.operator = operator;
    return observable;
  }

  customOperator() {
    return this;
  }

  static of() {
    const observable = new MyObservable();
    observable.source = Observable.of(...arguments);
    return observable;
  }
}


MyObservable.of(1,2,3)
  .customOperator()
  .subscribe(value => console.log(value));
@niklasfasching niklasfasching changed the title Subclassing Observable - operators returning instances of other classes Subclassing Observable - Observable SubClasses extend Observable rather than MyObservable Jan 3, 2016
@kwonoj
Copy link
Member

kwonoj commented Jan 3, 2016

This is related with #642. Current modularity streategy does not correctly support extended definitions due to creation of observable (i.e, of, etcs..) always directly created Observable as you've mentioned above. It's not resolved 100% in current codebase and trying to find feasible way.

@benlesh
Copy link
Member

benlesh commented Jan 4, 2016

MyObservable.of(42).customOperator() 

This wouldn't work on any type you subclassed in ES6.

Imagine this simplified form:

class A {
  foo() {
    console.log('foo from A');
  }

  static of() {
    return new A();
  }
}

class B extends A {
  foo() {
    console.log('foo from B');
  }
}


var x = B.of();

console.log(x.foo()); // "foo from A"

While B gets A's static of method, it's not going to change the type returned by of, because it's not going to call different code to create a B instead of an A. This is because there really isn't an automatic way to have this type of psuedo higher-kinded typing in JavaScript. If you want to do this, you have to override the static methods.

@benlesh
Copy link
Member

benlesh commented Jan 4, 2016

On the ConnectableObservable front. We could implement something that dynamically creates that type by extending this.constructor or something with ES5 internal to the publish method, but if we did this, we'd end up creating a new class for each publish call. That's probably not going to optimize very well.

@benlesh
Copy link
Member

benlesh commented Jan 4, 2016

@Nupf does the above information help you?

@kwonoj
Copy link
Member

kwonoj commented Jan 4, 2016

While I was trying mixin approach in PR #643, I came to thought above scenario (maybe) possible in combination of enhanced typescript support (relates to #846). It was very vague idea at the moment and still not concrete clear due to known limitataions, reason bringing #642 is related. Even without direct static creator (of, etcs), Observable creation limits extended behavior regarding type definition perspective while runtime behavior's updated via patch operators.

@trxcllnt
Copy link
Member

trxcllnt commented Jan 4, 2016

This is what let is for. As long as your custom Observable subclass can accept a source and return an instance of itself, it will work:

Observable.of(42)
  .let(o => MyObservable.from(o))
  .customOperator()
  .subscribe()

@niklasfasching
Copy link
Author

Thanks for taking the time.

@kwonoj: Thanks, I'll have a look at those.

@Blesh: Yep, I was just hoping there was a way I didn't know about and/or I was just doing something wrong.

@trxcllnt: Thanks, I didn't know about that operator. As far as I can tell, it's waiting to be merged in #1105, so I can't test it but it looks like it merely invokes the provided function on the source observable

export function letProto<T, R>(func: (selector: Observable<T>) => Observable<R>): Observable<R> {
  return func(this);
}

As MyObservable.from returns the same thing Observable.from (@Blesh's comment) returns, this (as far as I can tell) won't convert o into an instance of MyObservable.

As my original question is answered (not possible at the moment) I'll close this issue for now.

@trxcllnt
Copy link
Member

trxcllnt commented Jan 4, 2016

@nupf ah, I was assuming MyObservable.from would be your own from implementation that returns an instance of MyObservable. You could also accept a source Observable in the constructor instead, and return new MyObservable(o) from the let selector.

@niklasfasching
Copy link
Author

@trxcllnt I thought about that but ran into problems with ConnectableObservable / publish. Is that how you would have implemented it?

class MyObservable extends Observable {
  constructor(observable) {
    super();
    this.source = observable;
  }
}

It works great as long as the source observable only has a subscribe method (because that's what will be forwarded to myObservable.source on subscription to myObservable). ConnectableObservable destroys the illusion.

let observable = Observable.of(42).publish();
let myObservable = new MyObservable(observable);
myObservable.source.connect(); // rather than myObservable.connect()

It works, but it doesn't feel clean. Thanks anyways :)

@lock
Copy link

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

4 participants