-
Notifications
You must be signed in to change notification settings - Fork 1.5k
descendant bindings of 'foreach' are evaluated when corresponding item in array is removed #2305
Description
Documentation states: "When you delete array entries, foreach will simply remove the corresponding DOM elements." However, it appears that in some cases the bindings in the corresponding DOM elements are re-evaluated before the DOM elements are removed. And when the bindings are re-evaluated, they are evaluated with the old/previous values of $index (and $data) even though the array in the parent foreach has been updated with new values.
The net effect is that the bindings can be evaluated with an out-of-bound $index value - a value that is greater than the length of the array.
Compare this to how the if binding behaves (after ##2226 and #2234).
The descendant bindings of if now have a dependency on the condition of the if binding causing the condition of the if bindning to be evaluated before the descendants. If the condition is false then the descendants are removed and not re-evaluated.
I think the behavior of the foreach binding should be something similar. If the array is replaced with new (fewer) items then the corresponding descendants should be removed and not re-evaluated before being removed.
Normally that happens but I have a situation where the descendants are re-evaluated before being removed, causing an error.
In my case, I have a view model with an observableArray phrases (used in the foreach). The view model
also maintains another array, translatedPhrases - a one-to-one mapping of phrases using computed observable.
A foreach binding is used to render the phrases. While rendering each phrase, the corresponding translated phrase is also rendered by accessing it from translatedPhrases using $index.
function ViewModel() {
this.phrases = ko.observableArray();
this.translatedPhrases = ko.pureComputed(function(){
return this.phrases().map(function(phrase){ return { phrase: phrase.toUpperCase() }; });
}, this);
}
...
<!-- ko if: translatedPhrases --><!-- /ko -->
<!-- ko foreach: phrases -->
<span data-bind="text: $data"></span>
<span data-bind="text: $parent.translatedPhrases()[$index()].phrase"></span>
<!-- /ko -->
The problem happens when phrases is replaced with another array that is shorter than the previous one.
Then, under some circumstances, the descendants of foreach gets re-evaluated before they are removed and they are evaluated with values of $index (and $data) of the previous array.
http://jsfiddle.net/fastfasterfastest/dg5dmbyL/ demonstrates the problem - click "New Phrases" to set phrases to a new, shorter, array to demonstrate the problem.
http://jsfiddle.net/fastfasterfastest/85ptgv6f/ is very similar and uses a "log" to show more details.
Note, one "circumstance" that triggers this is if translatedPhrases is referenced prior to the foreach.
If translatedPhrases is not referenced prior to the foreach then the bindings in the corresponding descendants are not re-evaluated and things "work".
If translatedPhrases is referenced prior to the foreach then the bindings in the corresponding descendants are re-evaluated before they are removed causing an error.
I use a if binding in my fiddle to reference translatedPhrases - remove it and things work.
If using 3.4.2, then the value of ko.options.deferUpdates plays a role - if false then things "work", if true then errors.
If using knockout-latest.debug.js, one of @mbest's builds he made available in another issue a while ago, then the value of ko.options.deferUpdates does not matter - errors whether true or false.
So something appear to have happened with the behavior of foreach using deferUpdates between these two versions.