fix(directives): recreate directives when component changes

And destroy them when component unset
gund · Dec 13, 2018 · 85f10db · 85f10db
1 parent 723c240
commit 85f10db
Show file tree

Hide file tree

Showing 2 changed files with 115 additions and 23 deletions.
diff --git a/src/dynamic/dynamic-directives.directive.spec.ts b/src/dynamic/dynamic-directives.directive.spec.ts
@@ -19,26 +19,22 @@ import { Subject } from 'rxjs';
 
 import {
   ComponentInjectorComponent,
-  getDirective,
-  InjectedBoundComponent,
   InjectedComponent,
-  MockedInjectedComponent,
   TestComponent,
   TestModule,
+  MockedInjectedComponent,
 } from '../test/index';
 import { COMPONENT_INJECTOR } from './component-injector';
 import { ComponentOutletInjectorDirective } from './component-outlet-injector.directive';
 import {
   DirectiveRef,
   dynamicDirectiveDef,
   DynamicDirectivesDirective,
+  DynamicDirectiveDef,
 } from './dynamic-directives.directive';
 import { IoFactoryService } from './io-factory.service';
 import { WindowRefService, WINDOW_REF } from './window-ref.service';
-
-const getComponentInjectorFrom = getDirective(ComponentInjectorComponent);
-const getInjectedComponentFrom = getDirective(InjectedComponent);
-const getInjectedBoundComponentFrom = getDirective(InjectedBoundComponent);
+import { By } from '@angular/platform-browser';
 
 @Directive({ selector: 'mock' })
 class MockDirective
@@ -111,13 +107,13 @@ describe('Directive: DynamicDirectives', () => {
 
   describe('directives', () => {
     let fixture: ComponentFixture<TestComponent>;
-    let hostComp: any;
+    let hostComp: { dirs: DynamicDirectiveDef<any>[] };
 
     beforeEach(() => {
       const template = `<component-injector><div [ndcDynamicDirectives]="dirs"></div></component-injector>`;
       TestBed.overrideComponent(TestComponent, { set: { template } });
       fixture = TestBed.createComponent(TestComponent);
-      hostComp = fixture.componentInstance;
+      hostComp = fixture.componentInstance as any;
     });
 
     it('should init directives', () => {
@@ -221,11 +217,74 @@ describe('Directive: DynamicDirectives', () => {
 
       expect(MockDirective.INSTANCES.size).toBe(1);
     });
+
+    it('should not do anything when no component', () => {
+      hostComp.dirs = [dynamicDirectiveDef(MockDirective)];
+      const compInjectorElem = fixture.debugElement.query(
+        By.directive(ComponentInjectorComponent),
+      );
+      expect(compInjectorElem).toBeTruthy();
+      const compInjector = compInjectorElem.componentInstance as ComponentInjectorComponent;
+      compInjector.component = null;
+
+      fixture.detectChanges();
+
+      expect(MockDirective.INSTANCES.size).toBe(0);
+    });
+
+    it('should destroy directives when component unset', () => {
+      const compInjectorElem = fixture.debugElement.query(
+        By.directive(ComponentInjectorComponent),
+      );
+      expect(compInjectorElem).toBeTruthy();
+      const compInjector = compInjectorElem.componentInstance as ComponentInjectorComponent;
+
+      hostComp.dirs = [dynamicDirectiveDef(MockDirective)];
+
+      fixture.detectChanges();
+
+      expect(MockDirective.INSTANCES.size).toBe(1);
+      const dir = getFirstDir();
+      expect(dir).toEqual(expect.any(MockDirective));
+
+      compInjector.component = null;
+
+      fixture.detectChanges();
+
+      expect(MockDirective.INSTANCES.size).toBe(0);
+      expect(dir.ngOnDestroy).toHaveBeenCalled();
+    });
+
+    it('should re-create directives when component changed', () => {
+      const compInjectorElem = fixture.debugElement.query(
+        By.directive(ComponentInjectorComponent),
+      );
+      expect(compInjectorElem).toBeTruthy();
+      const compInjector = compInjectorElem.componentInstance as ComponentInjectorComponent;
+
+      hostComp.dirs = [dynamicDirectiveDef(MockDirective)];
+
+      fixture.detectChanges();
+
+      expect(MockDirective.INSTANCES.size).toBe(1);
+      const dir = getFirstDir();
+      expect(dir).toEqual(expect.any(MockDirective));
+
+      compInjector.component = new MockedInjectedComponent();
+
+      fixture.detectChanges();
+
+      expect(MockDirective.INSTANCES.size).toBe(1);
+      expect(dir.ngOnDestroy).toHaveBeenCalled();
+      const newDir = getFirstDir();
+      expect(newDir).toEqual(expect.any(MockDirective));
+      expect(newDir).not.toBe(dir);
+    });
   });
 
   describe('@Output(ndcDynamicDirectivesCreated)', () => {
     let fixture: ComponentFixture<TestComponent>;
-    let hostComp: any;
+    let hostComp: { dirs: DynamicDirectiveDef<any>[]; created: jest.Mock };
     let created = jest.fn();
 
     beforeEach(() => {
@@ -235,7 +294,7 @@ describe('Directive: DynamicDirectives', () => {
         (ndcDynamicDirectivesCreated)="created($event)"></div></component-injector>`;
       TestBed.overrideComponent(TestComponent, { set: { template } });
       fixture = TestBed.createComponent(TestComponent);
-      hostComp = fixture.componentInstance;
+      hostComp = fixture.componentInstance as any;
       hostComp.created = created;
     });
 

diff --git a/src/dynamic/dynamic-directives.directive.ts b/src/dynamic/dynamic-directives.directive.ts
@@ -9,6 +9,7 @@ import {
   Inject,
   Injector,
   Input,
+  IterableChanges,
   IterableDiffers,
   OnDestroy,
   Optional,
@@ -67,6 +68,8 @@ export class DynamicDirectivesDirective implements OnDestroy, DoCheck {
     null,
   );
 
+  private lastCompInstance: any;
+
   private get directives() {
     return (
       this.ndcDynamicDirectives || this.ngComponentOutletNdcDynamicDirectives
@@ -81,6 +84,18 @@ export class DynamicDirectivesDirective implements OnDestroy, DoCheck {
     return this.compInjector.componentRef;
   }
 
+  private get compInstance() {
+    return this.componentRef && this.componentRef.instance;
+  }
+
+  private get isCompChanged() {
+    if (this.lastCompInstance !== this.compInstance) {
+      this.lastCompInstance = this.compInstance;
+      return true;
+    }
+    return false;
+  }
+
   private get hostInjector() {
     return this.componentRef.injector;
   }
@@ -112,27 +127,39 @@ export class DynamicDirectivesDirective implements OnDestroy, DoCheck {
   ) {}
 
   ngDoCheck(): void {
-    this.checkDirectives();
-  }
-
-  ngOnDestroy(): void {
-    this.dirRef.forEach(dir => this.destroyDirRef(dir));
-    this.dirRef.clear();
-    this.dirIo.clear();
-  }
+    if (this.maybeDestroyDirectives()) {
+      return;
+    }
 
-  private checkDirectives() {
     const dirsChanges = this.dirsDiffer.diff(this.directives);
 
     if (!dirsChanges) {
       return this.updateDirectives();
     }
 
-    dirsChanges.forEachRemovedItem(({ item }) => this.destroyDirective(item));
+    this.processDirChanges(dirsChanges);
+  }
 
-    const createdDirs = [];
+  ngOnDestroy(): void {
+    this.destroyAllDirectives();
+  }
+
+  private maybeDestroyDirectives() {
+    if (this.isCompChanged || !this.componentRef) {
+      this.dirsDiffer.diff([]);
+      this.destroyAllDirectives();
+    }
+
+    return !this.componentRef;
+  }
 
-    dirsChanges.forEachAddedItem(({ item }) =>
+  private processDirChanges(
+    changes: IterableChanges<DynamicDirectiveDef<any>>,
+  ) {
+    changes.forEachRemovedItem(({ item }) => this.destroyDirective(item));
+
+    const createdDirs = [];
+    changes.forEachAddedItem(({ item }) =>
       createdDirs.push(this.initDirective(item)),
     );
 
@@ -178,6 +205,12 @@ export class DynamicDirectivesDirective implements OnDestroy, DoCheck {
     return dir;
   }
 
+  private destroyAllDirectives() {
+    this.dirRef.forEach(dir => this.destroyDirRef(dir));
+    this.dirRef.clear();
+    this.dirIo.clear();
+  }
+
   private destroyDirective(dirDef: DynamicDirectiveDef<any>) {
     this.destroyDirRef(this.dirRef.get(dirDef.type));
     this.dirRef.delete(dirDef.type);