knockout.rx icon indicating copy to clipboard operation
knockout.rx copied to clipboard

disposing an observable should dispose its rxObservable

Open gilesbradshaw opened this issue 10 years ago • 4 comments

gilesbradshaw avatar Sep 20 '14 22:09 gilesbradshaw

just looked a bit more at this, it would also be good if the knockout observable only subscribed to the underlying rx observable when it is subscribed to. to do that I did..

    function addSubscribeDispose(observable, subscribe, dispose) {
        var subscriptions = [];
        var oldSubscribe = observable.subscribe.bind(observable);
        observable.subscribe = function (callback, callbackTarget, event) {
            var oldDispose, ret;
            ret = oldSubscribe(callback, callbackTarget, event);
            if (!subscriptions.length) {
                subscribe();
            }
            subscriptions.push(ret);
            oldDispose = ret.dispose;
            ret.dispose = function () {
                subscriptions.splice(subscriptions.indexOf(ret), 1);
                oldDispose.bind(this)();

                if (!subscriptions.length) {
                    dispose();
                }
            };
            return ret;
        };
    }
    function rx2koObservable(initialValue) {
        var observable = ko.observable(initialValue);
        var rxSubscription;
        var _this = this;
        addSubscribeDispose(observable,function(){
            rxSubscription = _this.subscribe(observable);
        },
        function () {
            return rxSubscription.dispose();
        });
        return observable;
    }

I'm guessing the same applies vice versa too

gilesbradshaw avatar Sep 21 '14 09:09 gilesbradshaw

Thanks for PR, you're the first! :+1:

  1. Can't accept PR, because .js is not a source file here, it has been generated from TypeScript source.
  2. I've got your concern about disposing, but there's a difference in behavior between Rx's observable and Knockout's one.

KO's observable doesn't provide dispose method to dispose it the whole object, it only has ability to dispose distinct subscribables. But the problem, that KO's observable contract requires accessor to last value of observable. So here's a problem: if we stop observing source Rx's observable even if mapped KO's observable doesn't have any subscription, we can't provide last value by the accessor:

var rxObs = new Rx.Subject();
var koObs = rxObs.toKoObservable();

assert.ok(koObs() === undefined, "value wasn't set yet");

rxObs.onNext(1);

assert.equal(koObs(), 1);

Your code will fail the test, because there's no KO's subscriptions yet.

  • Rx's observable can be represented by KO's subscribable object. And the library have such one: rxObservable.toKoSubscribable(). It counts subscriptions and keeps Rx's subscription only if it have any in KO.
  • KO's observable can be represented by Rx's BehaviorSubject. So, BehaviorSubject should be convertible to KO's subscribable with smart subscribing. And I'm going to implement this.

Igor

Igorbek avatar Sep 22 '14 03:09 Igorbek

ah that's true - however if the rx observable is a cold observable and it isn't subscribed to until the ko observable is subscribed to then it won't produce any values until the ko observable subscribes. I'm not sue about subjects tbh - I don't use them just rx.Observable.create. Maybe it's a special case but it works for what I am doing where the underlying rx observable listens to a web api and subscribes to signalr endpoint when ko observable is active and then unsubscribes to signlar when disposed.

its a bit llike ko.computed vs ko.purecomputed

I wonder if there could be an alternative to rx2koObservable say coldRx2koObservable which do0es the above.

best wishes

gilesbradshaw avatar Sep 22 '14 09:09 gilesbradshaw

ps my first foray into typescript (only hacking your code :P)

gilesbradshaw avatar Sep 22 '14 09:09 gilesbradshaw