Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"Navigation triggered outside Angular zone" warning in unit tests #25837

Open
cexbrayat opened this Issue Sep 6, 2018 · 24 comments

Comments

Projects
None yet
@cexbrayat
Copy link
Contributor

cexbrayat commented Sep 6, 2018

I'm submitting a...


[x] Regression (a behavior that used to work and stopped working in a new release)
[ ] Bug report  
[ ] Performance issue
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead see https://github.com/angular/angular/blob/master/CONTRIBUTING.md#question
[ ] Other... Please describe:

Current behavior

Using the last Angular 6.1.7 which introduced a warning if a navigation is triggered outside a zone (see 010e35d),
a simple unit test calling router.navigate yields the warning (resulting in hundreds of warnings in the console).

Expected behavior

The warning should not be emitted in unit tests.
It's currently possible to remove it by using zone.run or using NoopNgZone, but this is cumbersome. A way to disable it in unit tests would be great.

Minimal reproduction of the problem with instructions

Build a minimal app with the CLI

ng new router-warning --routing

Then replace the default unit test with:

import { TestBed, async } from '@angular/core/testing';
import { RouterTestingModule } from '@angular/router/testing';
import { Router } from '@angular/router';

import { AppComponent } from './app.component';

describe('AppComponent', () => {
  beforeEach(async(() => {
    TestBed.configureTestingModule({
      imports: [
        RouterTestingModule
      ],
      declarations: [
        AppComponent
      ],
    }).compileComponents();
  }));

  it('should create the app', async(() => {
    const router = TestBed.get(Router) as Router;
    router.navigateByUrl('/');
    const fixture = TestBed.createComponent(AppComponent);
    const app = fixture.debugElement.componentInstance;
    expect(app).toBeTruthy();
  }));
});

Run ng test, and the following warning should appear:

WARN: 'Navigation triggered outside Angular zone, did you forget to call 'ngZone.run()'?'

Environment


Angular version: 6.1.7 or 7.0.0-beta.5

Browser:
- [x] Chrome (desktop) version XX
- [ ] Chrome (Android) version XX
- [ ] Chrome (iOS) version XX
- [ ] Firefox version XX
- [ ] Safari (desktop) version XX
- [ ] Safari (iOS) version XX
- [ ] IE version XX
- [ ] Edge version XX
 
For Tooling issues:
- Node version: 8.11.3
- Platform:  Mac

cc @trotyl

@trotyl

This comment has been minimized.

Copy link
Contributor

trotyl commented Sep 6, 2018

Relates to #24181 (#24185), caused by NgZone.isInAngularZone() implementation.

CC @JiaLiPassion, any suggestion on checking whether it's Angular zone?

trotyl added a commit to trotyl/angular that referenced this issue Sep 6, 2018

@trotyl trotyl referenced a pull request that will close this issue Sep 6, 2018

Open

fix(router): better ngZone checking for warning #25839

2 of 3 tasks complete

@ngbot ngbot bot added this to the needsTriage milestone Sep 6, 2018

@vasiliy0s

This comment has been minimized.

Copy link

vasiliy0s commented Sep 7, 2018

The same for us. But our tests watches console.warn logging and this issue was sensitive for tests fail.

@JiaLiPassion

This comment has been minimized.

Copy link
Contributor

JiaLiPassion commented Sep 16, 2018

@trotyl, yeah, several other issues have the same problems, in test, NgZone.isInAngularZone() will be false, we may need a TestNgZone to work with ComponentFixture.autoDetectChanges

@webben-de

This comment has been minimized.

Copy link

webben-de commented Sep 28, 2018

+1

1 similar comment
@iotshaman

This comment has been minimized.

Copy link

iotshaman commented Oct 3, 2018

+1

@orjandesmet

This comment has been minimized.

Copy link

orjandesmet commented Oct 9, 2018

I'd say you don't need to navigate in unit tests.
You should spy on those methods and assert that they have been called.

@dhhyi

This comment has been minimized.

Copy link

dhhyi commented Oct 20, 2018

I'd say you don't need to navigate in unit tests.

For unit tests, yes. But the same framework can also be used for integration tests where you should be able to instantiate routing and do navigation.

@samuelt1

This comment has been minimized.

Copy link

samuelt1 commented Oct 29, 2018

I am getting this in my actual code. Not just in the tests.
this.router.navigate([this.returnUrl])

@ItzhakBokris

This comment has been minimized.

Copy link

ItzhakBokris commented Oct 29, 2018

Workaround:

constructor(private ngZone: NgZone, private router: Router) {}

public navigate(commands: any[]): void {
    this.ngZone.run(() => this.router.navigate(commands)).then();
}
@CharlyRipp

This comment has been minimized.

Copy link

CharlyRipp commented Oct 30, 2018

In unit test:

it('should just work', async(() => {
    fixture.ngZone.run(() => {
        router.navigate(['path1']);
        fixture.detectChanges();
        fixture.whenStable().then(() => {
            expect({stuff}).toHaveBeenCalled();
        });
    });
}));
@AliakbarSu

This comment has been minimized.

Copy link

AliakbarSu commented Nov 5, 2018

You also need to wrap router.initialNavigation();

fixture.ngZone.run(() => {
      router.initialNavigation();
});
@calebeaires

This comment has been minimized.

Copy link

calebeaires commented Nov 8, 2018

Workround proposed by @ItzhakBokris worked, but it must not be a permanent solution. Waiting to angular team!

this.ngZone.run(() => this.router.navigate(['home'])).then();

@quirogamauricio

This comment has been minimized.

Copy link

quirogamauricio commented Nov 12, 2018

I'm getting this warning not in a test, but in the actual application, when navigating from a component to another. I'm doing a sort of hack to be able to navigate using the router, from links present in a table that's generated with DataTables. Basically I'm temporarily adding a property to the window object, which holds a function that calls a component's method that uses the router. I'm not sure if the warning is actually correctly displayed or if this is a bug. Also I'd appreciate if someone could provide a better way to trigger navigation from dynamically generated content.

@ngbot ngbot bot modified the milestones: needsTriage, Backlog Nov 15, 2018

AndrejEPAM added a commit to AndrejEPAM/SportsBook that referenced this issue Nov 16, 2018

@AndrejEPAM AndrejEPAM referenced this issue Nov 16, 2018

Closed

T3 Log In #6

5 of 5 tasks complete
@trotyl

This comment has been minimized.

Copy link
Contributor

trotyl commented Nov 20, 2018

@quirogamauricio It's the expected warning, when perform navigation outside Angular zone than no change detection will be triggered. You do can perform navigation from anywhere of the JavaScript engine, but just need to ensure it inside Angular zone.

@piotrl

This comment has been minimized.

Copy link

piotrl commented Dec 10, 2018

As a workaround for tests, I created small proxy that runs all methods of router within NgZone.

import { NgZone } from '@angular/core';
import { Router } from '@angular/router';
import { isFunction } from 'lodash';

/**
 * Wrapper of Angular router (only for testing purpose)
 * Meant to run all router operations within Angular zone
 *  * Keep change detection enabled
 *  * Avoids flooded console with warnings
 *    https://github.com/angular/angular/issues/25837
 *
 * @see Router
 */
export function wrapRouterInNgZone(router: Router, ngZone: NgZone): Router {
  return new Proxy(router, {
    get(target: Router, p: PropertyKey): unknown {
      const invokedProperty = target[p];
      if (!isFunction(invokedProperty)) {
        return invokedProperty;
      }

      return function(...args: Array<unknown>): unknown {
        return ngZone.run(() => invokedProperty.apply(target, args));
      };
    },
  });
}

Note: unknown is replacement for any in TypeScript 3.0.

Usage:

  let router: Router;
  beforeEach(() => {
     router = wrapRouterInNgZone(
      TestBed.get(Router),
      TestBed.get(NgZone),
    );
  });

  // no flood console:
  it('some test', fakeAsync(() => {
    router.navigate(['']);
    tick();
  }));
@joshuakeel

This comment has been minimized.

Copy link

joshuakeel commented Dec 27, 2018

I see this warning in the console when running my app normally, not in unit tests.

constructor(private router: Router) { }

navigateAway() {
  this.router.navigateByUrl('foo');
}
@tylerjgarland

This comment has been minimized.

Copy link

tylerjgarland commented Jan 8, 2019

I have an app with 100s of links that make use of the router and am experiencing this issue after upgrading to angular 7.1. I created a zoned router service that uses the work around, but I'd like to avoid pulling out every routerLink into a function call to use the service. Any idea what the root cause is and how to resolve?

@trotyl

This comment has been minimized.

Copy link
Contributor

trotyl commented Jan 9, 2019

Please open a new issue with minimal reproduction if you're encountering this outside testing.

@lukasholzer

This comment has been minimized.

Copy link

lukasholzer commented Jan 11, 2019

As a workaround for tests, I created small proxy that runs all methods of router within NgZone.

import { NgZone } from '@angular/core';
import { Router } from '@angular/router';
import { isFunction } from 'lodash';

/**
 * Wrapper of Angular router (only for testing purpose)
 * Meant to run all router operations within Angular zone
 *  * Keep change detection enabled
 *  * Avoids flooded console with warnings
 *    https://github.com/angular/angular/issues/25837
 *
 * @see Router
 */
export function wrapRouterInNgZone(router: Router, ngZone: NgZone): Router {
  return new Proxy(router, {
    get(target: Router, p: PropertyKey): unknown {
      const invokedProperty = target[p];
      if (!isFunction(invokedProperty)) {
        return invokedProperty;
      }

      return function(...args: Array<unknown>): unknown {
        return ngZone.run(() => invokedProperty.apply(target, args));
      };
    },
  });
}

Note: unknown is replacement for any in TypeScript 3.0.

Usage:

  let router: Router;
  beforeEach(() => {
     router = wrapRouterInNgZone(
      TestBed.get(Router),
      TestBed.get(NgZone),
    );
  });

  // no flood console:
  it('some test', fakeAsync(() => {
    router.navigate(['']);
    tick();
  }));

nice workaround!

@Airblader

This comment has been minimized.

Copy link
Contributor

Airblader commented Jan 12, 2019

I've also run into this in tests, and the workaround proposed by @piotrl doesn't work for me as some navigations are triggered by the tested code, also causing these warnings – since that is what I am testing I don't want to mock everything here.

My approach is thus to instead silence the particular warning:

/** See https://github.com/angular/angular/issues/25837 */
export function setupNavigationWarnStub() {
    const warn = console.warn;

    spyOn(console, 'warn').and.callFake((...args: any[]) => {
        const [firstArg] = args;
        if (typeof firstArg === 'string' && firstArg.startsWith('Navigation triggered outside Angular zone')) {
            return;
        }

        return warn.apply(console, args);
    });
}

and then

beforeEach(() => setupNavigationWarnStub());
@rajugvsn

This comment has been minimized.

Copy link

rajugvsn commented Jan 16, 2019

I guess It is because latest angular CLI creates routing in a separate module
app-routing.module.ts

import { NgModule } from '@angular/core';
import { Routes, RouterModule } from '@angular/router';

const routes: Routes = [];

@NgModule({
  imports: [RouterModule.forRoot(routes)],
  exports: [RouterModule]
})
export class AppRoutingModule { }

so to run navigation with in Angular zone it is required to implement work around -

constructor(private ngZone: NgZone, private router: Router) {}

public navigate(commands: any[]): void {
    this.ngZone.run(() => this.router.navigate(commands)).then();
}

Alternatively if you define routing with in app module, it works fine.
app.routing.ts

import { Routes, RouterModule } from '@angular/router';

const appRoutes: Routes = [
    //add app routes here
];

export const routing = RouterModule.forRoot(appRoutes);
@DDieudonne

This comment has been minimized.

Copy link

DDieudonne commented Jan 16, 2019

Works perfectly for me @calebeaires thanks !!

@hollygood

This comment has been minimized.

Copy link

hollygood commented Feb 6, 2019

I also received the same warning by using ngrx/effects:

@Effect({ dispatch: false })
redirectToLogin$: Observable<Action> = this.actions$.pipe(
  ofType<RedirectToLogin>(AuthActionTypes.REDIRECT_TO_LOGIN),
    tap(() => {
      this.router.navigate(['/signin']);
     })
  );
@dianjuar

This comment has been minimized.

Copy link

dianjuar commented Feb 11, 2019

To avoid the callback hell in the @CharlyRipp suggestion you can use I used await

await fixture.ngZone.run(async () => {
    await router.navigate(['/home']);
});

// Put your asserts here
expect(true).toBeTruthy();
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.