New 2-way bindings run setter twice with default values #2049

Closed
dylanrtt opened this Issue Oct 30, 2015 · 4 comments

Comments

Projects
None yet
2 participants
@dylanrtt
Contributor

dylanrtt commented Oct 30, 2015

When a component's viewmodel has a default value for a property...

can.Component.extend({
  tag: 'foo-bar',
  viewModel: {
    foo: 5
  }
});

...if you setup a 2-way binding to that property with the parent scope any of these ways...

<foo-bar foo="{bar}"></foo-bar>
<foo-bar {(foo)}="bar"></foo-bar>
<foo-bar {(foo)}="*bar"></foo-bar>

...the child property foo is set to its default (5) twice during initialization if the parent scope value (bar or *bar) is undefined.

http://jsbin.com/cokofusagu/edit?html,js,console,output

I added a log to the jsin so you can see it setting twice. I am using the old syntax in there so you can easily rollback to 2.2.9 or earlier to see it's a regression.

This is especially a problem with using 2-way reference scope bindings because they aren't set initially.

@justinbmeyer justinbmeyer added the bug label Oct 31, 2015

@justinbmeyer justinbmeyer added this to the 2.3.2 milestone Oct 31, 2015

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Nov 3, 2015

Contributor

Is this causing actual problems? With #2046, which will restore the old behavior of not setting parent scope even if it's undefined, the legacy behavior should be restored, so there shouldn't be a regression with foo="{bar}".

This behavior is by design w/ the new syntax. It makes the parent the final arbiter of truth. What's happening is that the child is being set, the parent changes, and whatever the parent's new value is is set to the child.

This is valuable because if the child changes to a value like "5", and the parent then updates its own value to 5, the child will then be set to 5. Updating a child always gets the resulting parent value set back on the child.

However, the child should not end up having a "change" or other event fired in the case the default value is set twice.

The only place where I can see this being a problem is if a setter is doing complex logic. However, it can check the current value and just do nothing if the new set value equals the current value.

This is actually how 2.2.9 worked, just not during initialization. So, the only regression is with initialization.

Contributor

justinbmeyer commented Nov 3, 2015

Is this causing actual problems? With #2046, which will restore the old behavior of not setting parent scope even if it's undefined, the legacy behavior should be restored, so there shouldn't be a regression with foo="{bar}".

This behavior is by design w/ the new syntax. It makes the parent the final arbiter of truth. What's happening is that the child is being set, the parent changes, and whatever the parent's new value is is set to the child.

This is valuable because if the child changes to a value like "5", and the parent then updates its own value to 5, the child will then be set to 5. Updating a child always gets the resulting parent value set back on the child.

However, the child should not end up having a "change" or other event fired in the case the default value is set twice.

The only place where I can see this being a problem is if a setter is doing complex logic. However, it can check the current value and just do nothing if the new set value equals the current value.

This is actually how 2.2.9 worked, just not during initialization. So, the only regression is with initialization.

@justinbmeyer justinbmeyer removed the bug label Nov 3, 2015

@dylanrtt

This comment has been minimized.

Show comment
Hide comment
@dylanrtt

dylanrtt Nov 3, 2015

Contributor

Ah, thanks for the explanation. Looks like there's no bug here.

Contributor

dylanrtt commented Nov 3, 2015

Ah, thanks for the explanation. Looks like there's no bug here.

@dylanrtt dylanrtt closed this Nov 3, 2015

@dylanrtt

This comment has been minimized.

Show comment
Hide comment
@dylanrtt

dylanrtt Nov 4, 2015

Contributor

I thought on this some more. When a child updates the references scope, what is the point in setting that value back on the same child that caused the update if the references scope can't possibly have changed the value because it can't have setters?

In general, I think it is important to prevent unnecessary updates as much as possible because I have run into valid situations where unexpected updates have instigated complex logic.

Contributor

dylanrtt commented Nov 4, 2015

I thought on this some more. When a child updates the references scope, what is the point in setting that value back on the same child that caused the update if the references scope can't possibly have changed the value because it can't have setters?

In general, I think it is important to prevent unnecessary updates as much as possible because I have run into valid situations where unexpected updates have instigated complex logic.

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Nov 4, 2015

Contributor

The references scope can't have setters, so what you are saying is valid. But references is a special case. For the most part, bindings don't know if they are binding to a reference or any other type of value.

I'd prefer not to add code for a special case unless someone sees real-world problems with this. It just feels a bit strange to have this behavior with every other observable except for can.view.Scope.Ref.

Sent from my iPhone

On Nov 4, 2015, at 1:14 AM, dylanrtt notifications@github.com wrote:

I thought on this some more. When a child updates the references scope, what is the point in setting that value back on the same child that caused the update if the references scope can't possibly have changed the value because it can't have setters?

In general, I think it is important to prevent unnecessary updates as much as possible because I have run into valid situations where unexpected updates have instigated complex logic.


Reply to this email directly or view it on GitHub.

Contributor

justinbmeyer commented Nov 4, 2015

The references scope can't have setters, so what you are saying is valid. But references is a special case. For the most part, bindings don't know if they are binding to a reference or any other type of value.

I'd prefer not to add code for a special case unless someone sees real-world problems with this. It just feels a bit strange to have this behavior with every other observable except for can.view.Scope.Ref.

Sent from my iPhone

On Nov 4, 2015, at 1:14 AM, dylanrtt notifications@github.com wrote:

I thought on this some more. When a child updates the references scope, what is the point in setting that value back on the same child that caused the update if the references scope can't possibly have changed the value because it can't have setters?

In general, I think it is important to prevent unnecessary updates as much as possible because I have run into valid situations where unexpected updates have instigated complex logic.


Reply to this email directly or view it on GitHub.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment