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

Spectator-mode subscriptions OR "side-effect-free" subscriptions #1511

Closed
wants to merge 1 commit into from
Closed

Conversation

lcorneliussen
Copy link
Contributor

In #855 knockout "fixed" the flaw that a subscribe on a deferredComputed wouldn't lead to it's computation.

However, in some cases it is still desired to subscribe to a deferred or pure computed without beeing counted as an active subscriber.

This could be solved by adding an option to subscriptions, that could be called spectatorMode (true /false). Subscriptions in spectatorMode would behave differently in a couple of ways:

For deferred computed observables:
(same as before the #855 fix)

  • they would not lead to computation
  • when the computed is evaluated after subscribe, the subscription would be informed about the initial computed value

For pure computed observables:

  • They would not awaken a sleeping pure computed
  • The pure computed would fall asleep, if all "normal" subscriptions are disposed
  • On awake, they would get triggered with the computed value.
  • On falling asleep, they would get triggered with an "undefined" value.

It would also make sence to introduce a mode for pureComputed, stating, if the computed value should be cached, although the computed falls asleep. Then on awake it can compare the values and only notify the spectatorMode-subscriptions, if the value has changed since when it was awake last time.

We have already coded this in a patch, since we need it. But we'd like to discuss it before integrating it into knockout, submitting a pull request and implementing (more) tests.

This is a very important feature for us, since we have designed our api around it. And we would be very happy, if we didn't need to maintain our own fork, but find a solution together instead.

@lcorneliussen
Copy link
Contributor Author

BTW, do you prefer me to separate "concept issue" and "pull request", or is it totally fine to combine the two?

@mbest
Copy link
Member

mbest commented Sep 24, 2014

I had an idea today while I was at the tennis courts that I think may work for you without requiring any new concepts or option parameters. My idea is that we could expose isSleeping as an observable. Thus you'd be able to define a "spectator-mode" subscription using a computed observable:

var subscription = ko.computed(function () {
    if (!theComputed.isSleeping()) {
       do something with theComputed()
    }
});

@lcorneliussen
Copy link
Contributor Author

When I than subscribe whenever it's not sleeping, it will never fall asleep again :(

@mbest
Copy link
Member

mbest commented Sep 25, 2014

You're quite right. I was obviously a bit confused.

@mbest
Copy link
Member

mbest commented Sep 25, 2014

Here's an idea of how part of this could work:

var originalNotifySubscribers = c.notifySubscribers;
c.notifySubscribers = function (value, event) {
    if (!event || event == 'change') {
        console.log('spectated: ' + value);
    }
    originalNotifySubscribers.apply(c, arguments);
}

http://jsfiddle.net/mbest/g2adh9s0/

This doesn't get any notification when the computed goes to sleep or wake up, but that is something we can work on.

@lcorneliussen
Copy link
Contributor Author

Right. I also need a notification when it is evaluated the first time or awakes.

@lcorneliussen
Copy link
Contributor Author

Also i to be able to have multiple of those interceptions - what actually subscribe is for...

@mbest
Copy link
Member

mbest commented Sep 26, 2014

I think I have a good handle on how I want to implement support for this:

  1. A pure computed only awakens when a change subscription is registered and goes to sleep when there are no more change subscriptions. This means we can use other subscription types that don't affect the computed state.
  2. When awakening, a pure computed will notify an awake event with the current computed value. And when going to sleep, it will notify an asleep event with an undefined value.

This provides some flexibility in designing a "spectate" implementation using these events. For example, you could overload notifySubscribers to generate spectate events based on the others:

ko.subscribable.fn.notifySubscribers = (function (original) {
    return function (value, event) {
        if (!event || event == 'change' || event == 'awake' || event == 'asleep') {
            original.call(this, value, 'spectate');
        }
        original.apply(this, arguments);
    };
}(ko.subscribable.fn.notifySubscribers));

@mbest
Copy link
Member

mbest commented Oct 22, 2014

@lcorneliussen Do you think what I've done in #1576 would work for you?

@lcorneliussen
Copy link
Contributor Author

@mbest Looks good! Hopefully I'll be able to test it soon.

@lcorneliussen
Copy link
Contributor Author

That worked fine! Thank you very much for you help.

@brianmhunt
Copy link
Member

On the basis that this is a dupe of #1576 or otherwise @lcorneliussen thinks the workaround is good, I'll close this. Please let me know though if I'm jumping the gun here. :)

@brianmhunt brianmhunt closed this Mar 16, 2015
@lcorneliussen
Copy link
Contributor Author

Actually this is not yet solved. Tested a bit more...

When I subscribe to a computed that doesn*t register dependencies, it won't awake and hence i won't get any notification about the new value.

What about an "evaluated" event that just publishes all changes done to _latestValue?


One might ask why I'd subscribe to something that won't ever change 😵

But if i want to spectate/observe a computed without triggering evaluation, I still need to get hold of the new value if it then is a) peeked, b) unwrapped, c) subscribed to - regardless of wether it registers dependencies or not.

@mbest
Copy link
Member

mbest commented Mar 18, 2015

@lcorneliussen - I don't know that we'll add an "evaluated" event, but you might be able to generate it outside of Knockout. Maybe something like this:

function myPureComputed (evaluator) {
    var wrappedEvaluator = function () {
        var value = evaluator();
        computed.notifySubscribers(value, 'evaluated');
        return value;
    }; 
    var computed = ko.pureComputed(wrappedEvaluator);
    return computed;  
}

@lcorneliussen
Copy link
Contributor Author

Hm. right. or just live with the fork. again.

"evaluate" is also misleading - shouldn't fire if it didn't change.

"change" is perfect. Just one that actually only fires when _latestValue is changed and does nothing else :-)

"cache" would be an option - that is what actually happens.

@mbest
Copy link
Member

mbest commented Mar 18, 2015

"change" is perfect. Just one that actually only fires when _latestValue is changed and does nothing else :-)

That might be possible. Can you open a new issue for this?

@lcorneliussen
Copy link
Contributor Author

"cache" or "change"? I think "cache" is a good name. The value in the cache was updatede - for whatever reason.

@lcorneliussen
Copy link
Contributor Author

Added "cache" in an edit - maybe you didn't see it.

@mbest
Copy link
Member

mbest commented Mar 19, 2015

Added "cache" in an edit - maybe you didn't see it.

Which issue?

@lcorneliussen
Copy link
Contributor Author

This one :-) #1511

@lcorneliussen
Copy link
Contributor Author

I'll open a new issue

lcorneliussen added a commit to lcorneliussen/knockout that referenced this pull request Mar 19, 2015
For pure computeds the "change"-event does not actually fire on every change, but only for changes on active observables. (not on awake and peek) Also, the subscriptions to "change" lead to computation.

This is a necessary hook for knockout#1511.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants