Stack overflow using .depends function with computed observable

[Paul-Martin]

Hello Boris,

I've run into what appears to be a regression when using .depends functions with computed observables. In this case I am using them to create context sensitive menus that change based on the selected item. This jsfiddle reproduces the issue.

https://jsfiddle.net/PaulMartin/d51emo1x/

In short if I use this syntax the stack is blown:

ControlBarAction.prototype.enabled.depends = function() {
    // !!!! This produces stack overflow !!!!
    return [this, 'selected', this.selected(), 'enabled'];
};      
    // !!!! This does not 
    return [this, 'selected', 'selected.enabled'];
};

but if I use this syntax (which I believe to be functionally equivalent) it is not

ControlBarAction.prototype.enabled.depends = function() {
    // !!!! This does not 
    return [this, 'selected', 'selected.enabled'];
    // !!!! Nor does this
    return [this, 'selected', 'selected^enabled'];
};      

In the fiddle, you can just comment / uncomment to see the stack overflow.

I had been using the first syntax for quite some time and upgraded to the latest version from 64 yesterday when I encountered this issue. Judging from the change logs, I would guess the enhancements made in commit 72 introduced this issue.

Please let me know if I can provide you any additional information.

[BorisMoore]

Hi Paul,

Thanks for calling this out.

Yes, it is a regression. Here is my current version, with a fix for that:

jsviews74-inprogress.js.txt

What is happening is that in your code

ControlBarAction.prototype.enabled.depends = function() {
    return [this, 'selected', this.selected(), 'enabled'];
};

this.selected() is returning undefined and so you are setting enabled.depends to [this, 'selected', undefined, 'enabled']. There is a bug in jsviews (73) which treats that as [this, 'selected', 'enabled'] and which then gets into a stack overflow from the circular dependency of enabled depending on enabled. Both aspects are fixed in the attached update.

Let me know if you still see issues with this update...

[Paul-Martin]

Thanks Boris!

This does indeed resolve the issue. That was a scaled down snippet of the code I actually use. In this case I use an overloaded definition of selected for which some menus it is initialized (and remains) null and for others it is initialized to a valid value.

With depends, is there a difference between:

[this.selected(), 'enabled'] and ['selected.enabled'] ?

Also, will ['selected^enabled'] pick up the dependency if selected is undefined at the time of binding and later observably updated? I suspect not and I don't rely on this, but am curious.

As always, thanks for all your great work!

On Mar 2, 2016, at 1:28 PM, Boris Moore notifications@github.com wrote:

Hi Paul,

Thanks for calling this out.

Yes, it is a regression. Here is my current version, with a fix for that:

jsviews24-inprogress.js.txt

What is happening is that in your code

ControlBarAction.prototype.enabled.depends = function() { return [this, 'selected', this.selected(), 'enabled']; }; this.selected() is returning undefined and so you are setting enabled.depends to [this, 'selected', undefined, 'enabled']. There is a bug in jsviews (73) which treats that as [this, 'selected', 'enabled'] and which then gets into a stack overflow from the circular dependency of enabled depending on enabled. Both aspects are fixed in the attached update.

Let me know if you still see issues with this update...

— Reply to this email directly or view it on GitHub.

[BorisMoore]

Simply setting xxx.depends = ["selected^enabled"] is the right way to do that scenario. (Even if selected is set later.)

xxx.depends = ["selected.enabled"] or xxx.depends = function() {return [this.selected(), "enabled"]} will be equivalent - and in both cases the depends expression is evaluated once (during template linking) and will not re-evaluate after a change in selected - so there will not be a listener for changes to the enabled property of the new selected. Even depends = ["selected", "selected.enabled"] will only listen to changes to the "selected" and to the initial selected.enabled. - Whereas "selected^enabled"` will do it all!

If you haven't seen it, you should probably take a look at http://www.jsviews.com/#computed@depends. There is a new feature of calling a callback in the depends function style declaration which will give you additional programmatic power if you need it. Note that it allows you (for example) to observe all changes - equivalent to the new ** declarative syntax.

[Paul-Martin]

Wow. Amazing! Thanks for the explanation Boris.

On Mar 2, 2016, at 7:14 PM, Boris Moore notifications@github.com wrote:

Simply setting xxx.depends = ["selected^enabled"] is the right way to do that scenario. (Even if selected is set later.)

xxx.depends = ["selected.enabled"] or xxx.depends = function() {return [this.selected(), "enabled"]} will be equivalent - and in both cases the depends expression is evaluated once (during template linking) and will not re-evaluate after a change in selected - so there will not be a listener for changes to the enabled property of the new selected. Even depends = ["selected", "selected.enabled"] will only listen to changes to the "selected" and to the initial selected.enabled. - Whereas "selected^enabled"` will do it all!

— Reply to this email directly or view it on GitHub.

[BorisMoore]

This fix has now been committed. Closing. (Please re-open if you still see any issues!)