fix: allow context of EmbeddedViewRef to be changed and avoid mutating context in NgTemplateOutlet

goldens/public-api/core/core.d.ts

@@ -301,7 +301,7 @@ export declare class ElementRef<T = any> {
 }
 
 export declare abstract class EmbeddedViewRef<C> extends ViewRef {
-    abstract get context(): C;
+    abstract context: C;
     abstract get rootNodes(): any[];
 }
 

goldens/size-tracking/aio-payloads.json

@@ -26,4 +26,4 @@
       }
     }
   }
-}
+}

packages/common/src/directives/ng_template_outlet.ts

@@ -52,9 +52,7 @@ export class NgTemplateOutlet implements OnChanges {
   constructor(private _viewContainerRef: ViewContainerRef) {}
 
   ngOnChanges(changes: SimpleChanges) {
-    const recreateView = this._shouldRecreateView(changes);
-
-    if (recreateView) {
+    if (changes['ngTemplateOutlet']) {
       const viewContainerRef = this._viewContainerRef;
 
       if (this._viewRef) {
@@ -64,44 +62,9 @@ export class NgTemplateOutlet implements OnChanges {
       this._viewRef = this.ngTemplateOutlet ?
           viewContainerRef.createEmbeddedView(this.ngTemplateOutlet, this.ngTemplateOutletContext) :
           null;
-    } else if (this._viewRef && this.ngTemplateOutletContext) {
-      this._updateExistingContext(this.ngTemplateOutletContext);
-    }
-  }
-
-  /**
-   * We need to re-create existing embedded view if:
-   * - templateRef has changed
-   * - context has changes
-   *
-   * We mark context object as changed when the corresponding object
-   * shape changes (new properties are added or existing properties are removed).
-   * In other words we consider context with the same properties as "the same" even
-   * if object reference changes (see https://github.com/angular/angular/issues/13407).
-   */
-  private _shouldRecreateView(changes: SimpleChanges): boolean {
-    const ctxChange = changes['ngTemplateOutletContext'];
-    return !!changes['ngTemplateOutlet'] || (ctxChange && this._hasContextShapeChanged(ctxChange));
-  }
-
-  private _hasContextShapeChanged(ctxChange: SimpleChange): boolean {
-    const prevCtxKeys = Object.keys(ctxChange.previousValue || {});
-    const currCtxKeys = Object.keys(ctxChange.currentValue || {});
-
-    if (prevCtxKeys.length === currCtxKeys.length) {
-      for (let propName of currCtxKeys) {
-        if (prevCtxKeys.indexOf(propName) === -1) {
-          return true;
-        }
-      }
-      return false;
-    }
-    return true;
-  }
-
-  private _updateExistingContext(ctx: Object): void {
-    for (let propName of Object.keys(ctx)) {
-      (<any>this._viewRef!.context)[propName] = (<any>this.ngTemplateOutletContext)[propName];
+    } else if (
+        this._viewRef && changes['ngTemplateOutletContext'] && this.ngTemplateOutletContext) {
+      this._viewRef.context = this.ngTemplateOutletContext;
     }
   }
 }

packages/common/test/directives/ng_template_outlet_spec.ts

@@ -29,7 +29,7 @@ describe('NgTemplateOutlet', () => {
 
   beforeEach(() => {
     TestBed.configureTestingModule({
-      declarations: [TestComponent, CaptureTplRefs, DestroyableCmpt],
+      declarations: [TestComponent, CaptureTplRefs, DestroyableCmpt, MultiContextComponent],
       imports: [CommonModule],
       providers: [DestroyedSpyService]
     });
@@ -142,7 +142,7 @@ describe('NgTemplateOutlet', () => {
     expect(spyService.destroyed).toBeFalsy();
   });
 
-  it('should recreate embedded view when context shape changes', () => {
+  it('should update but not destroy embedded view when context shape changes', () => {
     const template =
         `<ng-template let-foo="foo" #tpl><destroyable-cmpt></destroyable-cmpt>:{{foo}}</ng-template>` +
         `<ng-template [ngTemplateOutlet]="tpl" [ngTemplateOutletContext]="context"></ng-template>`;
@@ -155,7 +155,7 @@ describe('NgTemplateOutlet', () => {
 
     fixture.componentInstance.context = {foo: 'baz', other: true};
     detectChangesAndExpectText('Content to destroy:baz');
-    expect(spyService.destroyed).toBeTruthy();
+    expect(spyService.destroyed).toBeFalsy();
   });
 
   it('should destroy embedded view when context value changes and templateRef becomes undefined', () => {
@@ -241,6 +241,27 @@ describe('NgTemplateOutlet', () => {
          detectChangesAndExpectText('foo');
        }).not.toThrow();
      }));
+
+  it('should not mutate context object if two contexts with an identical shape are swapped', () => {
+    fixture = TestBed.createComponent(MultiContextComponent);
+    const {componentInstance, nativeElement} = fixture;
+    componentInstance.context1 = {name: 'one'};
+    componentInstance.context2 = {name: 'two'};
+    fixture.detectChanges();
+
+    expect(nativeElement.textContent.trim()).toBe('one | two');
+    expect(componentInstance.context1).toEqual({name: 'one'});
+    expect(componentInstance.context2).toEqual({name: 'two'});
+
+    const temp = componentInstance.context1;
+    componentInstance.context1 = componentInstance.context2;
+    componentInstance.context2 = temp;
+    fixture.detectChanges();
+
+    expect(nativeElement.textContent.trim()).toBe('two | one');
+    expect(componentInstance.context1).toEqual({name: 'two'});
+    expect(componentInstance.context2).toEqual({name: 'one'});
+  });
 });
 
 @Injectable()
@@ -271,6 +292,19 @@ class TestComponent {
   value = 'bar';
 }
 
+@Component({
+  template: `
+  <ng-template #template let-name="name">{{name}}</ng-template>
+  <ng-template [ngTemplateOutlet]="template" [ngTemplateOutletContext]="context1"></ng-template>
+  |
+  <ng-template [ngTemplateOutlet]="template" [ngTemplateOutletContext]="context2"></ng-template>
+  `
+})
+class MultiContextComponent {
+  context1: {name: string}|undefined;
+  context2: {name: string}|undefined;
+}
+
 function createTestComponent(template: string): ComponentFixture<TestComponent> {
   return TestBed.overrideComponent(TestComponent, {set: {template: template}})
       .configureTestingModule({schemas: [NO_ERRORS_SCHEMA]})

packages/core/src/linker/view_ref.ts

@@ -93,7 +93,7 @@ export abstract class EmbeddedViewRef<C> extends ViewRef {
   /**
    * The context for this view, inherited from the anchor element.
    */
-  abstract get context(): C;
+  abstract context: C;
 
   /**
    * The root nodes for this embedded view.

packages/core/src/render3/view_ref.ts

@@ -61,6 +61,10 @@ export class ViewRef<T> implements viewEngine_EmbeddedViewRef<T>, viewEngine_Int
     return this._lView[CONTEXT] as T;
   }
 
+  set context(value: T) {
+    this._lView[CONTEXT] = value;
+  }
+
   get destroyed(): boolean {
     return (this._lView[FLAGS] & LViewFlags.Destroyed) === LViewFlags.Destroyed;
   }

packages/core/src/view/refs.ts

@@ -264,6 +264,10 @@ export class ViewRef_ implements EmbeddedViewRef<any>, InternalViewRef {
     return this._view.context;
   }
 
+  set context(value: any) {
+    this._view.context = value;
+  }
+
   get destroyed(): boolean {
     return (this._view.state & ViewState.Destroyed) !== 0;
   }

packages/core/test/acceptance/template_ref_spec.ts

@@ -273,4 +273,87 @@ describe('TemplateRef', () => {
          });
     });
   });
+
+  describe('context', () => {
+    @Component({
+      template: `
+      <ng-template #templateRef let-name="name">{{name}}</ng-template>
+      <ng-container #containerRef></ng-container>
+    `
+    })
+    class App {
+      @ViewChild('templateRef') templateRef!: TemplateRef<any>;
+      @ViewChild('containerRef', {read: ViewContainerRef}) containerRef!: ViewContainerRef;
+    }
+
+    it('should update if the context of a view ref is mutated', () => {
+      TestBed.configureTestingModule({declarations: [App]});
+      const fixture = TestBed.createComponent(App);
+      fixture.detectChanges();
+      const context = {name: 'Frodo'};
+      const viewRef = fixture.componentInstance.templateRef.createEmbeddedView(context);
+      fixture.componentInstance.containerRef.insert(viewRef);
+      fixture.detectChanges();
+
+      expect(fixture.nativeElement.textContent).toBe('Frodo');
+
+      context.name = 'Bilbo';
+      fixture.detectChanges();
+
+      expect(fixture.nativeElement.textContent).toBe('Bilbo');
+    });
+
+    it('should update if the context of a view ref is replaced', () => {
+      TestBed.configureTestingModule({declarations: [App]});
+      const fixture = TestBed.createComponent(App);
+      fixture.detectChanges();
+      const viewRef = fixture.componentInstance.templateRef.createEmbeddedView({name: 'Frodo'});
+      fixture.componentInstance.containerRef.insert(viewRef);
+      fixture.detectChanges();
+
+      expect(fixture.nativeElement.textContent).toBe('Frodo');
+
+      viewRef.context = {name: 'Bilbo'};
+      fixture.detectChanges();
+
+      expect(fixture.nativeElement.textContent).toBe('Bilbo');
+    });
+
+    it('should use the latest context information inside template listeners', () => {
+      const events: string[] = [];
+
+      @Component({
+        template: `
+          <ng-template #templateRef let-name="name">
+            <button (click)="log(name)"></button>
+          </ng-template>
+          <ng-container #containerRef></ng-container>
+        `
+      })
+      class ListenerTest {
+        @ViewChild('templateRef') templateRef!: TemplateRef<any>;
+        @ViewChild('containerRef', {read: ViewContainerRef}) containerRef!: ViewContainerRef;
+
+        log(name: string) {
+          events.push(name);
+        }
+      }
+
+      TestBed.configureTestingModule({declarations: [ListenerTest]});
+      const fixture = TestBed.createComponent(ListenerTest);
+      fixture.detectChanges();
+      const viewRef = fixture.componentInstance.templateRef.createEmbeddedView({name: 'Frodo'});
+      fixture.componentInstance.containerRef.insert(viewRef);
+      fixture.detectChanges();
+
+      const button = fixture.nativeElement.querySelector('button');
+      button.click();
+      expect(events).toEqual(['Frodo']);
+
+      viewRef.context = {name: 'Bilbo'};
+      fixture.detectChanges();
+      button.click();
+      expect(events).toEqual(['Frodo', 'Bilbo']);
+    });
+  });
 });