fix(ivy): node placed in incorrect order inside ngFor with ng-contain…

…er (angular#32324)

Fixes an issue where Ivy incorrectly inserts items in the beginning of an `ngFor`, if the `ngFor` is set on an `ng-container`. The issue comes from the fact that we choose the `ng-container` comment node as the anchor point before which to insert the content, however the node might be after any of the nodes inside the container. These changes switch to picking out the first node inside of the container instead.

PR Close angular#32324

packages/core/src/render3/node_manipulation.ts

@@ -669,12 +669,22 @@ export function getBeforeNodeForView(viewIndexInContainer: number, lContainer: L
   if (nextViewIndex < lContainer.length) {
     const lView = lContainer[nextViewIndex] as LView;
     ngDevMode && assertDefined(lView[T_HOST], 'Missing Host TNode');
-    const tViewNodeChild = (lView[T_HOST] as TViewNode).child;
-    return tViewNodeChild !== null ? getNativeByTNodeOrNull(tViewNodeChild, lView) :
-                                     lContainer[NATIVE];
-  } else {
-    return lContainer[NATIVE];
+    let tViewNodeChild = (lView[T_HOST] as TViewNode).child;
+    if (tViewNodeChild !== null) {
+      if (tViewNodeChild.type === TNodeType.ElementContainer ||
+          tViewNodeChild.type === TNodeType.IcuContainer) {
+        let currentChild = tViewNodeChild.child;
+        while (currentChild && (currentChild.type === TNodeType.ElementContainer ||
+                                currentChild.type === TNodeType.IcuContainer)) {
+          currentChild = currentChild.child;
+        }
+        tViewNodeChild = currentChild || tViewNodeChild;
+      }
+      return getNativeByTNodeOrNull(tViewNodeChild, lView);
+    }
   }
+
+  return lContainer[NATIVE];
 }
 
 /**

packages/core/test/acceptance/view_container_ref_spec.ts

@@ -1276,6 +1276,178 @@ describe('ViewContainerRef', () => {
               '</loop-comp>');
     });
 
+    it('should insert elements in the proper order when template root is an ng-container', () => {
+      @Component({
+        template: `
+          <ng-container *ngFor="let item of items">|{{ item }}|</ng-container>
+        `
+      })
+      class App {
+        items = ['one', 'two', 'three'];
+      }
+
+      TestBed.configureTestingModule({imports: [CommonModule], declarations: [App]});
+      const fixture = TestBed.createComponent(App);
+      fixture.detectChanges();
+
+      expect(fixture.nativeElement.textContent).toBe('|one||two||three|');
+
+      fixture.componentInstance.items.unshift('zero');
+      fixture.detectChanges();
+
+      expect(fixture.nativeElement.textContent).toBe('|zero||one||two||three|');
+
+      fixture.componentInstance.items.push('four');
+      fixture.detectChanges();
+
+      expect(fixture.nativeElement.textContent).toBe('|zero||one||two||three||four|');
+
+      fixture.componentInstance.items.splice(3, 0, 'two point five');
+      fixture.detectChanges();
+
+      expect(fixture.nativeElement.textContent)
+          .toBe('|zero||one||two||two point five||three||four|');
+    });
+
+    it('should insert elements in the proper order when template root is an ng-container and is wrapped by an ng-container',
+       () => {
+         @Component({
+           template: `
+              <ng-container>
+                <ng-container *ngFor="let item of items">|{{ item }}|</ng-container>
+              </ng-container>
+            `
+         })
+         class App {
+           items = ['one', 'two', 'three'];
+         }
+
+         TestBed.configureTestingModule({imports: [CommonModule], declarations: [App]});
+         const fixture = TestBed.createComponent(App);
+         fixture.detectChanges();
+
+         expect(fixture.nativeElement.textContent).toBe('|one||two||three|');
+
+         fixture.componentInstance.items.unshift('zero');
+         fixture.detectChanges();
+
+         expect(fixture.nativeElement.textContent).toBe('|zero||one||two||three|');
+
+         fixture.componentInstance.items.push('four');
+         fixture.detectChanges();
+
+         expect(fixture.nativeElement.textContent).toBe('|zero||one||two||three||four|');
+
+         fixture.componentInstance.items.splice(3, 0, 'two point five');
+         fixture.detectChanges();
+
+         expect(fixture.nativeElement.textContent)
+             .toBe('|zero||one||two||two point five||three||four|');
+       });
+
+    it('should insert elements in the proper order when template root is an ng-container and first node is a ng-container',
+       () => {
+         @Component({
+           template: `
+            <ng-container *ngFor="let item of items"><ng-container>|{{ item }}|</ng-container></ng-container>
+          `
+         })
+         class App {
+           items = ['one', 'two', 'three'];
+         }
+
+         TestBed.configureTestingModule({imports: [CommonModule], declarations: [App]});
+         const fixture = TestBed.createComponent(App);
+         fixture.detectChanges();
+
+         expect(fixture.nativeElement.textContent).toBe('|one||two||three|');
+
+         fixture.componentInstance.items.unshift('zero');
+         fixture.detectChanges();
+
+         expect(fixture.nativeElement.textContent).toBe('|zero||one||two||three|');
+
+         fixture.componentInstance.items.push('four');
+         fixture.detectChanges();
+
+         expect(fixture.nativeElement.textContent).toBe('|zero||one||two||three||four|');
+
+         fixture.componentInstance.items.splice(3, 0, 'two point five');
+         fixture.detectChanges();
+
+         expect(fixture.nativeElement.textContent)
+             .toBe('|zero||one||two||two point five||three||four|');
+       });
+
+    it('should insert elements in the proper order when template root is an ng-container, wrapped in an ng-container with the root node as an ng-container',
+       () => {
+         @Component({
+           template: `
+            <ng-container>
+              <ng-container *ngFor="let item of items"><ng-container>|{{ item }}|</ng-container></ng-container>
+            </ng-container>
+          `
+         })
+         class App {
+           items = ['one', 'two', 'three'];
+         }
+
+         TestBed.configureTestingModule({imports: [CommonModule], declarations: [App]});
+         const fixture = TestBed.createComponent(App);
+         fixture.detectChanges();
+
+         expect(fixture.nativeElement.textContent).toBe('|one||two||three|');
+
+         fixture.componentInstance.items.unshift('zero');
+         fixture.detectChanges();
+
+         expect(fixture.nativeElement.textContent).toBe('|zero||one||two||three|');
+
+         fixture.componentInstance.items.push('four');
+         fixture.detectChanges();
+
+         expect(fixture.nativeElement.textContent).toBe('|zero||one||two||three||four|');
+
+         fixture.componentInstance.items.splice(3, 0, 'two point five');
+         fixture.detectChanges();
+
+         expect(fixture.nativeElement.textContent)
+             .toBe('|zero||one||two||two point five||three||four|');
+       });
+
+    it('should insert elements in the proper order when the first child node is an ICU expression',
+       () => {
+         @Component({
+           template: `
+          <ng-container *ngFor="let item of items">{count, select, other {|{{ item }}|}}</ng-container>
+        `
+         })
+         class App {
+           items = ['one', 'two', 'three'];
+         }
+
+         TestBed.configureTestingModule({imports: [CommonModule], declarations: [App]});
+         const fixture = TestBed.createComponent(App);
+         fixture.detectChanges();
+
+         expect(fixture.nativeElement.textContent).toBe('|one||two||three|');
+
+         fixture.componentInstance.items.unshift('zero');
+         fixture.detectChanges();
+
+         expect(fixture.nativeElement.textContent).toBe('|zero||one||two||three|');
+
+         fixture.componentInstance.items.push('four');
+         fixture.detectChanges();
+
+         expect(fixture.nativeElement.textContent).toBe('|zero||one||two||three||four|');
+
+         fixture.componentInstance.items.splice(3, 0, 'two point five');
+         fixture.detectChanges();
+
+         expect(fixture.nativeElement.textContent)
+             .toBe('|zero||one||two||two point five||three||four|');
+       });
   });
 
   describe('lifecycle hooks', () => {