fix(tests): update/add tests, add comments, rename variables for better readability

Author: bigopon

src/array-virtual-repeat-strategy.ts

@@ -46,7 +46,7 @@ export class ArrayVirtualRepeatStrategy extends ArrayRepeatStrategy implements I
   }
 
   /**@internal */
-  _inPlaceProcessItems(repeat: VirtualRepeat, items: any[], first: number): void {
+  _inPlaceProcessItems(repeat: VirtualRepeat, items: any[], firstIndex: number): void {
     const currItemCount = items.length;
     if (currItemCount === 0) {
       repeat.removeAllViews(/*return to cache?*/true, /*skip animation?*/false);
@@ -70,17 +70,17 @@ export class ArrayVirtualRepeatStrategy extends ArrayRepeatStrategy implements I
     }
     const local = repeat.local;
     const lastIndex = currItemCount - 1;
-    if (first + realViewsCount > lastIndex) {
+    if (firstIndex + realViewsCount > lastIndex) {
       // first = currItemCount - realViewsCount instead of: first = currItemCount - 1 - realViewsCount;
       //    this is because during view update
       //    view(i) starts at 0 and ends at less than last
-      first = Math.max(0, currItemCount - realViewsCount);
+      firstIndex = Math.max(0, currItemCount - realViewsCount);
     }
 
-    repeat._first = first;
+    repeat._first = firstIndex;
     // re-evaluate bindings on existing views.
     for (let i = 0; i < realViewsCount; i++) {
-      const currIndex = i + first;
+      const currIndex = i + firstIndex;
       const view = repeat.view(i);
       const last = currIndex === currItemCount - 1;
       const middle = currIndex !== 0 && !last;

src/virtual-repeat.ts

@@ -70,7 +70,7 @@ export class VirtualRepeat extends AbstractRepeater {
    * @internal
    * First view index, for proper follow up calculations
    */
-  _first = 0;
+  _first: number = 0;
 
   /**
    * @internal
@@ -376,6 +376,8 @@ export class VirtualRepeat extends AbstractRepeater {
    * - handle scroll as if scroll event happened
    */
   itemsChanged(): void {
+    // the current collection subscription may be irrelevant
+    // unsubscribe and resubscribe later
     this._unsubscribeCollection();
     // still bound? and still attached?
     if (!this.scope || !this._isAttached) {
@@ -385,11 +387,11 @@ export class VirtualRepeat extends AbstractRepeater {
     let previousLastViewIndex = this._getIndexOfLastView();
 
     const items = this.items;
-    const currentItemCount = items.length;
     const shouldCalculateSize = !!items;
     const strategy = this.strategy = this.strategyLocator.getStrategy(items);
-
+    
     if (shouldCalculateSize) {
+      const currentItemCount = items.length;
       if (currentItemCount > 0 && this.viewCount() === 0) {
         strategy.createFirstItem(this);
       }
@@ -412,6 +414,7 @@ export class VirtualRepeat extends AbstractRepeater {
     strategy.instanceChanged(this, items, this._first);
 
     if (shouldCalculateSize) {
+      const currentItemCount = items.length;
       // Reset rebinding
       this._lastRebind = this._first;
 
@@ -534,31 +537,31 @@ export class VirtualRepeat extends AbstractRepeater {
       this._skipNextScrollHandle = false;
       return;
     }
-    if (!this.items) {
+    const items = this.items;
+    if (!items) {
       return;
     }
-    let itemHeight = this.itemHeight;
-    let scrollTop = this._fixedHeightContainer
+    const itemHeight = this.itemHeight;
+    const scrollTop = this._fixedHeightContainer
       ? this.scrollContainer.scrollTop
       : (pageYOffset - this.distanceToTop);
 
     // Calculate the index of first view
     // Using Math floor to ensure it has correct space for both small and large calculation
-    let firstViewIndex = itemHeight > 0 ? Math.floor(scrollTop / itemHeight) : 0;
-    this._first = firstViewIndex < 0 ? 0 : firstViewIndex;
+    let firstIndex = itemHeight > 0 ? Math.floor(scrollTop / itemHeight) : 0;
+    this._first = firstIndex < 0 ? 0 : firstIndex;
     // if first index starts somewhere after the last view
     // then readjust based on the delta
-    if (this._first > this.items.length - this.elementsInView) {
-      firstViewIndex = this.items.length - this.elementsInView;
-      this._first = firstViewIndex < 0 ? 0 : firstViewIndex;
+    if (firstIndex > items.length - this.elementsInView) {
+      this._first = Math.max(0, items.length - this.elementsInView);
     }
 
     // Check scrolling states and adjust flags
     this._checkScrolling();
 
     // store buffers' heights into local variables
-    let currentTopBufferHeight = this._topBufferHeight;
-    let currentBottomBufferHeight = this._bottomBufferHeight;
+    const currentTopBufferHeight = this._topBufferHeight;
+    const currentBottomBufferHeight = this._bottomBufferHeight;
 
     // TODO if and else paths do almost same thing, refactor?
     if (this._scrollingDown) {

test/interfaces.ts

@@ -0,0 +1,3 @@
+export declare class ITestAppInterface<T> {
+  items: T[];
+}

test/utilities.ts

@@ -128,7 +128,9 @@ export async function scrollToIndex(virtualRepeat: VirtualRepeat, itemIndex: num
 }
 
 /**
- * Wait for a small time for repeat to finish processing
+ * Wait for a small time for repeat to finish processing.
+ * 
+ * Default to 10
  */
 export async function ensureScrolled(time: number = 10): Promise<void> {
   await waitForTimeout(time);

test/virtual-repeat-integration.spec.ts

@@ -1,7 +1,7 @@
 import './setup';
 import { StageComponent } from './component-tester';
 import { PLATFORM } from 'aurelia-pal';
-import { createAssertionQueue, validateState, validateScrolledState, AsyncQueue } from './utilities';
+import { createAssertionQueue, validateState, validateScrolledState, AsyncQueue, waitForTimeout } from './utilities';
 import { VirtualRepeat } from '../src/virtual-repeat';
 
 PLATFORM.moduleName('src/virtual-repeat');
@@ -123,7 +123,7 @@ describe('VirtualRepeat Integration', () => {
 
   describe('iterating div', () => {
     let component;
-    let virtualRepeat;
+    let virtualRepeat: VirtualRepeat;
     let viewModel;
     let create;
     let items;
@@ -308,19 +308,17 @@ describe('VirtualRepeat Integration', () => {
       create.then(() => validateArrayChange(virtualRepeat, viewModel, done));
     });
 
-    it('handles array changes with null / undefined', done => {
-      create.then(() => {
-        viewModel.items = null;
+    it('handles array changes with null / undefined', async (done) => {
+      await create;
+      viewModel.items = null;
+      await waitForTimeout(50);
 
-        setTimeout(() => {
-          let topBufferHeight = virtualRepeat.topBuffer.getBoundingClientRect().height;
-          let bottomBufferHeight = virtualRepeat.bottomBuffer.getBoundingClientRect().height;
+      let topBufferHeight = virtualRepeat.topBufferEl.getBoundingClientRect().height;
+      let bottomBufferHeight = virtualRepeat.bottomBufferEl.getBoundingClientRect().height;
 
-          expect(topBufferHeight + bottomBufferHeight).toBe(0);
+      expect(topBufferHeight + bottomBufferHeight).toBe(0);
 
-          validateArrayChange(virtualRepeat, viewModel, done);
-        }, 1000);
-      });
+      validateArrayChange(virtualRepeat, viewModel, done);
     });
   });