From d2be675acc284cd7df981b5819c15c528e457f3a Mon Sep 17 00:00:00 2001 From: Andrew Seguin Date: Thu, 1 Mar 2018 11:26:29 -0800 Subject: [PATCH] feat(elements): add tests for component factory strategy (#22413) PR Close #22413 --- .../src/component-factory-strategy.ts | 11 +- packages/elements/src/utils.ts | 2 +- .../test/component-factory-strategy_spec.ts | 220 ++++++++++++++++-- packages/elements/test/utils_spec.ts | 8 +- 4 files changed, 218 insertions(+), 23 deletions(-) diff --git a/packages/elements/src/component-factory-strategy.ts b/packages/elements/src/component-factory-strategy.ts index 51c785a71b..33f05bccfe 100644 --- a/packages/elements/src/component-factory-strategy.ts +++ b/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, injector: Injector) { const attributeToPropertyInputs = new Map(); 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); } /** diff --git a/packages/elements/src/utils.ts b/packages/elements/src/utils.ts index e750e0af63..6e105483e1 100644 --- a/packages/elements/src/utils.ts +++ b/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()}`); } diff --git a/packages/elements/test/component-factory-strategy_spec.ts b/packages/elements/test/component-factory-strategy_spec.ts index e0fc8a8a6d..5f035aa6d9 100644 --- a/packages/elements/test/component-factory-strategy_spec.ts +++ b/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 { - 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 { 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 { 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); + } + }); +} diff --git a/packages/elements/test/utils_spec.ts b/packages/elements/test/utils_spec.ts index 718b140294..930fcf086a 100644 --- a/packages/elements/test/utils_spec.ts +++ b/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()', () => {