From 7a05b6a0849d1a1cc52495a4e03832b941475a30 Mon Sep 17 00:00:00 2001 From: gund Date: Fri, 20 Apr 2018 01:21:29 +0200 Subject: [PATCH] fix(directive): Update inputs changes when both comp and inputs are changed So that OnChanges hook is called only once closes #88 --- src/dynamic/dynamic.directive.spec.ts | 31 +++++++++++++++++++++--- src/dynamic/dynamic.directive.ts | 24 +++++++++--------- src/test/component-injector.component.ts | 2 +- 3 files changed, 40 insertions(+), 17 deletions(-) diff --git a/src/dynamic/dynamic.directive.spec.ts b/src/dynamic/dynamic.directive.spec.ts index 102a89686..42881f751 100644 --- a/src/dynamic/dynamic.directive.spec.ts +++ b/src/dynamic/dynamic.directive.spec.ts @@ -68,7 +68,7 @@ describe('Directive: Dynamic', () => { }); it('should trigger initially `OnChanges` life-cycle hook', () => { - injectedComp.ngOnChanges.and.callFake((changes: SimpleChanges) => { + injectedComp.ngOnChanges.mockImplementation((changes: SimpleChanges) => { expect(changes.prop1).toBeDefined(); expect(changes.prop1.currentValue).toBe('prop1'); expect(changes.prop1.isFirstChange()).toBeTruthy(); @@ -87,7 +87,7 @@ describe('Directive: Dynamic', () => { expect(injectedComp.ngOnChanges).toHaveBeenCalledTimes(1); - injectedComp.ngOnChanges.and.callFake((changes: SimpleChanges) => { + injectedComp.ngOnChanges.mockImplementation((changes: SimpleChanges) => { expect(changes.prop1).toBeDefined(); expect(changes.prop1.currentValue).toBe('123'); expect(changes.prop1.isFirstChange()).toBeFalsy(); @@ -103,7 +103,7 @@ describe('Directive: Dynamic', () => { }); it('should trigger `OnChanges` life-cycle hook if component instance was updated', () => { - injectedComp.ngOnChanges.and.callFake((changes: SimpleChanges) => { + injectedComp.ngOnChanges.mockImplementation((changes: SimpleChanges) => { expect(changes.prop1).toBeDefined(); expect(changes.prop1.currentValue).toBe('prop1'); expect(changes.prop1.isFirstChange()).toBeTruthy(); @@ -142,6 +142,31 @@ describe('Directive: Dynamic', () => { fixture.componentInstance['inputs'] = { ...fixture.componentInstance['inputs'] }; expect(() => fixture.detectChanges()).not.toThrow(); }); + + it('should call `ngOnChanges` once when inputs and component updated', () => { + fixture.detectChanges(); + injectorComp.component.ngOnChanges.mockReset(); + + const inputs = fixture.componentInstance['inputs']; + fixture.componentInstance['inputs'] = { ...inputs, prop: 'any' }; + const newInjectedComp = injectorComp.component = { ...injectorComp.component }; + + newInjectedComp.ngOnChanges.mockImplementation((changes: SimpleChanges) => { + expect(changes.prop).toBeDefined(); + expect(changes.prop.currentValue).toBe('any'); + expect(changes.prop.previousValue).toBeUndefined(); + expect(changes.prop.isFirstChange()).toBeTruthy(); + + expect(changes.prop1).toBeDefined(); + expect(changes.prop1.currentValue).toBe('prop1'); + expect(changes.prop1.previousValue).toBeUndefined(); + expect(changes.prop1.isFirstChange()).toBeTruthy(); + }); + + fixture.detectChanges(); + + expect(newInjectedComp.ngOnChanges).toHaveBeenCalledTimes(1); + }); }); describe('inputs with `NgComponentOutlet`', () => { diff --git a/src/dynamic/dynamic.directive.ts b/src/dynamic/dynamic.directive.ts index 9a99132ae..0bc916a16 100644 --- a/src/dynamic/dynamic.directive.ts +++ b/src/dynamic/dynamic.directive.ts @@ -85,22 +85,20 @@ export class DynamicDirective implements OnChanges, DoCheck, OnDestroy { ) { } ngOnChanges(changes: SimpleChanges) { - if (this._componentInstChanged) { - this.updateInputs(true); - this.bindOutputs(); - } else { - if (this._inputsChanged(changes)) { - const inputsChanges = this._getInputsChanges(this._inputs); + const compChanged = this._componentInstChanged; - if (inputsChanges) { - this._updateInputChanges(inputsChanges); - this.updateInputs(!this._lastInputChanges); - } - } + if (compChanged || this._inputsChanged(changes)) { + const inputsChanges = this._getInputsChanges(this._inputs); - if (this._outputsChanged(changes)) { - this.bindOutputs(); + if (inputsChanges) { + this._updateInputChanges(inputsChanges); } + + this.updateInputs(compChanged || !this._lastInputChanges); + } + + if (compChanged || this._outputsChanged(changes)) { + this.bindOutputs(); } } diff --git a/src/test/component-injector.component.ts b/src/test/component-injector.component.ts index ab9b5b126..fb9d2e1bc 100644 --- a/src/test/component-injector.component.ts +++ b/src/test/component-injector.component.ts @@ -18,6 +18,6 @@ export class ComponentInjectorComponent implements ComponentInjector { } export class MockedInjectedComponent { - ngOnChanges = jasmine.createSpy('ngOnChanges'); + ngOnChanges = jest.fn(); onEvent = new EventEmitter(); }