fix(elements): run strategy methods in correct zone (#37814)

Default change detection fails in some cases for @angular/elements where
component events are called from the wrong zone.

This fixes the issue by running all ComponentNgElementStrategy methods
in the same zone it was created in.

Fixes #24181

PR Close #37814
This commit is contained in:
remackgeek 2020-06-28 17:18:19 -07:00 committed by Misko Hevery
parent 778ad3776a
commit 8df888dfb4
3 changed files with 108 additions and 45 deletions

View File

@ -12,7 +12,7 @@
"master": { "master": {
"uncompressed": { "uncompressed": {
"runtime-es2015": 2987, "runtime-es2015": 2987,
"main-es2015": 450301, "main-es2015": 450596,
"polyfills-es2015": 52630 "polyfills-es2015": 52630
} }
} }
@ -21,7 +21,7 @@
"master": { "master": {
"uncompressed": { "uncompressed": {
"runtime-es2015": 3097, "runtime-es2015": 3097,
"main-es2015": 429200, "main-es2015": 429885,
"polyfills-es2015": 52195 "polyfills-es2015": 52195
} }
} }

View File

@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license * found in the LICENSE file at https://angular.io/license
*/ */
import {ApplicationRef, ComponentFactory, ComponentFactoryResolver, ComponentRef, EventEmitter, Injector, OnChanges, SimpleChange, SimpleChanges, Type} from '@angular/core'; import {ApplicationRef, ComponentFactory, ComponentFactoryResolver, ComponentRef, EventEmitter, Injector, NgZone, OnChanges, SimpleChange, SimpleChanges, Type} from '@angular/core';
import {merge, Observable, ReplaySubject} from 'rxjs'; import {merge, Observable, ReplaySubject} from 'rxjs';
import {map, switchMap} from 'rxjs/operators'; import {map, switchMap} from 'rxjs/operators';
@ -73,6 +73,13 @@ export class ComponentNgElementStrategy implements NgElementStrategy {
*/ */
private readonly unchangedInputs = new Set<string>(); private readonly unchangedInputs = new Set<string>();
/** Service for setting zone context. */
private readonly ngZone = this.injector.get<NgZone>(NgZone);
/** The zone the element was created in or `null` if Zone.js is not loaded. */
private readonly elementZone =
(typeof Zone === 'undefined') ? null : this.ngZone.run(() => Zone.current);
constructor(private componentFactory: ComponentFactory<any>, private injector: Injector) {} constructor(private componentFactory: ComponentFactory<any>, private injector: Injector) {}
/** /**
@ -80,16 +87,19 @@ export class ComponentNgElementStrategy implements NgElementStrategy {
* destruction. * destruction.
*/ */
connect(element: HTMLElement) { connect(element: HTMLElement) {
// If the element is marked to be destroyed, cancel the task since the component was reconnected this.runInZone(() => {
if (this.scheduledDestroyFn !== null) { // If the element is marked to be destroyed, cancel the task since the component was
this.scheduledDestroyFn(); // reconnected
this.scheduledDestroyFn = null; if (this.scheduledDestroyFn !== null) {
return; this.scheduledDestroyFn();
} this.scheduledDestroyFn = null;
return;
}
if (this.componentRef === null) { if (this.componentRef === null) {
this.initializeComponent(element); this.initializeComponent(element);
} }
});
} }
/** /**
@ -97,19 +107,21 @@ export class ComponentNgElementStrategy implements NgElementStrategy {
* being moved across the DOM. * being moved across the DOM.
*/ */
disconnect() { disconnect() {
// Return if there is no componentRef or the component is already scheduled for destruction this.runInZone(() => {
if (this.componentRef === null || this.scheduledDestroyFn !== null) { // Return if there is no componentRef or the component is already scheduled for destruction
return; if (this.componentRef === null || this.scheduledDestroyFn !== null) {
} return;
// Schedule the component to be destroyed after a small timeout in case it is being
// moved elsewhere in the DOM
this.scheduledDestroyFn = scheduler.schedule(() => {
if (this.componentRef !== null) {
this.componentRef.destroy();
this.componentRef = null;
} }
}, DESTROY_DELAY);
// Schedule the component to be destroyed after a small timeout in case it is being
// moved elsewhere in the DOM
this.scheduledDestroyFn = scheduler.schedule(() => {
if (this.componentRef !== null) {
this.componentRef.destroy();
this.componentRef = null;
}
}, DESTROY_DELAY);
});
} }
/** /**
@ -117,11 +129,13 @@ export class ComponentNgElementStrategy implements NgElementStrategy {
* retrieved from the cached initialization values. * retrieved from the cached initialization values.
*/ */
getInputValue(property: string): any { getInputValue(property: string): any {
if (this.componentRef === null) { return this.runInZone(() => {
return this.initialInputValues.get(property); if (this.componentRef === null) {
} return this.initialInputValues.get(property);
}
return this.componentRef.instance[property]; return this.componentRef.instance[property];
});
} }
/** /**
@ -129,22 +143,24 @@ export class ComponentNgElementStrategy implements NgElementStrategy {
* cached and set when the component is created. * cached and set when the component is created.
*/ */
setInputValue(property: string, value: any): void { setInputValue(property: string, value: any): void {
if (this.componentRef === null) { this.runInZone(() => {
this.initialInputValues.set(property, value); if (this.componentRef === null) {
return; this.initialInputValues.set(property, value);
} return;
}
// Ignore the value if it is strictly equal to the current value, except if it is `undefined` // Ignore the value if it is strictly equal to the current value, except if it is `undefined`
// and this is the first change to the value (because an explicit `undefined` _is_ strictly // and this is the first change to the value (because an explicit `undefined` _is_ strictly
// equal to not having a value set at all, but we still need to record this as a change). // equal to not having a value set at all, but we still need to record this as a change).
if (strictEquals(value, this.getInputValue(property)) && if (strictEquals(value, this.getInputValue(property)) &&
!((value === undefined) && this.unchangedInputs.has(property))) { !((value === undefined) && this.unchangedInputs.has(property))) {
return; return;
} }
this.recordInputChange(property, value); this.recordInputChange(property, value);
this.componentRef.instance[property] = value; this.componentRef.instance[property] = value;
this.scheduleDetectChanges(); this.scheduleDetectChanges();
});
} }
/** /**
@ -264,4 +280,9 @@ export class ComponentNgElementStrategy implements NgElementStrategy {
this.callNgOnChanges(this.componentRef); this.callNgOnChanges(this.componentRef);
this.componentRef.changeDetectorRef.detectChanges(); this.componentRef.changeDetectorRef.detectChanges();
} }
/** Runs in the angular zone, if present. */
private runInZone(fn: () => unknown) {
return (this.elementZone && Zone.current !== this.elementZone) ? this.ngZone.run(fn) : fn();
}
} }

View File

@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license * found in the LICENSE file at https://angular.io/license
*/ */
import {ComponentFactory, ComponentRef, Injector, NgModuleRef, SimpleChange, SimpleChanges, Type} from '@angular/core'; import {ApplicationRef, ComponentFactory, ComponentFactoryResolver, ComponentRef, Injector, NgModuleRef, NgZone, SimpleChange, SimpleChanges, Type} from '@angular/core';
import {fakeAsync, tick} from '@angular/core/testing'; import {fakeAsync, tick} from '@angular/core/testing';
import {Subject} from 'rxjs'; import {Subject} from 'rxjs';
@ -20,22 +20,40 @@ describe('ComponentFactoryNgElementStrategy', () => {
let injector: any; let injector: any;
let componentRef: any; let componentRef: any;
let applicationRef: any; let applicationRef: any;
let ngZone: any;
let injectables: Map<unknown, unknown>;
beforeEach(() => { beforeEach(() => {
factory = new FakeComponentFactory(); factory = new FakeComponentFactory();
componentRef = factory.componentRef; componentRef = factory.componentRef;
applicationRef = jasmine.createSpyObj('applicationRef', ['attachView']); applicationRef = jasmine.createSpyObj('applicationRef', ['attachView']);
ngZone = jasmine.createSpyObj('ngZone', ['run']);
ngZone.run.and.callFake((fn: () => unknown) => fn());
injector = jasmine.createSpyObj('injector', ['get']); injector = jasmine.createSpyObj('injector', ['get']);
injector.get.and.returnValue(applicationRef); injector.get.and.callFake((token: unknown) => {
if (!injectables.has(token)) {
throw new Error(`Failed to get injectable from mock injector: ${token}`);
}
return injectables.get(token);
});
injectables = new Map<unknown, unknown>([
[ApplicationRef, applicationRef],
[NgZone, ngZone],
]);
strategy = new ComponentNgElementStrategy(factory, injector); strategy = new ComponentNgElementStrategy(factory, injector);
ngZone.run.calls.reset();
}); });
it('should create a new strategy from the factory', () => { it('should create a new strategy from the factory', () => {
const factoryResolver = jasmine.createSpyObj('factoryResolver', ['resolveComponentFactory']); const factoryResolver = jasmine.createSpyObj('factoryResolver', ['resolveComponentFactory']);
factoryResolver.resolveComponentFactory.and.returnValue(factory); factoryResolver.resolveComponentFactory.and.returnValue(factory);
injector.get.and.returnValue(factoryResolver); injectables.set(ComponentFactoryResolver, factoryResolver);
const strategyFactory = new ComponentNgElementStrategyFactory(FakeComponent, injector); const strategyFactory = new ComponentNgElementStrategyFactory(FakeComponent, injector);
expect(strategyFactory.create(injector)).toBeTruthy(); expect(strategyFactory.create(injector)).toBeTruthy();
@ -266,6 +284,30 @@ describe('ComponentFactoryNgElementStrategy', () => {
expect(componentRef.destroy).toHaveBeenCalledTimes(1); expect(componentRef.destroy).toHaveBeenCalledTimes(1);
})); }));
}); });
describe('runInZone', () => {
const param = 'foofoo';
const fn = () => param;
it('should run the callback directly when invoked in element\'s zone', () => {
expect(strategy['runInZone'](fn)).toEqual('foofoo');
expect(ngZone.run).not.toHaveBeenCalled();
});
it('should run the callback inside the element\'s zone when invoked in a different zone',
() => {
expect(Zone.root.run(() => (strategy['runInZone'](fn)))).toEqual('foofoo');
expect(ngZone.run).toHaveBeenCalledWith(fn);
});
it('should run the callback directly when called without zone.js loaded', () => {
// simulate no zone.js loaded
(strategy as any)['elementZone'] = null;
expect(Zone.root.run(() => (strategy['runInZone'](fn)))).toEqual('foofoo');
expect(ngZone.run).not.toHaveBeenCalled();
});
});
}); });
export class FakeComponentWithoutNgOnChanges { export class FakeComponentWithoutNgOnChanges {