two-way binding on variable with setter causes initial call to setter

[mbroadst]

I have a pretty involved grid component that uses two-way binding to (among other things) control pagination and sort order data. For sort order, I have a number of grid-column-header elements which two-way bind to an order variable, and toggle it based on click of that header: works fantastically. What I am not noticing (and hadn't in previous versions) is that on the initial display, each of the individual column headers is causing a set on the order variable which is of course leading my code to update the entire table N times (N === number of columns). Is there any way to avoid this behavior?

relevant example: http://plnkr.co/edit/3tsFCI0xG30QHYU6rsPt
note that when you start the example you have 20 console.log statements for sharedValue set

[mbroadst]

Hmm, this might be more of an issue with the design on my part. It seems that even if I short circuit the initial set call back to the origin, every time I change it in each grid-column-header that also triggers N-col sets. So it seems the behavior is inherent to the binding type?

[CasiOo]

Changing sharedValue directly from the App class, will call sharedValue's set method twice.
I wouldn't expect it to be set twice either. The bindeable is clearly setting the value again, after it registered the change.

I don't believe it should behave that way, but I have been wrong before haha

[mbroadst]

@CasiOo yeah I'm hoping this is indeed a bug, it would save me some refactoring time 😄

[mbroadst]

I've updated plunker with a button on the top to change the sharedValue directly from the view model, so you can see that setting it there also causes 20 additional sets to the variable

[EisenbergEffect]

@jdanyow Can you look into this or comment?

[jdanyow]

The controller calls the BehaviorPropertyObserver. Maybe this isn't necessary? Then what happens is the behavior property's oldValue is always initially null here, causing the behavior property observer to call it's subscribers (eg the binding instance) resulting in sourceExpression.assign.

I wonder if we should remove the call in the controller and also set the BehaviorPropertyObserver's initial value according to what it's bound to.

[mbroadst]

@jdanyow the last sentence sounds like a decent proposal, would that potentially break other expected behaviors?

[EisenbergEffect]

A bindable has a particular behavior as part of the binding cycle of a component:

• If the bind callback has been implemented on the component, then the bindable's callback should not be called because the developer will recieve the notification via the general bind callback.
• If the bind callback has not been implemented on the component, then the bindable's callback must be called when the initial value is set, otherwise the developer has no way to respond to the initial value of the property.

[EisenbergEffect]

@jdanyow Are you suggesting to have a second implemention, similar to call but that doesn't notify the subscribers?

[EisenbergEffect]

I'm not sure that would work...when would the subscribers be notified? Consider one component that binds to another component's property. That subscriber needs to be notified of the initial value.

[mbroadst]

@EisenbergEffect so you're saying the way to avoid the "initial bind" behavior is to implement bind on the VM of the bindable? That doesn't seem to be the case here, in fact in my personal project I have:

  bind(bindingContext, overrideContext) {
    this.$parent = bindingContext;
  }

so that I can use the VM as a parent for a sub-dialog

[jdanyow]

@EisenbergEffect makes sense, so I guess we have no choice but to evaluate the source expression and compare it to the new value.

This line:

this.updateSource(newValue);

Would become:

if (this.sourceExpression.evaluate(...) !== newValue) {
  this.updateSource(newValue);
}

sound good?

[EisenbergEffect]

What do you think would be the impact of that? It's only going to affect two-way bindings correct? So, that shouldn't be a perf issue. Am I following correctly?

[jdanyow]

right- two-way bindings only, shouldn't be an issue.

[EisenbergEffect]

ok, sounds good