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

Subscribe to deferred computed #855

Closed

Conversation

bman654
Copy link
Contributor

@bman654 bman654 commented Feb 20, 2013

Hi,

If you create a computed with "deferEvaluation: true", and then you never read the computed, but you subscribe to it, your subscription will never trigger, even if you update the underlying observables. This pull request resolves that:

Example:

var d = ko.observable(1),
    c = ko.computed({ read: d, deferEvaluation: true });

c.subscribe(function (value) { alert ("received " + value); });
d(10); // should trigger the subscription but it does not.
c();
d(11); // will now trigger the subscription

This is a trivial example, but imagine large applications where one piece of code creates the deferEvaluation computed (because the computation is expensive and not always used), and some other part of the application receives a computed that it should subscribe to. It doesn't know if the computed is deferred or not.

@SteveSanderson
Copy link
Contributor

Thanks for bringing this up. Also thanks for writing a good clear spec!

I'm not totally sure about the desired semantics of this scenario though. This proposed alteration would change the meaning of deferEvaluation so that it means "delay evaluation until the property is read or is subscribed to". Is that definitely the desired behavior? The existing meaning of deferEvaluation is just "delay evaluation until the property is read", which creates the possibility of queueing a set of subscribers before the computed is ever evaluated. Isn't it possible that people sometimes want to do that? I'd be interested in feedback from others about whether they have ever been in a situation where it matters.

If you could describe the situation where you need to subscribe to a computed that is never naturally evaluated, that would be very helpful! Thanks.

@bman654
Copy link
Contributor Author

bman654 commented Feb 26, 2013

Hi Steven,

Good point.  I thought the purpose of deferEvaluation was to defer evaluation until something has expressed interest in the value.  I had not considered that there are cases where people might in fact be setting up multiple subscriptions before evaluating.

It turns out thus far I've only encountered this when writing unit tests.  But as our application gets more complex, I think it is likely to happen with a scenario sort of like this:

function model() {
   self.a = ko.observable(10);
   self.b = ko.observable(20);

   // we defer evaluation of this because it isn't always used and it is expensive to calculate
   self.c = ko.computed({ read: someExpensiveCalculation, deferEvaluation: true });
}

function bindUpdatesToServer(someSubscribable, context) {
   if (!ko.isSubscribable(someSubscribable)) { throw new Error ("must be subscribable"); }
   return observableValue.subscribe(updateServer.bind(context));

}

// elsewhere in the application

var myModel = new model();

if (weNeedToSendCalculatedResultsToServer) {
  // With current KO, this will not do anything unless "weNeedToVisualizeCalculation" is also true
  bindUpdatesToServer(myModel.c, ...);
}

// in some view
<div data-bind="if: weNeedToVisualizeCalculation"><div data-bind="text: c"></div></div>

This is a somewhat contrived example, but hopefully indicates the scenario.  Only the creator of the computed should know/care that it is deferred.  A consumer of the subscribable interface (like bindUpdatesToServer) doesn't even care if the object is an observable or computed.  It wouldn't be appropriate for the bindUpdatesToServer() to do a ko.isComputed(someSubscribable) && someSubscribable() statement.  It also doesn't seem appropriate for the code that is calling bindUpdatesToServer() to do this.  The model itself doesn't know about the conditions under which its property might actually be evaluated so it cannot do it.  Which doesn't leave us with any good place to "start" the computed evaluations.

To your original question.  Instead of my current pull request (which is indeed potentially a breaking change), then what about either a new property, or (probably preferable) a special deferEvaluation value to signal this specific behavior.  Something like:

ko.deferUntilRead = true;
ko.deferUntilReadOrSubscribe = {}; // will test using === so will never match any existing code.


c = ko.computed({ read: ..., deferEvaluation: ko.deferUntilReadOrSubscribe }); // triggers new behavior.  Guaranteed not to conflict with any existing code.

d = ko.computed({ read: ..., deferEvaluation: ko.deferUntilRead }); // existing behavior
e = ko.computed({ read: ..., deferEvaluation: true }); // existing behavior
f = ko.computed({ read: ..., deferEvaluation: anyTruthyValue }); // existing behavior (I cannot remember if you do an === true or just any truthy value.

If you like the direction this is taking, then I'll update my pull request, and also include a spec which guarantees the existing deferEvaluation behavior.

 

Brandon
http://palladiumblog.wordpress.com/


From: Steven Sanderson notifications@github.com
To: SteveSanderson/knockout knockout@noreply.github.com
Cc: Brandon Wallace brandon@palladiumconsulting.com
Sent: Tuesday, February 26, 2013 6:34 AM
Subject: Re: [knockout] Subscribe to deferred computed (#855)

Thanks for bringing this up. Also thanks for writing a good clear spec!
I'm not totally sure about the desired semantics of this scenario though. This proposed alteration would change the meaning of deferEvaluation so that it means "delay evaluation until the property is read or is subscribed to". Is that definitely the desired behavior? The existing meaning of deferEvaluation is just "delay evaluation until the property is read", which creates the possibility of queueing a set of subscribers before the computed is ever evaluated. Isn't it possible that people will sometimes want to do that? I'd be interested in feedback from others about whether they have ever been in a situation where it matters.
If you could describe the situation where you need to subscribe to a computed that is never naturally evaluated, that would be very helpful! Thanks.

Reply to this email directly or view it on GitHub.

@mbest
Copy link
Member

mbest commented Dec 23, 2013

In my development with Knockout, I almost always use the deferEvaluation option for computed observables because it simplifies the process of initializing the view model objects (the initialization order doesn't matter). But I have never tried to subscribe to a computed before it's evaluated. In fact, I have always written my code assuming the subscribe callback isn't called with an observable's initial value. If I want it called with the initial value, I do it manually:

callback(observable());
observable.subscribe(callback);

In my opinion, the current behavior is broken and needs to be fixed. Of course, we need to be careful when changing behavior. I'd like to hear what @rniemeyer thinks about this also.

@rniemeyer
Copy link
Member

This is an interesting case. I feel that someone doing a manual subscription should not need to know if the computed is using deferEvaluation (or even that it is a computed rather than an observable) and should not need to know if something else is actually requesting its value elsewhere. So, I do think this should be changed and think that deferEvaluation should mean that it is deferred until someone is interested in the value and/or changes to it (they could just peek).

However, I would not expect the subscription callback to be immediately called, if it triggered the computed evaluation. Like @mbest, I always manually trigger it, if that is required.

If we are concerned about this being a breaking change, then we could consider providing a separate option to force evaluation on subscriptions, at least until the next major version.

@bman654
Copy link
Contributor Author

bman654 commented Jan 1, 2014

Yes I agree and I believe that is how I originally wrote this pull request. My intent was to make deferred computeds behave exactly the same as regular computeds (and observables) to an outside observer.

As for backcompat, one could just define a constant to set deferEvaluation to for the new behavior such as...

ko.deferUntilAccessed = {};

Now code that wants the new behavior just sets

deferEvaluation: ko.deferUntilAccessed

@mbest mbest mentioned this pull request Jan 1, 2014
5 tasks
@lcorneliussen
Copy link
Contributor

I wouldn't like subscribe to run the observable. and there is no other way to determine it's dependencies than running it...

So i think it is totally fine like it is now. But I admit that it is tricky to write lazy code. We try to setup everything so that most of the computeds are only evaluated and ran when a template finally accesses them.

@rniemeyer What do you mean by "they could just peek?" - a peek to a computed also evaluates it!

@rniemeyer
Copy link
Member

@lcorneliussen - What I said about peek was to clarify that I think deferEvaluation should mean that it is deferred until either someone wants the value (by accessing the value or peeking it) or is interested in changes (manual subscription). I was not trying to indicate that peek was a workaround.

@mbest
Copy link
Member

mbest commented Jan 9, 2014

Updated implementation is in #1272.

@mbest
Copy link
Member

mbest commented Feb 3, 2014

Fixed with #1272

@mbest mbest closed this Feb 3, 2014
@lcorneliussen
Copy link
Contributor

I'm misused the "bug" to "attach" functionality to a computed that I only want to execute, if someone else is/was interested in the values of the computed. How can I still achieve that behavior?

An example: If someone is interested in an observableArray (by evaluating it...), I start listening to changes and call cleanup-functionality on discarded values.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants