Skip to content

Binding context incorrect when using `with` and `$parent` #1

Closed
drosen0 opened this Issue Mar 6, 2012 · 10 comments

2 participants

@drosen0
drosen0 commented Mar 6, 2012

I made a fiddle to illustrate the issue:
http://jsfiddle.net/drosen/RRVrQ/

Update: Here's an updated fiddle:
http://jsfiddle.net/drosen/RRVrQ/8/

@mbest
Owner
mbest commented Mar 6, 2012

When used with Knockout 2.0.0, your fiddle will work: http://jsfiddle.net/mbest/RRVrQ/4/.

The difference when using the latest Knockout is that I'm using the new ko.bindingContext.extend function, which creates a child context (but with $data == $parent). This wasn't how I thought it should work, but see issue 290 where Steve and I discussed this topic in depth. To make your code work with the new version, change $parent to $parents[1]: http://jsfiddle.net/mbest/RRVrQ/5/

@drosen0
drosen0 commented Mar 7, 2012

So, the first parent is the case binding and the second is the switch binding? I wonder if Steve would reconsider that extensibility point, seeing how it affects consistency in this multi-stage binding. If not, it might help others to have a note about this in the documentation.

@mbest
Owner
mbest commented Mar 7, 2012

No, case doesn't create a child context; only switch does. There are two parents because with also creates a child context:

  1. Top level: $data = $root = vm
  2. with binding: $data = vm.sub, $parents[0] = $parent = $root = vm
  3. switch binding: $data = $parent = $parents[0] = vm.sub, $parents[1] = $root = vm

I still think it's confusing to have the extra "parent" and I wonder if it might be better to consider that a child context that doesn't change the value of $data should also not change $parent or $parents.

@mbest
Owner
mbest commented Mar 7, 2012

I submitted a pull request for Knockout to change bindingContext.extend to keep $parent the same.

@drosen0
drosen0 commented Mar 7, 2012

Okay, now I understand your first comment better. I think this counts as a real world use case where we would benefit from that change, so maybe Steve will change his mind. Thank you for the explanation.

@drosen0
drosen0 commented Mar 7, 2012

For anyone reading here from the pull request, I modified the fiddle to be easier to follow, and added this fiddle to the original post.

@mbest
Owner
mbest commented Mar 17, 2012

The latest Knockout code has been changed so that bindingContext.extend works the way it should. We'll have to wait for Steve to re-build it before we can see it working though.

@mbest mbest was assigned Mar 17, 2012
@drosen0
drosen0 commented Mar 17, 2012

That's good news. I'm looking forward to testing the next build.

@mbest
Owner
mbest commented Mar 20, 2012

Okay the build is updated. I tested your fiddle and had to take out the $parents[1].name bindings: http://jsfiddle.net/RRVrQ/9/

@mbest mbest closed this Mar 20, 2012
@drosen0
drosen0 commented Mar 20, 2012

Looks good to me. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.