fix(elements): run strategy methods in correct zone

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 angular#24181

Author: remackgeek

goldens/size-tracking/aio-payloads.json

@@ -12,7 +12,7 @@
     "master": {
       "uncompressed": {
         "runtime-es2015": 2987,
-        "main-es2015": 450883,
+        "main-es2015": 451395,
         "polyfills-es2015": 52630
       }
     }

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

@@ -6,7 +6,7 @@
  * 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 {map, switchMap} from 'rxjs/operators';
 
@@ -73,78 +73,94 @@ export class ComponentNgElementStrategy implements NgElementStrategy {
    */
   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) {}
 
   /**
    * Initializes a new component if one has not yet been created and cancels any scheduled
    * destruction.
    */
   connect(element: HTMLElement) {
-    // If the element is marked to be destroyed, cancel the task since the component was reconnected
-    if (this.scheduledDestroyFn !== null) {
-      this.scheduledDestroyFn();
-      this.scheduledDestroyFn = null;
-      return;
-    }
+    this.runInZone(() => {
+      // If the element is marked to be destroyed, cancel the task since the component was
+      // reconnected
+      if (this.scheduledDestroyFn !== null) {
+        this.scheduledDestroyFn();
+        this.scheduledDestroyFn = null;
+        return;
+      }
 
-    if (this.componentRef === null) {
-      this.initializeComponent(element);
-    }
+      if (this.componentRef === null) {
+        this.initializeComponent(element);
+      }
+    });
   }
 
   /**
    * Schedules the component to be destroyed after some small delay in case the element is just
    * being moved across the DOM.
    */
   disconnect() {
-    // Return if there is no componentRef or the component is already scheduled for destruction
-    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;
+    this.runInZone(() => {
+      // Return if there is no componentRef or the component is already scheduled for destruction
+      if (this.componentRef === null || this.scheduledDestroyFn !== null) {
+        return;
       }
-    }, 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);
+    });
   }
 
   /**
    * Returns the component property value. If the component has not yet been created, the value is
    * retrieved from the cached initialization values.
    */
   getInputValue(property: string): any {
-    if (this.componentRef === null) {
-      return this.initialInputValues.get(property);
-    }
+    return this.runInZone(() => {
+      if (this.componentRef === null) {
+        return this.initialInputValues.get(property);
+      }
 
-    return this.componentRef.instance[property];
+      return this.componentRef.instance[property];
+    });
   }
 
   /**
    * Sets the input value for the property. If the component has not yet been created, the value is
    * cached and set when the component is created.
    */
   setInputValue(property: string, value: any): void {
-    if (this.componentRef === null) {
-      this.initialInputValues.set(property, value);
-      return;
-    }
+    this.runInZone(() => {
+      if (this.componentRef === null) {
+        this.initialInputValues.set(property, value);
+        return;
+      }
 
-    // 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
-    // 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)) &&
-        !((value === undefined) && this.unchangedInputs.has(property))) {
-      return;
-    }
+      // 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
+      // 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)) &&
+          !((value === undefined) && this.unchangedInputs.has(property))) {
+        return;
+      }
 
-    this.recordInputChange(property, value);
-    this.componentRef.instance[property] = value;
-    this.scheduleDetectChanges();
+      this.recordInputChange(property, value);
+      this.componentRef.instance[property] = value;
+      this.scheduleDetectChanges();
+    });
   }
 
   /**
@@ -264,4 +280,9 @@ export class ComponentNgElementStrategy implements NgElementStrategy {
     this.callNgOnChanges(this.componentRef);
     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();
+  }
 }

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

@@ -6,7 +6,7 @@
  * 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 {Subject} from 'rxjs';
 
@@ -20,22 +20,40 @@ describe('ComponentFactoryNgElementStrategy', () => {
   let injector: any;
   let componentRef: any;
   let applicationRef: any;
+  let ngZone: any;
+
+  let injectables: Map<unknown, unknown>;
 
   beforeEach(() => {
     factory = new FakeComponentFactory();
     componentRef = factory.componentRef;
 
     applicationRef = jasmine.createSpyObj('applicationRef', ['attachView']);
+
+    ngZone = jasmine.createSpyObj('ngZone', ['run']);
+    ngZone.run.and.callFake((fn: () => unknown) => fn());
+
     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);
+    ngZone.run.calls.reset();
   });
 
   it('should create a new strategy from the factory', () => {
     const factoryResolver = jasmine.createSpyObj('factoryResolver', ['resolveComponentFactory']);
     factoryResolver.resolveComponentFactory.and.returnValue(factory);
-    injector.get.and.returnValue(factoryResolver);
+    injectables.set(ComponentFactoryResolver, factoryResolver);
 
     const strategyFactory = new ComponentNgElementStrategyFactory(FakeComponent, injector);
     expect(strategyFactory.create(injector)).toBeTruthy();
@@ -266,6 +284,30 @@ describe('ComponentFactoryNgElementStrategy', () => {
          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 {