diff --git a/goldens/size-tracking/aio-payloads.json b/goldens/size-tracking/aio-payloads.json index 5765876886..fea778e139 100755 --- a/goldens/size-tracking/aio-payloads.json +++ b/goldens/size-tracking/aio-payloads.json @@ -26,4 +26,4 @@ } } } -} \ No newline at end of file +} diff --git a/packages/common/src/directives/ng_template_outlet.ts b/packages/common/src/directives/ng_template_outlet.ts index d2b53b1bd8..4ddb84391b 100644 --- a/packages/common/src/directives/ng_template_outlet.ts +++ b/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)) { - (this._viewRef!.context)[propName] = (this.ngTemplateOutletContext)[propName]; + } else if ( + this._viewRef && changes['ngTemplateOutletContext'] && this.ngTemplateOutletContext) { + this._viewRef.context = this.ngTemplateOutletContext; } } } diff --git a/packages/common/test/directives/ng_template_outlet_spec.ts b/packages/common/test/directives/ng_template_outlet_spec.ts index 865f8c40da..ee3132180e 100644 --- a/packages/common/test/directives/ng_template_outlet_spec.ts +++ b/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 = `:{{foo}}` + ``; @@ -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: ` + {{name}} + + | + + ` +}) +class MultiContextComponent { + context1: {name: string}|undefined; + context2: {name: string}|undefined; +} + function createTestComponent(template: string): ComponentFixture { return TestBed.overrideComponent(TestComponent, {set: {template: template}}) .configureTestingModule({schemas: [NO_ERRORS_SCHEMA]})