Permalink
Browse files

feat(router): warn if navigation triggered outside Angular zone (#24959)

closes #15770, closes #15946, closes #24728

PR Close #24959
  • Loading branch information...
trotyl authored and mhevery committed Aug 9, 2018
1 parent 234661b commit 010e35d99596e1b35808aaaeecfc32f3b247a7d8
Showing with 61 additions and 4 deletions.
  1. +1 −1 packages/core/src/zone.ts
  2. +11 −1 packages/router/src/router.ts
  3. +49 −2 packages/router/test/integration.spec.ts
@@ -7,4 +7,4 @@
*/
// Public API for Zone
export {NgZone} from './zone/ng_zone';
export {NgZone, NoopNgZone as ɵNoopNgZone} from './zone/ng_zone';
@@ -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;
@@ -314,6 +316,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();
@@ -495,6 +500,11 @@ export class Router {
*/
navigateByUrl(url: string|UrlTree, extras: NavigationExtras = {skipLocationChange: false}):
Promise<boolean> {
if (isDevMode() && 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);
@@ -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';
@@ -20,10 +20,13 @@ import {forEach} from '../src/utils/collection';
import {RouterTestingModule, SpyNgModuleFactoryLoader} from '../testing';
describe('Integration', () => {
const noopConsole: Console = {log() {}, warn() {}};
beforeEach(() => {
TestBed.configureTestingModule({
imports:
[RouterTestingModule.withRoutes([{path: 'simple', component: SimpleCmp}]), TestModule]
[RouterTestingModule.withRoutes([{path: 'simple', component: SimpleCmp}]), TestModule],
providers: [{provide: Console, useValue: noopConsole}]
});
});
@@ -154,6 +157,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[] = [];

0 comments on commit 010e35d

Please sign in to comment.