feat(router): warn if navigation triggered outside Angular zonecloses a…

…ngular#24728
trotyl · Jul 19, 2018 · e76aebe · e76aebe
1 parent 0b28732
commit e76aebe
Show file tree

Hide file tree

Showing 3 changed files with 59 additions and 4 deletions.
diff --git a/packages/core/src/zone.ts b/packages/core/src/zone.ts
@@ -7,4 +7,4 @@
  */
 
 // Public API for Zone
-export {NgZone} from './zone/ng_zone';
+export {NgZone, NoopNgZone as ɵNoopNgZone} from './zone/ng_zone';
diff --git a/packages/router/src/router.ts b/packages/router/src/router.ts
@@ -7,7 +7,7 @@
  */
 
 import {Location} from '@angular/common';
-import {Compiler, Injector, NgModuleFactoryLoader, NgModuleRef, Type, isDevMode} from '@angular/core';
+import {Compiler, Injector, NgModuleFactoryLoader, NgModuleRef, NgZone, Optional, Type, isDevMode, ɵConsole as Console} from '@angular/core';
 import {BehaviorSubject, Observable, Subject, Subscription, of } from 'rxjs';
 import {concatMap, map, mergeMap} from 'rxjs/operators';
 
@@ -224,6 +224,8 @@ export class Router {
   private navigationId: number = 0;
   private configLoader: RouterConfigLoader;
   private ngModule: NgModuleRef<any>;
+  private console: Console;
+  private isNgZoneEnabled: boolean = false;
 
   public readonly events: Observable<Event> = new Subject<Event>();
   public readonly routerState: RouterState;
@@ -309,6 +311,9 @@ export class Router {
     const onLoadEnd = (r: Route) => this.triggerEvent(new RouteConfigLoadEnd(r));
 
     this.ngModule = injector.get(NgModuleRef);
+    this.console = injector.get(Console);
+    const ngZone = injector.get(NgZone);
+    this.isNgZoneEnabled = ngZone instanceof NgZone;
 
     this.resetConfig(config);
     this.currentUrlTree = createEmptyUrlTree();
@@ -490,6 +495,11 @@ export class Router {
    */
   navigateByUrl(url: string|UrlTree, extras: NavigationExtras = {skipLocationChange: false}):
       Promise<boolean> {
+    if (isDevMode() && this.console && this.isNgZoneEnabled && !NgZone.isInAngularZone()) {
+      this.console.warn(
+          `Navigation triggered outside Angular zone, did you forget to call 'ngZone.run()'?`);
+    }
+
     const urlTree = url instanceof UrlTree ? url : this.parseUrl(url);
     const mergedTree = this.urlHandlingStrategy.merge(urlTree, this.rawUrlTree);
 

diff --git a/packages/router/test/integration.spec.ts b/packages/router/test/integration.spec.ts
@@ -8,7 +8,7 @@
 
 import {CommonModule, Location} from '@angular/common';
 import {SpyLocation} from '@angular/common/testing';
-import {ChangeDetectionStrategy, Component, Injectable, NgModule, NgModuleFactoryLoader, NgModuleRef} from '@angular/core';
+import {ChangeDetectionStrategy, Component, Injectable, NgModule, NgModuleFactoryLoader, NgModuleRef, NgZone, ɵConsole as Console, ɵNoopNgZone as NoopNgZone} from '@angular/core';
 import {ComponentFixture, TestBed, fakeAsync, inject, tick} from '@angular/core/testing';
 import {By} from '@angular/platform-browser/src/dom/debug/by';
 import {expect} from '@angular/platform-browser/testing/src/matchers';
@@ -23,7 +23,8 @@ describe('Integration', () => {
   beforeEach(() => {
     TestBed.configureTestingModule({
       imports:
-          [RouterTestingModule.withRoutes([{path: 'simple', component: SimpleCmp}]), TestModule]
+          [RouterTestingModule.withRoutes([{path: 'simple', component: SimpleCmp}]), TestModule],
+      providers: [{provide: Console, useValue: undefined}]
     });
   });
 
@@ -154,6 +155,50 @@ describe('Integration', () => {
        })));
   });
 
+  describe('navigation warning', () => {
+    let warnings: string[] = [];
+
+    class MockConsole {
+      warn(message: string) { warnings.push(message); }
+    }
+
+    beforeEach(() => {
+      warnings = [];
+      TestBed.overrideProvider(Console, {useValue: new MockConsole()});
+    });
+
+    describe('with NgZone enabled', () => {
+      it('should warn when triggered outside Angular zone',
+         fakeAsync(inject([Router, NgZone], (router: Router, ngZone: NgZone) => {
+           ngZone.runOutsideAngular(() => { router.navigateByUrl('/simple'); });
+
+           expect(warnings.length).toBe(1);
+           expect(warnings[0])
+               .toBe(
+                   `Navigation triggered outside Angular zone, did you forget to call 'ngZone.run()'?`);
+         })));
+
+      it('should not warn when triggered inside Angular zone',
+         fakeAsync(inject([Router, NgZone], (router: Router, ngZone: NgZone) => {
+           ngZone.run(() => { router.navigateByUrl('/simple'); });
+
+           expect(warnings.length).toBe(0);
+         })));
+    });
+
+    describe('with NgZone disabled', () => {
+      beforeEach(() => { TestBed.overrideProvider(NgZone, {useValue: new NoopNgZone()}); });
+
+      it('should not warn when triggered outside Angular zone',
+         fakeAsync(inject([Router, NgZone], (router: Router, ngZone: NgZone) => {
+
+           ngZone.runOutsideAngular(() => { router.navigateByUrl('/simple'); });
+
+           expect(warnings.length).toBe(0);
+         })));
+    });
+  });
+
   describe('should execute navigations serially', () => {
     let log: any[] = [];