Reducing the number of items in the array will not render correctly

[altenstedt]

I'm submitting a bug report

• Library Version:
  1.0.0-beta.3.0.2

Please tell us about your environment:

• Operating System:
  Windows 10

• Node Version:
  4.5.0

• NPM Version:
  3.8.9

• JSPM OR Webpack AND Version
  JSPM 0.16.32

• Browser:
  all

• Language:
  all

Current behavior:

When reducing the number of items in the array, virtual-repeat.for will not render items correctly.

Expected/desired behavior:

The code in ArrayVirtualRepeatStrategy._inPlaceProcessItems does not seem to properly handle the case when a new array is bound to the virtual-repeat attribute that has fewer items than the current view
displays.

So, for example, if I have a view like so:

<div repeat.for="item of items"></div>

And update the items array in the view model like so:

this.items = array;

Notice that I re-bind to a new array. I am not using a value converter. This cause VirtualRepeat.itemsChanged to run.

Where array has fewer items that the previous value of this.items, then the code that access the items array around line 46 (items[i + first]) might access items outside the bounds of the new array, if the user is scrolled down in the view.

I do not understand the code enough to provide a fix, but I am willing to put in an effort given a little guidance. This module is very useful for us.

[altenstedt]

A naive update, like adding the following line just before re-evaluating the existing bindings:

first = Math.min(first, itemsLength - viewsLength);

does not fully resolve this issue. I think I will wait for the pending pull requests 96 and 101 to be merged, as it all seem somewhat related.

[AStoker]

@EisenbergEffect, can you assign this issue to me?

[AStoker]

@altenstedt, more than just providing a correct first property, we need to update the $index as well (which leads me to believe there might be some other binding data that I need to ensure is modified). Looking into it now.

[Thanood]

Could this be related to #60? I'm hoping... 😃
At least the workaround looks similar.

[AStoker]

@Thanood, it is most likely related. I'm digging into some of the bugs in the virtualization list, and there does seem to be some bugs in how it handles mutation of arrays (which includes arrays with filters).

[AStoker]

I have it so the indexes are all correct now. The last little bit is getting the scrolling scenario under wraps.
What can happen is you scroll to the end of a short list where we are at the bottom of our buffer heights, and we have our excess elements above our view space. When that is expanded into a long list, we want to move those excess elements that were above our list to be below (like it normally is while scrolling), and we want to move them while still keeping our position in the scroll space. Have a few ideas on it, but getting close and wanted to give an update.

[bdacoderchris]

@AStoker we are experiencing a similar issue of certain bindings not updating until you refresh the page when an item is removed from our array. What is the status on your potential release?

[AStoker]

@bdacoderchris, I think I have the solution, but I need to finish writing some tests and fixing one that is failing. I'm out for the weekend (no internet where I'm going). Best case is I get the PR in the next few hours, otherwise probably Monday morning.

[AStoker]

Resolved (hopefully) by #107