From 70081568da4ca1b1c4d765cb248faaf2145607ef Mon Sep 17 00:00:00 2001 From: Alex Malkevich Date: Fri, 23 Mar 2018 14:29:23 +0000 Subject: [PATCH] feat(directive): Add support for bound inputs and outputs Also fix a bug when inputs/outputs where comletely reassigned and changes were not picked up closes #102 --- src/dynamic/dynamic.directive.spec.ts | 131 ++++++++++++++++++++++++ src/dynamic/dynamic.directive.ts | 139 ++++++++++++++++++++++++-- src/test/test.component.ts | 31 ++++-- 3 files changed, 281 insertions(+), 20 deletions(-) diff --git a/src/dynamic/dynamic.directive.spec.ts b/src/dynamic/dynamic.directive.spec.ts index 551d45a29..3447f07ff 100644 --- a/src/dynamic/dynamic.directive.spec.ts +++ b/src/dynamic/dynamic.directive.spec.ts @@ -12,6 +12,7 @@ import { InjectedComponent, MockedInjectedComponent, TestComponent, + InjectedBoundComponent, TestModule, } from '../test/index'; import { COMPONENT_INJECTOR } from './component-injector'; @@ -19,6 +20,7 @@ import { DynamicDirective } from './dynamic.directive'; const getComponentInjectorFrom = getByPredicate(By.directive(ComponentInjectorComponent)); const getInjectedComponentFrom = getByPredicate(By.directive(InjectedComponent)); +const getInjectedBoundComponentFrom = getByPredicate(By.directive(InjectedBoundComponent)); describe('Directive: Dynamic', () => { beforeEach(() => { @@ -50,6 +52,23 @@ describe('Directive: Dynamic', () => { expect(injectedComp['prop2']).toBe(2); }); + it('should be reassigned when replaced', () => { + fixture.detectChanges(); + fixture.componentInstance['inputs'] = { otherProp: 'set' }; + fixture.detectChanges(); + + expect(injectedComp['otherProp']).toBe('set'); + }); + + it('should be reassigned from `null|undefined` when replaced', () => { + fixture.componentInstance['inputs'] = null; + fixture.detectChanges(); + fixture.componentInstance['inputs'] = { otherProp: 'set' }; + fixture.detectChanges(); + + expect(injectedComp['otherProp']).toBe('set'); + }); + it('should trigger initially `OnChanges` life-cycle hook', () => { injectedComp.ngOnChanges.and.callFake((changes: SimpleChanges) => { expect(changes.prop1).toBeDefined(); @@ -175,6 +194,66 @@ describe('Directive: Dynamic', () => { }); }); + describe('bound inputs', () => { + let fixture: ComponentFixture; + let testComp: any; + let injectedComp: InjectedBoundComponent; + let onChangesMock: jest.Mock; + + beforeEach(async(() => { + TestBed.configureTestingModule({ + imports: [TestModule], + declarations: [DynamicDirective, TestComponent], + }); + + const template = ``; + TestBed + .overrideComponent(TestComponent, { set: { template } }) + .compileComponents(); + fixture = TestBed.createComponent(TestComponent); + + testComp = fixture.componentInstance; + testComp.comp = InjectedBoundComponent; + testComp.inputs = null; + + fixture.detectChanges(); + injectedComp = getInjectedBoundComponentFrom(fixture).component; + injectedComp['ngOnChanges'] = onChangesMock = jest.fn(); + })); + + it('should correctly be passed to dynamic component', () => { + testComp.inputs = { outerProp: '123' }; + fixture.detectChanges(); + + expect(injectedComp.innerProp).toBe('123'); + }); + + it('should trigger `OnChanges` life-cycle hook with correct names', () => { + onChangesMock.mockImplementation((changes: SimpleChanges) => { + expect(changes.innerProp).toBeDefined(); + expect(changes.innerProp.currentValue).toBe('123'); + expect(changes.innerProp.isFirstChange()).toBeTruthy(); + }); + + testComp.inputs = { outerProp: '123' }; + fixture.detectChanges(); + + expect(onChangesMock).toHaveBeenCalledTimes(1); + + onChangesMock.mockImplementation((changes: SimpleChanges) => { + expect(changes.innerProp).toBeDefined(); + expect(changes.innerProp.currentValue).toBe('456'); + expect(changes.innerProp.previousValue).toBe('123'); + expect(changes.innerProp.isFirstChange()).toBeFalsy(); + }); + + testComp.inputs = { outerProp: '456' }; + fixture.detectChanges(); + + expect(onChangesMock).toHaveBeenCalledTimes(2); + }); + }); + describe('outputs', () => { let fixture: ComponentFixture , injectorComp: ComponentInjectorComponent @@ -201,6 +280,18 @@ describe('Directive: Dynamic', () => { expect(outputSpy).toHaveBeenCalledWith('data'); })); + it('should re-bind outputs after `null|undefiined` to component and receive events', async(() => { + fixture.componentInstance['outputs'] = null; + fixture.detectChanges(); + fixture.componentInstance['outputs'] = { onEvent: outputSpy }; + fixture.detectChanges(); + + injectedComp.onEvent.next('data'); + + expect(outputSpy).toHaveBeenCalledTimes(1); + expect(outputSpy).toHaveBeenCalledWith('data'); + })); + it('should NOT bind outputs to component when outputs undefined', async(() => { fixture.componentInstance['outputs'] = undefined; @@ -306,4 +397,44 @@ describe('Directive: Dynamic', () => { expect(outputSpy).toHaveBeenCalledWith('data'); }); }); + + describe('bound outputs', () => { + let fixture: ComponentFixture; + let testComp: any; + let injectedComp: InjectedBoundComponent; + let outputHandler: jest.Mock; + + beforeEach(async(() => { + TestBed.configureTestingModule({ + imports: [TestModule], + declarations: [DynamicDirective, TestComponent], + }); + + const template = ``; + TestBed + .overrideComponent(TestComponent, { set: { template } }) + .compileComponents(); + fixture = TestBed.createComponent(TestComponent); + + testComp = fixture.componentInstance; + testComp.comp = InjectedBoundComponent; + testComp.outputs = null; + + fixture.detectChanges(); + injectedComp = getInjectedBoundComponentFrom(fixture).component; + + outputHandler = jest.fn(); + })); + + it('should correctly be passed to dynamic component', () => { + testComp.outputs = { outerEvt: outputHandler }; + fixture.detectChanges(); + + injectedComp.innerEvt.emit('data'); + + expect(outputHandler).toHaveBeenCalledTimes(1); + expect(outputHandler).toHaveBeenCalledWith('data'); + }); + + }); }); diff --git a/src/dynamic/dynamic.directive.ts b/src/dynamic/dynamic.directive.ts index 0aa6e2dab..6fe121e2a 100644 --- a/src/dynamic/dynamic.directive.ts +++ b/src/dynamic/dynamic.directive.ts @@ -1,5 +1,8 @@ import { NgComponentOutlet } from '@angular/common'; import { + ComponentFactory, + ComponentFactoryResolver, + ComponentRef, Directive, DoCheck, Host, @@ -21,6 +24,8 @@ import { COMPONENT_INJECTOR, ComponentInjector } from './component-injector'; import { CustomSimpleChange, UNINITIALIZED } from './custom-simple-change'; export type KeyValueChangeRecordAny = KeyValueChangeRecord; +export type IOMapInfo = { propName: string, templateName: string }; +export type IOMappingList = IOMapInfo[]; @Directive({ selector: '[ndcDynamicInputs],[ndcDynamicOutputs],[ngComponentOutletNdcDynamicInputs],[ngComponentOutletNdcDynamicOutputs]' @@ -37,6 +42,7 @@ export class DynamicDirective implements OnChanges, DoCheck, OnDestroy { private _lastInputChanges: SimpleChanges; private _inputsDiffer = this._differs.find({}).create(); private _destroyed$ = new Subject(); + private _compFactory: ComponentFactory | null = null; private get _inputs() { return this.ndcDynamicInputs || this.ngComponentOutletNdcDynamicInputs; @@ -64,9 +70,18 @@ export class DynamicDirective implements OnChanges, DoCheck, OnDestroy { } } + private get _compRef(): ComponentRef | null { + return this._extractCompRefFrom(this._componentOutlet) || this._componentInjector.componentRef; + } + + private get _canResolveCompRef(): boolean { + return !!this._compRef; + } + constructor( private _differs: KeyValueDiffers, private _injector: Injector, + private _cfr: ComponentFactoryResolver, @Inject(COMPONENT_INJECTOR) private _componentInjectorType: ComponentInjector, @Host() @Optional() private _componentOutlet: NgComponentOutlet, ) { } @@ -75,6 +90,15 @@ export class DynamicDirective implements OnChanges, DoCheck, OnDestroy { if (this._componentInstChanged) { this.updateInputs(true); this.bindOutputs(); + } else { + if (this._inputsChanged(changes)) { + this._updateInputChanges(this._getInputsChanges(this._inputs)); + this.updateInputs(!this._lastInputChanges); + } + + if (this._outputsChanged(changes)) { + this.bindOutputs(); + } } } @@ -91,11 +115,11 @@ export class DynamicDirective implements OnChanges, DoCheck, OnDestroy { return; } - const inputsChanges = this._inputsDiffer.diff(inputs); + const inputsChanges = this._getInputsChanges(this._inputs); if (inputsChanges) { const isNotFirstChange = !!this._lastInputChanges; - this._lastInputChanges = this._collectChangesFromDiffer(inputsChanges); + this._updateInputChanges(inputsChanges); if (isNotFirstChange) { this.updateInputs(); @@ -108,26 +132,35 @@ export class DynamicDirective implements OnChanges, DoCheck, OnDestroy { } updateInputs(isFirstChange = false) { - const inputs = this._inputs; + if (isFirstChange) { + this._updateCompFactory(); + } + + let inputs = this._inputs; if (!inputs || !this._componentInst) { return; } - Object.keys(inputs).forEach(p => - this._componentInst[p] = inputs[p]); + inputs = this._resolveInputs(inputs); + + Object + .keys(inputs) + .forEach(p => this._componentInst[p] = inputs[p]); this.notifyOnInputChanges(this._lastInputChanges, isFirstChange); } bindOutputs() { this._destroyed$.next(); - const outputs = this._outputs; + let outputs = this._outputs; if (!outputs || !this._componentInst) { return; } + outputs = this._resolveOutputs(outputs); + Object.keys(outputs) .filter(p => this._componentInst[p]) .forEach(p => this._componentInst[p] @@ -148,6 +181,14 @@ export class DynamicDirective implements OnChanges, DoCheck, OnDestroy { this._componentInst.ngOnChanges(changes); } + private _getInputsChanges(inputs: any) { + return this._inputsDiffer.diff(this._inputs); + } + + private _updateInputChanges(differ: any) { + this._lastInputChanges = this._collectChangesFromDiffer(differ); + } + private _collectFirstChanges(): SimpleChanges { const changes = {} as SimpleChanges; const inputs = this._inputs; @@ -155,7 +196,7 @@ export class DynamicDirective implements OnChanges, DoCheck, OnDestroy { Object.keys(inputs).forEach(prop => changes[prop] = new CustomSimpleChange(UNINITIALIZED, inputs[prop], true)); - return changes; + return this._resolveChanges(changes); } private _collectChangesFromDiffer(differ: any): SimpleChanges { @@ -167,11 +208,91 @@ export class DynamicDirective implements OnChanges, DoCheck, OnDestroy { differ.forEachAddedItem((record: KeyValueChangeRecordAny) => changes[record.key].previousValue = UNINITIALIZED); - return changes; + return this._resolveChanges(changes); + } + + private _inputsChanged(changes: SimpleChanges): boolean { + return 'ngComponentOutletNdcDynamicInputs' in changes || 'ndcDynamicInputs' in changes; + } + + private _outputsChanged(changes: SimpleChanges): boolean { + return 'ngComponentOutletNdcDynamicOutputs' in changes || 'ndcDynamicOutputs' in changes; + } + + private _extractCompRefFrom(outlet: NgComponentOutlet | null): ComponentRef | null { + return outlet && (outlet)._componentRef; } private _extractCompFrom(outlet: NgComponentOutlet | null): any { - return outlet && (outlet)._componentRef && (outlet)._componentRef.instance; + const compRef = this._extractCompRefFrom(outlet); + return compRef && compRef.instance; + } + + private _resolveCompFactory(): ComponentFactory | null { + if (!this._canResolveCompRef) { + return null; + } + + try { + try { + return this._cfr.resolveComponentFactory(this._compRef.componentType); + } catch (e) { + // Fallback if componentType does not exist (happens on NgComponentOutlet) + return this._cfr.resolveComponentFactory(this._compRef.instance.constructor); + } + } catch (e) { + // Factory not available - bailout + return null; + } + } + + private _updateCompFactory() { + this._compFactory = this._resolveCompFactory(); + } + + private _resolveInputs(inputs: any): any { + if (!this._compFactory) { + return inputs; + } + + return this._remapIO(inputs, this._compFactory.inputs); + } + + private _resolveOutputs(outputs: any): any { + if (!this._compFactory) { + return outputs; + } + + return this._remapIO(outputs, this._compFactory.outputs); + } + + private _resolveChanges(changes: SimpleChanges): SimpleChanges { + if (!this._compFactory) { + return changes; + } + + return this._remapIO(changes, this._compFactory.inputs); + } + + private _remapIO(io: any, mapping: IOMappingList): any { + const newIO = {}; + + Object.keys(io) + .forEach(key => { + const newKey = this._findPropByTplInMapping(key, mapping) || key; + newIO[newKey] = io[key]; + }); + + return newIO; + } + + private _findPropByTplInMapping(tplName: string, mapping: IOMappingList): string | null { + for (const map of mapping) { + if (map.templateName === tplName) { + return map.propName; + } + } + return null; } } diff --git a/src/test/test.component.ts b/src/test/test.component.ts index 5a7cd71e1..fcf8fa7b7 100644 --- a/src/test/test.component.ts +++ b/src/test/test.component.ts @@ -1,28 +1,37 @@ import { CommonModule } from '@angular/common'; -import { Component, NgModule } from '@angular/core'; +import { Component, Input, Output, EventEmitter, NgModule } from '@angular/core'; @Component({ selector: 'test', - template: '' + template: '', }) -export class TestComponent { } +export class TestComponent {} @Component({ selector: 'injected', - template: 'foo' + template: 'foo', }) -export class InjectedComponent { } +export class InjectedComponent {} @Component({ selector: 'another-injected', - template: 'bar' + template: 'bar', }) -export class AnotherInjectedComponent { } +export class AnotherInjectedComponent {} + +@Component({ + selector: 'test-bindings', + template: 'baz', +}) +export class InjectedBoundComponent { + @Input('outerProp') innerProp: any; + @Output('outerEvt') innerEvt = new EventEmitter(); +} @NgModule({ imports: [CommonModule], - declarations: [InjectedComponent, AnotherInjectedComponent], - exports: [InjectedComponent, AnotherInjectedComponent], - entryComponents: [InjectedComponent, AnotherInjectedComponent] + declarations: [InjectedComponent, AnotherInjectedComponent, InjectedBoundComponent], + exports: [InjectedComponent, AnotherInjectedComponent, InjectedBoundComponent], + entryComponents: [InjectedComponent, AnotherInjectedComponent, InjectedBoundComponent], }) -export class TestModule { } +export class TestModule {}