Skip to content

Commit

Permalink
fix(elements): fire custom element output events during component ini…
Browse files Browse the repository at this point in the history 
…tialization (#37570)

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 `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. Changing `NgElementImpl` to subscribe to `NgElementStrategy#events`
   (if available) before calling `NgElementStrategy#connect()` (which
   initializes the component instance) if available.
3. Falling back to the old behavior (subscribing to `events` after
   calling `connect()` for strategies that do not initialize `events`
   before their `connect()` is run).

NOTE:
By falling back to the old behavior when `NgElementStrategy#events` is
not initialized before calling `NgElementStrategy#connect()`, we avoid
breaking existing custom `NgElementStrategy` implementations (with
@remackgeek's [ElementZoneStrategy][4] being a commonly used example).

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
[4]: https://github.com/remackgeek/elements-zone-strategy/blob/f1b6699495a8c0909dbbfbdca12770ecca843e15/projects/elements-zone-strategy/src/lib/element-zone-strategy.ts

Fixes #36141

PR Close #37570
  • Loading branch information
@gkalpak @AndrewKushnir
gkalpak authored and AndrewKushnir committed Jun 29, 2020
1 parent 78a4476 commit eba9dd3
Show file tree
Hide file tree
Showing 4 changed files with 126 additions and 24 deletions.
21 changes: 12 additions & 9 deletions packages/elements/src/component-factory-strategy.ts
View file
Expand Up @@ -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';
Expand Down Expand Up @@ -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<Observable<NgElementStrategyEvent>[]>(1);

/** Merged stream of the component's output events. */
// TODO(issue/24571): remove '!'.
events!: Observable<NgElementStrategyEvent>;
readonly events = this.eventEmitters.pipe(switchMap(emitters => merge(...emitters)));

/** Reference to the component that was created on connect. */
private componentRef: ComponentRef<any>|null = null;
Expand Down Expand Up @@ -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<any>): void {
const eventEmitters = this.componentFactory.outputs.map(({propName, templateName}) => {
const emitter: EventEmitter<any> = componentRef.instance[propName];
return emitter.pipe(map(value => ({name: templateName, value})));
});
const eventEmitters: Observable<NgElementStrategyEvent>[] =
this.componentFactory.outputs.map(({propName, templateName}) => {
const emitter: EventEmitter<any> = 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. */
Expand Down
35 changes: 30 additions & 5 deletions packages/elements/src/create-custom-element.ts
View file
Expand Up @@ -187,13 +187,30 @@ export function createCustomElement<P>(
}

connectedCallback(): void {
// For historical reasons, some strategies may not have initialized the `events` property
// until after `connect()` is run. Subscribe to `events` if it is available before running
// `connect()` (in order to capture events emitted suring inittialization), otherwise
// subscribe afterwards.
//
// TODO: Consider deprecating/removing the post-connect subscription in a future major version
// (e.g. v11).

let subscribedToEvents = false;

if (this.ngElementStrategy.events) {
// `events` are already available: Subscribe to it asap.
this.subscribeToEvents();
subscribedToEvents = true;
}

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);
});
if (!subscribedToEvents) {
// `events` were not initialized before running `connect()`: Subscribe to them now.
// The events emitted during the component initialization have been missed, but at least
// future events will be captured.
this.subscribeToEvents();
}
}

disconnectedCallback(): void {
Expand All @@ -207,6 +224,14 @@ export function createCustomElement<P>(
this.ngElementEventsSubscription = null;
}
}

private subscribeToEvents(): void {
// 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);
});
}
}

// TypeScript 3.9+ defines getters/setters as configurable but non-enumerable properties (in
Expand Down
27 changes: 27 additions & 0 deletions packages/elements/test/component-factory-strategy_spec.ts
View file
Expand Up @@ -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
Expand Down
67 changes: 57 additions & 10 deletions packages/elements/test/create-custom-element_spec.ts
View file
Expand Up @@ -40,12 +40,7 @@ if (browserDetection.supportsCustomElements) {
strategyFactory = new TestStrategyFactory();
strategy = strategyFactory.testStrategy;

const {selector, ElementCtor} = createTestCustomElement();
NgElementCtor = ElementCtor;

// The `@webcomponents/custom-elements/src/native-shim.js` polyfill allows us to create
// new instances of the NgElement which extends HTMLElement, as long as we define it.
customElements.define(selector, NgElementCtor);
NgElementCtor = createAndRegisterTestCustomElement(strategyFactory);
})
.then(done, done.fail);
});
Expand Down Expand Up @@ -117,6 +112,47 @@ 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 not break if `NgElementStrategy#events` is not available before calling `NgElementStrategy#connect()`',
() => {
class TestStrategyWithLateEvents extends TestStrategy {
events: Subject<NgElementStrategyEvent> = undefined!;

connect(element: HTMLElement): void {
this.connectedElement = element;
this.events = new Subject<NgElementStrategyEvent>();
this.events.next({name: 'strategy-event', value: 'connect'});
}
}

const strategyWithLateEvents = new TestStrategyWithLateEvents();
const capturedEvents: string[] = [];

const NgElementCtorWithLateEventsStrategy =
createAndRegisterTestCustomElement({create: () => strategyWithLateEvents});

const element = new NgElementCtorWithLateEventsStrategy(injector);
element.addEventListener(
'strategy-event', evt => capturedEvents.push((evt as CustomEvent).detail));
element.connectedCallback();

// The "connect" event (emitted during initialization) was missed, but things didn't break.
expect(capturedEvents).toEqual([]);

// Subsequent events are still captured.
strategyWithLateEvents.events.next({name: 'strategy-event', value: 'after-connect'});
expect(capturedEvents).toEqual(['after-connect']);
});

it('should properly set getters/setters on the element', () => {
const element = new NgElementCtor(injector);
element.fooFoo = 'foo-foo-value';
Expand Down Expand Up @@ -144,7 +180,7 @@ if (browserDetection.supportsCustomElements) {

it('should capture properties set before upgrading the element', () => {
// Create a regular element and set properties on it.
const {selector, ElementCtor} = createTestCustomElement();
const {selector, ElementCtor} = createTestCustomElement(strategyFactory);
const element = Object.assign(document.createElement(selector), {
fooFoo: 'foo-prop-value',
barBar: 'bar-prop-value',
Expand All @@ -165,7 +201,7 @@ if (browserDetection.supportsCustomElements) {
it('should capture properties set after upgrading the element but before inserting it into the DOM',
() => {
// Create a regular element and set properties on it.
const {selector, ElementCtor} = createTestCustomElement();
const {selector, ElementCtor} = createTestCustomElement(strategyFactory);
const element = Object.assign(document.createElement(selector), {
fooFoo: 'foo-prop-value',
barBar: 'bar-prop-value',
Expand Down Expand Up @@ -193,7 +229,7 @@ if (browserDetection.supportsCustomElements) {
it('should allow overwriting properties with attributes after upgrading the element but before inserting it into the DOM',
() => {
// Create a regular element and set properties on it.
const {selector, ElementCtor} = createTestCustomElement();
const {selector, ElementCtor} = createTestCustomElement(strategyFactory);
const element = Object.assign(document.createElement(selector), {
fooFoo: 'foo-prop-value',
barBar: 'bar-prop-value',
Expand All @@ -219,7 +255,17 @@ if (browserDetection.supportsCustomElements) {
});

// Helpers
function createTestCustomElement() {
function createAndRegisterTestCustomElement(strategyFactory: NgElementStrategyFactory) {
const {selector, ElementCtor} = createTestCustomElement(strategyFactory);

// The `@webcomponents/custom-elements/src/native-shim.js` polyfill allows us to create
// new instances of the NgElement which extends HTMLElement, as long as we define it.
customElements.define(selector, ElementCtor);

return ElementCtor;
}

function createTestCustomElement(strategyFactory: NgElementStrategyFactory) {
return {
selector: `test-element-${++selectorUid}`,
ElementCtor: createCustomElement<WithFooBar>(TestComponent, {injector, strategyFactory}),
Expand Down Expand Up @@ -255,6 +301,7 @@ if (browserDetection.supportsCustomElements) {
events = new Subject<NgElementStrategyEvent>();

connect(element: HTMLElement): void {
this.events.next({name: 'strategy-event', value: 'connect'});
this.connectedElement = element;
}

Expand Down

0 comments on commit eba9dd3

Please sign in to comment.