Changes to the valueAccessor for the foreach binding aren't picked up

[papandreou]

In my app I have a view where the foreach binding is used with an observable array that's replaced when the user navigates somewhere else. Something like this:

<div data-bind="foreach: state.foo().entries, selection: {data: selection}">

This construct works fine with foreach, but knockout.selection doesn't react to it when state.foo().entries evaluates to a different observableArray instance. It keeps working on the old observable array, leading to quite bizarre results.

As far as I can tell, supporting this would require knockout.selection to have an update handler that checks whether allBindingsAccessor().foreach.data has changed and updates its state accordingly. I've verified that update does fire under those circumstances.

This will probably require saving a lot of the state that's currently kept in variables inside init to be moved somewhere so it can be accessed from update.

I've added some failing tests here: https://github.com/papandreou/knockout.selection/tree/dynamic_foreach

Ping @sunesimonsen -- this doesn't appear to be addressed by https://github.com/sunesimonsen/knockout.selection/tree/refactoring yet. Is it something that you were planning to fix?

[sunesimonsen]

@papandreou the refactoring branch is no longer relevant as it has been merged some time ago. I have delete the branch to avoid confusion in the future.

About monitoring when the data observable of the foreach binding is updated, I think it would be possible to just subscribe to changes on the observable. Just remember to check the it is actually observable and also remember to dispose the subscription when the element is removed from the DOM.

Something along the lines:

var value = valueAccessor();
if (ko.isObservable(value)) {
    var forEachSubscription = value.subscribe(function (data) {
        // Do stuff
    });

    ko.utils.domNodeDisposal.addDisposeCallback(element, function () {
        forEachSubscription.dispose();
    });
}

[mwoc]

@papandreou Perhaps this could be solved in the same subscription you added in this commit: https://github.com/papandreou/knockout.selection/commit/0205083

Something like: https://github.com/mwoc/knockout.selection/commit/2f39367

It's tested with my production code in which the foreach can both be replaced completely (change to different data set), or only to some degree (sorting/filtering/pagination of same data set, in which some selected items may stay in the foreach and others are not). Have not been able to get my head around writing unit tests for this though, going to bed now.

[papandreou]

@mwoc Unfortunately that subscription only fires when the contents of the previously bound observable array changes, but not at the moment when the foreach expression evaluates to a different observable array.

Hopefully it's just a matter of subscribing to allBindingsAccessor and reinitialize if allBindingsAccessor().foreach evaluates to a different observable array instance like Sune suggested.

[sunesimonsen]

@papandreou you are right, I of cause meant subscribing to the foreach variable in the ko.bindingHandlers.selection.init function - my bad.

[bramstein]

That indeed sounds like the right approach. @papandreou could you do a pull request for those failing tests. Would be nice to have the tests before we start working on the implementation.

[papandreou]

@bramstein Absolutely: https://github.com/bramstein/knockout.selection/pull/25

[papandreou]

I ended up working around the bug by wrapping my view in a with binding:

<!--ko 'with': state.foo()-->
<div data-bind="foreach: entries, selection: {...}">
   ...
</div>
<!--/ko-->

Would still be nice to get this fixed, of course.