feat(elements): add tests for component factory strategy (#22413)

PR Close #22413

packages/elements/src/component-factory-strategy.ts

@@ -13,7 +13,7 @@ import {map} from 'rxjs/operator/map';
 
 import {NgElementStrategy, NgElementStrategyEvent, NgElementStrategyFactory} from './element-strategy';
 import {extractProjectableNodes} from './extract-projectable-nodes';
-import {camelToKebabCase, isFunction, scheduler, strictEquals} from './utils';
+import {camelToDashCase, isFunction, scheduler, strictEquals} from './utils';
 
 /** Time in milliseconds to wait before destroying the component ref when disconnected. */
 const DESTROY_DELAY = 10;
@@ -28,7 +28,7 @@ export function getConfigFromComponentFactory(
     componentFactory: ComponentFactory<any>, injector: Injector) {
   const attributeToPropertyInputs = new Map<string, string>();
   componentFactory.inputs.forEach(({propName, templateName}) => {
-    const attr = camelToKebabCase(templateName);
+    const attr = camelToDashCase(templateName);
     attributeToPropertyInputs.set(attr, propName);
   });
 
@@ -114,8 +114,11 @@ export class ComponentFactoryNgElementStrategy implements NgElementStrategy {
 
     // Schedule the component to be destroyed after a small timeout in case it is being
     // moved elsewhere in the DOM
-    this.scheduledDestroyFn =
-        scheduler.schedule(() => { this.componentRef !.destroy(); }, DESTROY_DELAY);
+    this.scheduledDestroyFn = scheduler.schedule(() => {
+      if (this.componentRef) {
+        this.componentRef !.destroy();
+      }
+    }, DESTROY_DELAY);
   }
 
   /**

packages/elements/src/utils.ts

@@ -46,7 +46,7 @@ export const scheduler = {
 /**
  * Convert a camelCased string to kebab-cased.
  */
-export function camelToKebabCase(input: string): string {
+export function camelToDashCase(input: string): string {
   return input.replace(/[A-Z]/g, char => `-${char.toLowerCase()}`);
 }
 

packages/elements/test/component-factory-strategy_spec.ts

@@ -6,45 +6,223 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {ComponentFactory, ComponentRef, Injector, NgModuleRef, Type} from '@angular/core';
+import {ComponentFactory, ComponentRef, Injector, NgModuleRef, SimpleChange, SimpleChanges, Type} from '@angular/core';
+import {fakeAsync, tick} from '@angular/core/testing';
 import {Subject} from 'rxjs/Subject';
 
-import {ComponentFactoryNgElementStrategy} from '../src/component-factory-strategy';
+import {ComponentFactoryNgElementStrategy, ComponentFactoryNgElementStrategyFactory, getConfigFromComponentFactory} from '../src/component-factory-strategy';
+import {NgElementStrategyEvent} from '../src/element-strategy';
 
 describe('ComponentFactoryNgElementStrategy', () => {
   let factory: FakeComponentFactory;
-  let component: FakeComponent;
   let strategy: ComponentFactoryNgElementStrategy;
-  let injector;
+
+  let injector: any;
+  let componentRef: any;
+  let applicationRef: any;
 
   beforeEach(() => {
     factory = new FakeComponentFactory();
-    component = factory.componentRef;
+    componentRef = factory.componentRef;
 
     injector = jasmine.createSpyObj('injector', ['get']);
-    const applicationRef = jasmine.createSpyObj('applicationRef', ['attachView']);
+    applicationRef = jasmine.createSpyObj('applicationRef', ['attachView']);
     injector.get.and.returnValue(applicationRef);
 
     strategy = new ComponentFactoryNgElementStrategy(factory, injector);
   });
 
-  describe('connect', () => {
+  it('should generate a default config for NgElement', () => {
+    let config = getConfigFromComponentFactory(factory, injector);
+    expect(config.strategyFactory).toBeTruthy();
+    expect(config.propertyInputs).toEqual(['fooFoo', 'barBar']);
+    expect(config.attributeToPropertyInputs.get('foo-foo')).toBe('fooFoo');
+    expect(config.attributeToPropertyInputs.get('my-bar-bar')).toBe('barBar');
+  });
+
+  it('should create a new strategy from the factory', () => {
+    const strategyFactory = new ComponentFactoryNgElementStrategyFactory(factory, injector);
+    expect(strategyFactory.create()).toBeTruthy();
+  });
+
+  describe('after connected', () => {
     beforeEach(() => {
-      const element = document.createElement('div');
-      strategy.connect(element);
+      // Set up an initial value to make sure it is passed to the component
+      strategy.setPropertyValue('fooFoo', 'fooFoo-1');
+      strategy.connect(document.createElement('div'));
     });
 
-    // TODO(andrewseguin): Test everything
+    it('should attach the component to the view',
+       () => { expect(applicationRef.attachView).toHaveBeenCalledWith(componentRef.hostView); });
+
+    it('should detect changes',
+       () => { expect(componentRef.changeDetectorRef.detectChanges).toHaveBeenCalled(); });
+
+    it('should listen to output events', () => {
+      const events: NgElementStrategyEvent[] = [];
+      strategy.events.subscribe(e => events.push(e));
+
+      componentRef.instance.output1.next('output-1a');
+      componentRef.instance.output1.next('output-1b');
+      componentRef.instance.output2.next('output-2a');
+      expect(events).toEqual([
+        {name: 'templateOutput1', value: 'output-1a'},
+        {name: 'templateOutput1', value: 'output-1b'},
+        {name: 'templateOutput2', value: 'output-2a'},
+      ]);
+    });
+
+    it('should initialize the component with initial values', () => {
+      expect(strategy.getPropertyValue('fooFoo')).toBe('fooFoo-1');
+      expect(componentRef.instance.fooFoo).toBe('fooFoo-1');
+    });
+
+    it('should call ngOnChanges with the change', () => {
+      expectSimpleChanges(
+          componentRef.instance.simpleChanges[0],
+          {fooFoo: new SimpleChange(undefined, 'fooFoo-1', false)});
+    });
+  });
+
+  it('should not call ngOnChanges if not present on the component', () => {
+    factory.componentRef.instance = new FakeComponentWithoutNgOnChanges();
+
+    // Should simply succeed without problems (did not try to call ngOnChanges)
+    strategy.connect(document.createElement('div'));
+  });
+
+  describe('when inputs change and not connected', () => {
+    it('should cache the value', () => {
+      strategy.setPropertyValue('fooFoo', 'fooFoo-1');
+      expect(strategy.getPropertyValue('fooFoo')).toBe('fooFoo-1');
+
+      // Sanity check: componentRef isn't changed since its not even on the strategy
+      expect(componentRef.instance.fooFoo).toBe(undefined);
+    });
+
+    it('should not detect changes', fakeAsync(() => {
+         strategy.setPropertyValue('fooFoo', 'fooFoo-1');
+         tick(16);  // scheduler waits 16ms if RAF is unavailable
+         expect(componentRef.changeDetectorRef.detectChanges).toHaveBeenCalledTimes(0);
+       }));
+  });
+
+  describe('when inputs change and is connected', () => {
+    beforeEach(() => { strategy.connect(document.createElement('div')); });
+
+    it('should be set on the component instance', () => {
+      strategy.setPropertyValue('fooFoo', 'fooFoo-1');
+      expect(componentRef.instance.fooFoo).toBe('fooFoo-1');
+      expect(strategy.getPropertyValue('fooFoo')).toBe('fooFoo-1');
+    });
+
+    it('should detect changes', fakeAsync(() => {
+         // Connect detected changes automatically
+         expect(componentRef.changeDetectorRef.detectChanges).toHaveBeenCalledTimes(1);
+
+         strategy.setPropertyValue('fooFoo', 'fooFoo-1');
+         tick(16);  // scheduler waits 16ms if RAF is unavailable
+         expect(componentRef.changeDetectorRef.detectChanges).toHaveBeenCalledTimes(2);
+       }));
+
+    it('should detect changes once for multiple input changes', fakeAsync(() => {
+         // Connect detected changes automatically
+         expect(componentRef.changeDetectorRef.detectChanges).toHaveBeenCalledTimes(1);
+
+         strategy.setPropertyValue('fooFoo', 'fooFoo-1');
+         strategy.setPropertyValue('barBar', 'barBar-1');
+         tick(16);  // scheduler waits 16ms if RAF is unavailable
+         expect(componentRef.changeDetectorRef.detectChanges).toHaveBeenCalledTimes(2);
+       }));
+
+    it('should call ngOnChanges', fakeAsync(() => {
+         strategy.setPropertyValue('fooFoo', 'fooFoo-1');
+         tick(16);  // scheduler waits 16ms if RAF is unavailable
+         expectSimpleChanges(
+             componentRef.instance.simpleChanges[0],
+             {fooFoo: new SimpleChange(undefined, 'fooFoo-1', true)});
+       }));
+
+    it('should call ngOnChanges once for multiple input changes', fakeAsync(() => {
+         strategy.setPropertyValue('fooFoo', 'fooFoo-1');
+         strategy.setPropertyValue('barBar', 'barBar-1');
+         tick(16);  // scheduler waits 16ms if RAF is unavailable
+         expectSimpleChanges(componentRef.instance.simpleChanges[0], {
+           fooFoo: new SimpleChange(undefined, 'fooFoo-1', true),
+           barBar: new SimpleChange(undefined, 'barBar-1', true)
+         });
+       }));
+
+    it('should call ngOnChanges twice for changes in different rounds with previous values',
+       fakeAsync(() => {
+         strategy.setPropertyValue('fooFoo', 'fooFoo-1');
+         strategy.setPropertyValue('barBar', 'barBar-1');
+         tick(16);  // scheduler waits 16ms if RAF is unavailable
+         expectSimpleChanges(componentRef.instance.simpleChanges[0], {
+           fooFoo: new SimpleChange(undefined, 'fooFoo-1', true),
+           barBar: new SimpleChange(undefined, 'barBar-1', true)
+         });
+
+         strategy.setPropertyValue('fooFoo', 'fooFoo-2');
+         strategy.setPropertyValue('barBar', 'barBar-2');
+         tick(16);  // scheduler waits 16ms if RAF is unavailable
+         expectSimpleChanges(componentRef.instance.simpleChanges[1], {
+           fooFoo: new SimpleChange('fooFoo-1', 'fooFoo-2', false),
+           barBar: new SimpleChange('barBar-1', 'barBar-2', false)
+         });
+       }));
+  });
+
+  describe('disconnect', () => {
+    it('should be able to call if not connected', fakeAsync(() => {
+         strategy.disconnect();
+
+         // Sanity check: the strategy doesn't have an instance of the componentRef anyways
+         expect(componentRef.destroy).not.toHaveBeenCalled();
+       }));
+
+    it('should destroy the component after the destroy delay', fakeAsync(() => {
+         strategy.connect(document.createElement('div'));
+         strategy.disconnect();
+         expect(componentRef.destroy).not.toHaveBeenCalled();
+
+         tick(10);
+         expect(componentRef.destroy).toHaveBeenCalledTimes(1);
+       }));
+
+    it('should be able to call it multiple times but only destroy once', fakeAsync(() => {
+         strategy.connect(document.createElement('div'));
+         strategy.disconnect();
+         strategy.disconnect();
+         expect(componentRef.destroy).not.toHaveBeenCalled();
+
+         tick(10);
+         expect(componentRef.destroy).toHaveBeenCalledTimes(1);
+
+         strategy.disconnect();
+         expect(componentRef.destroy).toHaveBeenCalledTimes(1);
+       }));
   });
 });
 
-export class FakeComponent {
+export class FakeComponentWithoutNgOnChanges {
   output1 = new Subject();
   output2 = new Subject();
 }
 
+export class FakeComponent {
+  output1 = new Subject();
+  output2 = new Subject();
+
+  // Keep track of the simple changes passed to ngOnChanges
+  simpleChanges: SimpleChanges[] = [];
+
+  ngOnChanges(simpleChanges: SimpleChanges) { this.simpleChanges.push(simpleChanges); }
+}
+
 export class FakeComponentFactory extends ComponentFactory<any> {
-  componentRef = jasmine.createSpyObj('componentRef', ['instance', 'changeDetectorRef']);
+  componentRef: any = jasmine.createSpyObj(
+      'componentRef', ['instance', 'changeDetectorRef', 'hostView', 'destroy']);
 
   constructor() {
     super();
@@ -58,8 +236,8 @@ export class FakeComponentFactory extends ComponentFactory<any> {
   get ngContentSelectors(): string[] { return ['content-1', 'content-2']; }
   get inputs(): {propName: string; templateName: string}[] {
     return [
-      {propName: 'input1', templateName: 'templateInput1'},
-      {propName: 'input1', templateName: 'templateInput2'},
+      {propName: 'fooFoo', templateName: 'fooFoo'},
+      {propName: 'barBar', templateName: 'my-bar-bar'},
     ];
   }
 
@@ -76,3 +254,17 @@ export class FakeComponentFactory extends ComponentFactory<any> {
     return this.componentRef;
   }
 }
+
+function expectSimpleChanges(actual: SimpleChanges, expected: SimpleChanges) {
+  Object.keys(actual).forEach(
+      key => { expect(expected[key]).toBeTruthy(`Change included additional key ${key}`); });
+
+  Object.keys(expected).forEach(key => {
+    expect(actual[key]).toBeTruthy(`Change should have included key ${key}`);
+    if (actual[key]) {
+      expect(actual[key].previousValue).toBe(expected[key].previousValue);
+      expect(actual[key].currentValue).toBe(expected[key].currentValue);
+      expect(actual[key].firstChange).toBe(expected[key].firstChange);
+    }
+  });
+}

packages/elements/test/utils_spec.ts

@@ -6,7 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {camelToKebabCase, createCustomEvent, isElement, isFunction, kebabToCamelCase, matchesSelector, scheduler, strictEquals} from '../src/utils';
+import {camelToDashCase, createCustomEvent, isElement, isFunction, kebabToCamelCase, matchesSelector, scheduler, strictEquals} from '../src/utils';
 
 describe('utils', () => {
   describe('scheduler', () => {
@@ -77,12 +77,12 @@ describe('utils', () => {
 
   describe('camelToKebabCase()', () => {
     it('should convert camel-case to kebab-case', () => {
-      expect(camelToKebabCase('fooBarBazQux')).toBe('foo-bar-baz-qux');
-      expect(camelToKebabCase('foo1Bar2Baz3Qux4')).toBe('foo1-bar2-baz3-qux4');
+      expect(camelToDashCase('fooBarBazQux')).toBe('foo-bar-baz-qux');
+      expect(camelToDashCase('foo1Bar2Baz3Qux4')).toBe('foo1-bar2-baz3-qux4');
     });
 
     it('should keep existing dashes',
-       () => { expect(camelToKebabCase('fooBar-baz-Qux')).toBe('foo-bar-baz--qux'); });
+       () => { expect(camelToDashCase('fooBar-baz-Qux')).toBe('foo-bar-baz--qux'); });
   });
 
   describe('createCustomEvent()', () => {