fix(core): ensure ngFor only inserts/moves/removes elements when nece…

…ssary

Closes angular#9960
Closes angular#7239
Closes angular#9672
Closes angular#9454
Closes angular#10287

modules/@angular/common/src/directives/ng_for.ts

@@ -117,24 +117,23 @@ export class NgFor implements DoCheck, OnChanges {
   }
 
   private _applyChanges(changes: DefaultIterableDiffer) {
-    // TODO(rado): check if change detection can produce a change record that is
-    // easier to consume than current.
-    const recordViewTuples: RecordViewTuple[] = [];
-    changes.forEachRemovedItem(
-        (removedRecord: CollectionChangeRecord) =>
-            recordViewTuples.push(new RecordViewTuple(removedRecord, null)));
-
-    changes.forEachMovedItem(
-        (movedRecord: CollectionChangeRecord) =>
-            recordViewTuples.push(new RecordViewTuple(movedRecord, null)));
-
-    const insertTuples = this._bulkRemove(recordViewTuples);
-
-    changes.forEachAddedItem(
-        (addedRecord: CollectionChangeRecord) =>
-            insertTuples.push(new RecordViewTuple(addedRecord, null)));
-
-    this._bulkInsert(insertTuples);
+    const insertTuples: RecordViewTuple[] = [];
+    changes.forEachOperation(
+        (item: CollectionChangeRecord, adjustedPreviousIndex: number, currentIndex: number) => {
+          if (item.previousIndex == null) {
+            let view = this._viewContainer.createEmbeddedView(
+                this._templateRef, new NgForRow(null, null, null), currentIndex);
+            let tuple = new RecordViewTuple(item, view);
+            insertTuples.push(tuple);
+          } else if (currentIndex == null) {
+            this._viewContainer.remove(adjustedPreviousIndex);
+          } else {
+            let view = this._viewContainer.get(adjustedPreviousIndex);
+            this._viewContainer.move(view, currentIndex);
+            let tuple = new RecordViewTuple(item, <EmbeddedViewRef<NgForRow>>view);
+            insertTuples.push(tuple);
+          }
+        });
 
     for (let i = 0; i < insertTuples.length; i++) {
       this._perViewChange(insertTuples[i].view, insertTuples[i].record);
@@ -155,39 +154,6 @@ export class NgFor implements DoCheck, OnChanges {
   private _perViewChange(view: EmbeddedViewRef<NgForRow>, record: CollectionChangeRecord) {
     view.context.$implicit = record.item;
   }
-
-  private _bulkRemove(tuples: RecordViewTuple[]): RecordViewTuple[] {
-    tuples.sort(
-        (a: RecordViewTuple, b: RecordViewTuple) =>
-            a.record.previousIndex - b.record.previousIndex);
-    const movedTuples: RecordViewTuple[] = [];
-    for (let i = tuples.length - 1; i >= 0; i--) {
-      const tuple = tuples[i];
-      // separate moved views from removed views.
-      if (isPresent(tuple.record.currentIndex)) {
-        tuple.view =
-            <EmbeddedViewRef<NgForRow>>this._viewContainer.detach(tuple.record.previousIndex);
-        movedTuples.push(tuple);
-      } else {
-        this._viewContainer.remove(tuple.record.previousIndex);
-      }
-    }
-    return movedTuples;
-  }
-
-  private _bulkInsert(tuples: RecordViewTuple[]): RecordViewTuple[] {
-    tuples.sort((a, b) => a.record.currentIndex - b.record.currentIndex);
-    for (let i = 0; i < tuples.length; i++) {
-      var tuple = tuples[i];
-      if (isPresent(tuple.view)) {
-        this._viewContainer.insert(tuple.view, tuple.record.currentIndex);
-      } else {
-        tuple.view = this._viewContainer.createEmbeddedView(
-            this._templateRef, new NgForRow(null, null, null), tuple.record.currentIndex);
-      }
-    }
-    return tuples;
-  }
 }
 
 class RecordViewTuple {

modules/@angular/core/src/change_detection/differs/default_iterable_differ.ts

@@ -63,6 +63,56 @@ export class DefaultIterableDiffer implements IterableDiffer {
     }
   }
 
+  forEachOperation(
+      fn: (item: CollectionChangeRecord, previousIndex: number, currentIndex: number) => void) {
+    var nextIt = this._itHead;
+    var nextRemove = this._removalsHead;
+    var addRemoveOffset = 0;
+    var moveOffsets: number[] = null;
+    while (nextIt || nextRemove) {
+      // Figure out which is the next record to process
+      // Order: remove, add, move
+      let record = !nextRemove ||
+              nextIt &&
+                  nextIt.currentIndex < getPreviousIndex(nextRemove, addRemoveOffset, moveOffsets) ?
+          nextIt :
+          nextRemove;
+      var adjPreviousIndex = getPreviousIndex(record, addRemoveOffset, moveOffsets);
+      var currentIndex = record.currentIndex;
+
+      // consume the item, and adjust the addRemoveOffset and update moveDistance if necessary
+      if (record === nextRemove) {
+        addRemoveOffset--;
+        nextRemove = nextRemove._nextRemoved;
+      } else {
+        nextIt = nextIt._next;
+        if (record.previousIndex == null) {
+          addRemoveOffset++;
+        } else {
+          // INVARIANT:  currentIndex < previousIndex
+          if (!moveOffsets) moveOffsets = [];
+          let localMovePreviousIndex = adjPreviousIndex - addRemoveOffset;
+          let localCurrentIndex = currentIndex - addRemoveOffset;
+          if (localMovePreviousIndex != localCurrentIndex) {
+            for (var i = 0; i < localMovePreviousIndex; i++) {
+              var offset = i < moveOffsets.length ? moveOffsets[i] : (moveOffsets[i] = 0);
+              var index = offset + i;
+              if (localCurrentIndex <= index && index < localMovePreviousIndex) {
+                moveOffsets[i] = offset + 1;
+              }
+            }
+            var previousIndex = record.previousIndex;
+            moveOffsets[previousIndex] = localCurrentIndex - localMovePreviousIndex;
+          }
+        }
+      }
+
+      if (adjPreviousIndex !== currentIndex) {
+        fn(record, adjPreviousIndex, currentIndex);
+      }
+    }
+  }
+
   forEachPreviousItem(fn: Function) {
     var record: CollectionChangeRecord;
     for (record = this._previousItHead; record !== null; record = record._nextPrevious) {
@@ -700,3 +750,13 @@ class _DuplicateMap {
 
   toString(): string { return '_DuplicateMap(' + stringify(this.map) + ')'; }
 }
+
+function getPreviousIndex(item: any, addRemoveOffset: number, moveOffsets: number[]): number {
+  var previousIndex = item.previousIndex;
+  if (previousIndex === null) return previousIndex;
+  var moveOffset = 0;
+  if (moveOffsets && previousIndex < moveOffsets.length) {
+    moveOffset = moveOffsets[previousIndex];
+  }
+  return previousIndex + addRemoveOffset + moveOffset;
+}

modules/@angular/core/src/linker/element.ts

@@ -60,6 +60,30 @@ export class AppElement {
     return result;
   }
 
+  moveView(view: AppView<any>, currentIndex: number) {
+    var previousIndex = this.nestedViews.indexOf(view);
+    if (view.type === ViewType.COMPONENT) {
+      throw new BaseException(`Component views can't be moved!`);
+    }
+    var nestedViews = this.nestedViews;
+    if (nestedViews == null) {
+      nestedViews = [];
+      this.nestedViews = nestedViews;
+    }
+    ListWrapper.removeAt(nestedViews, previousIndex);
+    ListWrapper.insert(nestedViews, currentIndex, view);
+    var refRenderNode: any /** TODO #9100 */;
+    if (currentIndex > 0) {
+      var prevView = nestedViews[currentIndex - 1];
+      refRenderNode = prevView.lastRootNode;
+    } else {
+      refRenderNode = this.nativeElement;
+    }
+    if (isPresent(refRenderNode)) {
+      view.renderer.attachViewAfter(refRenderNode, view.flatRootNodes);
+    }
+    view.markContentChildAsMoved(this);
+  }
 
   attachView(view: AppView<any>, viewIndex: number) {
     if (view.type === ViewType.COMPONENT) {

modules/@angular/core/src/linker/view.ts

@@ -288,6 +288,8 @@ export abstract class AppView<T> {
     }
   }
 
+  markContentChildAsMoved(renderAppElement: AppElement): void { this.dirtyParentQueriesInternal(); }
+
   addToContentChildren(renderAppElement: AppElement): void {
     renderAppElement.parentView.contentChildren.push(this);
     this.viewContainerElement = renderAppElement;

modules/@angular/core/src/linker/view_container_ref.ts

@@ -99,6 +99,13 @@ export abstract class ViewContainerRef {
    */
   abstract insert(viewRef: ViewRef, index?: number): ViewRef;
 
+  /**
+   * Moves a View identified by a {@link ViewRef} into the container at the specified `index`.
+   *
+   * Returns the inserted {@link ViewRef}.
+   */
+  abstract move(viewRef: ViewRef, currentIndex: number): ViewRef;
+
   /**
    * Returns the index of the View, specified via {@link ViewRef}, within the current container or
    * `-1` if this container doesn't contain the View.
@@ -170,6 +177,14 @@ export class ViewContainerRef_ implements ViewContainerRef {
     return wtfLeave(s, viewRef_);
   }
 
+  move(viewRef: ViewRef, currentIndex: number): ViewRef {
+    var s = this._insertScope();
+    if (currentIndex == -1) return;
+    var viewRef_ = <ViewRef_<any>>viewRef;
+    this._element.moveView(viewRef_.internalView, currentIndex);
+    return wtfLeave(s, viewRef_);
+  }
+
   indexOf(viewRef: ViewRef): number {
     return ListWrapper.indexOf(this._element.nestedViews, (<ViewRef_<any>>viewRef).internalView);
   }