From 8b01c668d75fcfa6127537a1576fdf12a8fcd469 Mon Sep 17 00:00:00 2001 From: George Kalpakas Date: Fri, 20 Mar 2020 15:07:09 +0200 Subject: [PATCH] fix(elements): fire custom element output events during component initialization Previously, event listeners for component output events attached on an Angular custom element before inserting it into the DOM (i.e. before instantiating the underlying component) didn't fire for events emitted during initialization lifecycle hooks, such as `ngAfterContentInit`, `ngAfterViewInit`, `ngOnChanges` (initial call) and `ngOnInit`. The reason was that that `NgElementImpl` [subscribed to events][1] _after_ calling [ngElementStrategy#connect()][2], which is where the [initial change detection][3] takes place (running the initialization lifecycle hooks). This commit fixes this by: 1. Ensuring `ComponentNgElementStrategy#events` is defined and available for subscribing to, even before instantiating the component. 2. Ensuring `NgElementImpl` subscribes to `NgElementStrategy#events` before calling `NgElementStrategy#connect()` (which initializes the component instance). Jira issue: [FW-2010](https://angular-team.atlassian.net/browse/FW-2010) [1]: https://github.com/angular/angular/blob/c0143cb2abdd172de1b95fd1d2c4cfc738640e28/packages/elements/src/create-custom-element.ts#L167-L170 [2]: https://github.com/angular/angular/blob/c0143cb2abdd172de1b95fd1d2c4cfc738640e28/packages/elements/src/create-custom-element.ts#L164 [3]: https://github.com/angular/angular/blob/c0143cb2abdd172de1b95fd1d2c4cfc738640e28/packages/elements/src/component-factory-strategy.ts#L158 Fixes #36141 --- .../src/component-factory-strategy.ts | 21 ++++++++------- .../elements/src/create-custom-element.ts | 4 +-- .../test/component-factory-strategy_spec.ts | 27 +++++++++++++++++++ .../test/create-custom-element_spec.ts | 11 ++++++++ 4 files changed, 52 insertions(+), 11 deletions(-) diff --git a/packages/elements/src/component-factory-strategy.ts b/packages/elements/src/component-factory-strategy.ts index 550a1d74cecb5d..4ee3fc11039f8f 100644 --- a/packages/elements/src/component-factory-strategy.ts +++ b/packages/elements/src/component-factory-strategy.ts @@ -7,8 +7,8 @@ */ import {ApplicationRef, ComponentFactory, ComponentFactoryResolver, ComponentRef, EventEmitter, Injector, OnChanges, SimpleChange, SimpleChanges, Type} from '@angular/core'; -import {merge, Observable} from 'rxjs'; -import {map} from 'rxjs/operators'; +import {merge, Observable, ReplaySubject} from 'rxjs'; +import {map, switchMap} from 'rxjs/operators'; import {NgElementStrategy, NgElementStrategyEvent, NgElementStrategyFactory} from './element-strategy'; import {extractProjectableNodes} from './extract-projectable-nodes'; @@ -43,9 +43,11 @@ export class ComponentNgElementStrategyFactory implements NgElementStrategyFacto * @publicApi */ export class ComponentNgElementStrategy implements NgElementStrategy { + // Subject of `NgElementStrategyEvent` observables corresponding to the component's outputs. + private eventEmitters = new ReplaySubject[]>(1); + /** Merged stream of the component's output events. */ - // TODO(issue/24571): remove '!'. - events!: Observable; + readonly events = this.eventEmitters.pipe(switchMap(emitters => merge(...emitters))); /** Reference to the component that was created on connect. */ private componentRef: ComponentRef|null = null; @@ -187,12 +189,13 @@ export class ComponentNgElementStrategy implements NgElementStrategy { /** Sets up listeners for the component's outputs so that the events stream emits the events. */ protected initializeOutputs(componentRef: ComponentRef): void { - const eventEmitters = this.componentFactory.outputs.map(({propName, templateName}) => { - const emitter: EventEmitter = componentRef.instance[propName]; - return emitter.pipe(map(value => ({name: templateName, value}))); - }); + const eventEmitters: Observable[] = + this.componentFactory.outputs.map(({propName, templateName}) => { + const emitter: EventEmitter = componentRef.instance[propName]; + return emitter.pipe(map(value => ({name: templateName, value}))); + }); - this.events = merge(...eventEmitters); + this.eventEmitters.next(eventEmitters); } /** Calls ngOnChanges with all the inputs that have changed since the last call. */ diff --git a/packages/elements/src/create-custom-element.ts b/packages/elements/src/create-custom-element.ts index 1fbdcd4ecb1304..ec599c3f776a95 100644 --- a/packages/elements/src/create-custom-element.ts +++ b/packages/elements/src/create-custom-element.ts @@ -187,13 +187,13 @@ export function createCustomElement

( } connectedCallback(): void { - this.ngElementStrategy.connect(this); - // Listen for events from the strategy and dispatch them as custom events this.ngElementEventsSubscription = this.ngElementStrategy.events.subscribe(e => { const customEvent = createCustomEvent(this.ownerDocument!, e.name, e.value); this.dispatchEvent(customEvent); }); + + this.ngElementStrategy.connect(this); } disconnectedCallback(): void { diff --git a/packages/elements/test/component-factory-strategy_spec.ts b/packages/elements/test/component-factory-strategy_spec.ts index c93e182986b2f4..330874e93e4c94 100644 --- a/packages/elements/test/component-factory-strategy_spec.ts +++ b/packages/elements/test/component-factory-strategy_spec.ts @@ -41,6 +41,33 @@ describe('ComponentFactoryNgElementStrategy', () => { expect(strategyFactory.create(injector)).toBeTruthy(); }); + describe('before connected', () => { + it('should allow subscribing to output events', () => { + const events: NgElementStrategyEvent[] = []; + strategy.events.subscribe(e => events.push(e)); + + // No events before connecting (since `componentRef` is not even on the strategy yet). + componentRef.instance.output1.next('output-1a'); + componentRef.instance.output1.next('output-1b'); + componentRef.instance.output2.next('output-2a'); + expect(events).toEqual([]); + + // No events upon connecting (since events are not cached/played back). + strategy.connect(document.createElement('div')); + expect(events).toEqual([]); + + // Events emitted once connected. + componentRef.instance.output1.next('output-1c'); + componentRef.instance.output1.next('output-1d'); + componentRef.instance.output2.next('output-2b'); + expect(events).toEqual([ + {name: 'templateOutput1', value: 'output-1c'}, + {name: 'templateOutput1', value: 'output-1d'}, + {name: 'templateOutput2', value: 'output-2b'}, + ]); + }); + }); + describe('after connected', () => { beforeEach(() => { // Set up an initial value to make sure it is passed to the component diff --git a/packages/elements/test/create-custom-element_spec.ts b/packages/elements/test/create-custom-element_spec.ts index 3f9dcd5eafc7f7..a08cb73b6319a3 100644 --- a/packages/elements/test/create-custom-element_spec.ts +++ b/packages/elements/test/create-custom-element_spec.ts @@ -117,6 +117,16 @@ if (browserDetection.supportsCustomElements) { expect(eventValue).toEqual(null); }); + it('should listen to output events during initialization', () => { + const events: string[] = []; + + const element = new NgElementCtor(injector); + element.addEventListener('strategy-event', evt => events.push((evt as CustomEvent).detail)); + element.connectedCallback(); + + expect(events).toEqual(['connect']); + }); + it('should properly set getters/setters on the element', () => { const element = new NgElementCtor(injector); element.fooFoo = 'foo-foo-value'; @@ -255,6 +265,7 @@ if (browserDetection.supportsCustomElements) { events = new Subject(); connect(element: HTMLElement): void { + this.events.next({name: 'strategy-event', value: 'connect'}); this.connectedElement = element; }