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

router-outlet is appending rather than replacing when using BrowserAnimationsModule #20290

Closed
YaredTaddese opened this issue Nov 9, 2017 · 39 comments
Labels
area: animations area: router freq1: low P4 A relatively minor issue that is not relevant to core functions state: confirmed type: bug/fix
Milestone

Comments

@YaredTaddese
Copy link

I'm submitting a...


[ ] Regression (a behavior that used to work and stopped working in a new release)
[x] Bug report  
[ ] 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

Current behavior

A nested router-outlet for children components is appending the components rather than replacing. This behavior awkwardly, only appears when using BrowserAnimationsModule in imports of app.module.ts

Expected behavior

A nested router-outlet for children components should replace components rather than appending them when routing, even though I imported BrowserAnimationsModule in app.module.ts

Minimal reproduction of the problem with instructions

Create three components that means one parent and two children components when configuring their routes
parent component will have a router-outlet in its template for its two children
Then use BrowserAnimationsModule in imports property of app.module.ts
Finally when trying to route using the parent router-outlet, it will append its children rather than replacing

What is the motivation / use case for changing the behavior?

Want to use BrowserAnimationsModule

Environment


Angular version: 4.2.4


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: 6  
- Platform:  Linux

Others:

@jasonaden
Copy link
Contributor

Reported issues require a plunker (or other reproduction) so the issue can be verified. Also, you're reporting against an older version of Angular. If the issue persists, please provide:

  1. A way to reproduce.
  2. Confirm the issue exists in a currently supported version (4.4.5 or 5.0.1)

You can use this plunker template: http://plnkr.co/edit/tpl:AvJOMERrnz94ekVua0u5?p=catalogue

@doender
Copy link

doender commented Nov 14, 2017

@YaredTaddese In my case it was the same as this issue: #19093 and I handled an exception in such a way that it didn't show in the console, so I was not aware of any errors. Once I fixed the exception, the router-outlet worked normally again.

@Ismaestro
Copy link

This should not be closed...

@jasonaden
Copy link
Contributor

@Ismaestro This was closed because it doesn’t have steps to reproduce that can be executed. Plunker or StackBlitz. If the retro can be created, a new issue should be opened linking to it.

@Vinayaganga
Copy link

@jasonaden This problem still exist in latest Version of Angular.i.e.;Angular 5.0.1,Even I am facing the same problem

@jasonaden
Copy link
Contributor

@Vinayaganga @Ismaestro @YaredTaddese I really need a repro of this issue. I've gone through the steps above link here and I can't reproduce the problem. I'll re-open this issue for 24 hours, but if no one can produce a reproduction in that time, I need to close it since all issues need a reproduction. Feel free to fork and update the StackBlitz above to see if you can get it to reproduce.

@jasonaden jasonaden reopened this Nov 28, 2017
@jasonaden jasonaden added area: animations needs reproduction This issue needs a reproduction in order for the team to investigate further type: bug/fix labels Nov 28, 2017
@adgoncal
Copy link

adgoncal commented Nov 29, 2017

I don't know if this is the same issue, but it seems similar.
https://stackblitz.com/edit/animations-breaking-routing-ytqylg

Click the "Toggle error" button, then click it again. The second click should have removed <my-error> from the DOM due to *ngIf="hasError". Click the button again and now there are 2 <my-error> on the DOM. Every time the error is toggled off and on again, a new <my-error> is added to the DOM.

BTW, replacing BrowserAnimationsModule with NoopAnimationsModule did not solve the problem.

Note that I am providing my own ErrorHandler, which redirects to /apha/gamma.

image

EDIT: add diff

diff --git a/src/app/app.component.html b/src/app/app.component.html
index 2dcc5ca..d67bf34 100644
--- a/src/app/app.component.html
+++ b/src/app/app.component.html
@@ -1,2 +1,4 @@
 <a routerLink="/">Home</a>
+<button (click)="toggleError()">Toggle error</button>
+<my-error *ngIf="hasError"></my-error>
 <router-outlet>
\ No newline at end of file
diff --git a/src/app/app.component.ts b/src/app/app.component.ts
index fe4251b..4230955 100644
--- a/src/app/app.component.ts
+++ b/src/app/app.component.ts
@@ -7,4 +7,9 @@ import { Component } from '@angular/core';
 })
 export class AppComponent  {
   name = 'Angular 5';
+  hasError = false;
+
+  toggleError() {
+    this.hasError = !this.hasError;
+  }
 }
diff --git a/src/app/app.error-handler.ts b/src/app/app.error-handler.ts
new file mode 100644
index 0000000..8df29c3
--- /dev/null
+++ b/src/app/app.error-handler.ts
@@ -0,0 +1,16 @@
+import { ErrorHandler, Injectable, Injector } from '@angular/core';
+import { Router } from '@angular/router';
+
+@Injectable()
+export class AppErrorHandler implements ErrorHandler {
+  constructor(
+    private injector: Injector,
+  ) { }
+
+  handleError(err: any) {
+    console.log(err);
+
+    const router = this.injector.get(Router);
+    router.navigate(['/alpha/gamma']);
+  }
+}
\ No newline at end of file
diff --git a/src/app/app.module.ts b/src/app/app.module.ts
index 478b38f..2e74c61 100644
--- a/src/app/app.module.ts
+++ b/src/app/app.module.ts
@@ -1,11 +1,13 @@
-import { NgModule } from '@angular/core';
+import { ErrorHandler, NgModule } from '@angular/core';
 import { BrowserModule } from '@angular/platform-browser';
 import { BrowserAnimationsModule } from '@angular/platform-browser/animations';
 import { FormsModule } from '@angular/forms';
 
 import { AppRoutesModule } from './app-routes.module';
+import { AppErrorHandler } from './app.error-handler';
 
 import { AppComponent } from './app.component';
+import { ErrorComponent } from './error.component';
 
 import { AlphaModule } from './routes/alpha.module';
 
@@ -17,7 +19,10 @@ import { AlphaModule } from './routes/alpha.module';
     AlphaModule,
     BrowserAnimationsModule
   ],
-  declarations: [ AppComponent ],
+  providers: [
+    {provide: ErrorHandler, useClass: AppErrorHandler},
+  ],
+  declarations: [ AppComponent, ErrorComponent ],
   bootstrap:    [ AppComponent ]
 })
 export class AppModule { }
diff --git a/src/app/error.component.ts b/src/app/error.component.ts
new file mode 100644
index 0000000..d912f12
--- /dev/null
+++ b/src/app/error.component.ts
@@ -0,0 +1,11 @@
+import { Component, OnInit } from '@angular/core';
+
+@Component({
+  selector: 'my-error',
+  template: `<div>ErrorComponent</div>`,
+})
+export class ErrorComponent implements OnInit {
+  ngOnInit() {
+    throw Error('Error on purpose!');
+  }
+}

@jasonaden
Copy link
Contributor

@adgoncal Perfect! This is a great repro.

So I think maybe the reason replacing BrowserAnimationsModule didn't work is BrowserAnimationsModule is imported in multiple places. This was done in an attempt to replicate this previously, incase it needed to be imported within the module containing the routed-to components.

Removing those other BrowserAnimationsModule imports and leaving just the one in the app module appears to allow us to toggle the issue by including/not including BrowserAnimationsModule. Also, if we do include it, once you click the toggle error button, routing to the child routes produces multiple copies of the routed-to components.

I think this is close to reproducing the issue. It seems to rely on throwing an error that goes uncaught. If we catch the error, there's no problem.

Thanks a lot for the repro. This should help us get this debugged.

@Ismaestro
Copy link

This is exactly my case.

@hassanjuniedi
Copy link

The problem is that when using BrowserAnimationModule the router prevent destroying the component you leaving if it throwing an error .
The temporarily solution is make sure that the component you navigate from not throwing an error and router will work properly .

@ngbot ngbot bot added this to the Backlog milestone Jan 23, 2018
@PavanSinghx
Copy link

Trying this solved the problem for me:

this.zone.run(() => {
this.router.navigate(['/main']);
});

Remember to include the import:

import { NgZone } from '@angular/core';

constructor(public zone: NgZone) { }

@siqueiradg
Copy link

@PavanSinghx Thank you! Your solution solved the problem in my project.

@mkteagle
Copy link

mkteagle commented Feb 14, 2018

@PavanSinghx Wanted to say thanks because it works for me.

@Shamshiel
Copy link

I also have this problem in combination with a global error handler for unhandled errors.
A component is throwing an unhandled error and my error handler tries to route to the error-component but only appends the error-message. If I remove the BrowserAnimationsModule the routing works normally.
I worked around with
this.zone.run(() => { this.router.navigate(['/error']); });
but this feels like a hack.
Should I be save using this as a work around while this problem is getting fixed or could the usage of this.zone.run cause nasty side effect?

@ngbot ngbot bot modified the milestones: Backlog, needsTriage Feb 26, 2018
@freon27
Copy link

freon27 commented Mar 6, 2018

@PavanSinghx Thanks! This fixes a similar issue I'm facing due to BrowserAnimationsModule. I've spent the last hour reading about zones but are you able to maybe explain a little why it fixes it in this case please?

@fredyadriano90
Copy link

**@PavanSinghx** Thanks! This fix my problem with BrowserAnimationsModule and Routing

@Darwinyo
Copy link

Darwinyo commented Aug 6, 2018

Same problem here

@paulogr
Copy link

paulogr commented Aug 27, 2018

Having the same problem here when trying to redirect inside my global error handler

@akshaygujar1
Copy link

Issue only appears in ie11. Works fine in Chrome. Non error in console.

@kspearrin
Copy link

Same as @akshaygujar1 . I am seeing this in IE11 only.

@RichardMcElroy
Copy link

I have a similar problem - maybe it can help to resolve this issue. Creating a clean project with AngularCLI 6.2.4 (Angular 6.1.9) with some empty components and routing, if BrowserAnimationModule is included, when I try the app in localhost it works correctly, but on trying the app in a SharePoint Online page I get the same view stacking problem, but only when navigation is performed by the browser back/forward buttons or via the history object - history.back(), history.forward(), history.go(x) - i.e. a popstate event. Navigation via the router works as expected without stacking.
If I remove the BrowserAnimationModule from app.module the stacking problem disappears.

Other apps I have created for SharePoint up to and including Angular 5.1.3 do not show this problem.

I believe that it could be due to an error between the code already in the SharePoint page and the Angular app (maybe a zone conflict - that's just pure guessing), but there is no error in the debugging console and I am unsure how to proceed investigating this further.

If anyone would like to test this I can provide access to my SharePoint environment and the sample code I have used to test this - PM me with your email so I can grant access and send you the code sample.

Alternatively, if you can point me in the right direction as to how to proceed in trying to identify where the error is being generated, I can keep working on it.

@RichardMcElroy
Copy link

Update on my problem. I have gone through angular versions until I have found the version which shows the problem first - 5.2.4.
In the change log for this version there is the following change
core: fix #20582, don't need to wrap zone in location change listener (#22007) (ce51ea9)
I have tried patching the current 6.1.9 with the original code from before the fix and the problem disappears.

Hope this can help with the current problem.

@therepo90
Copy link

Same problem here. It almost drove me crazy, but removing browseranimationmodule fixed problem( which is no good )

@therepo90
Copy link

Found better solution. My api was not using NgZone. Include component dependency NgZone and run navigate inside this.zone.run

@harishkumar460
Copy link

@therepo90, The suggested solution is not working for me. Even I am not using BrowserAnimationModule.
Trying to run the code in this.zone.run. still its appending the component instead of replacing.

Guys, Please help me to resolve this issue. I am using the latest version of Angular 6.

@therepo90
Copy link

try to create stackblitz with minimal code so i can take a look

@thegeett
Copy link

thegeett commented Sep 24, 2019

I am having same issue, I am using Angular 8.1.3. Please take a look my stackblitz. I am able to reproduce there. I am not using BrowserAnimationsModule.
https://stackblitz.com/edit/angular-a8lmjs

@antontemchenko
Copy link

I am having same issue, I am using Angular 8.1.3. Please take a look my stackblitz. I am able to reproduce there. I am not using BrowserAnimationsModule.
https://stackblitz.com/edit/angular-a8lmjs

Hi @thegeett , if you are not using BrowserAnimtionModule it is not same issue. It looks like you are trying to achieve something different, like passing parameters to route. You can ask a question on StackOverflow if you still have an issue.

@meanstack-perficient
Copy link

this continues to be an issue within ionic-5.2.8/angular-8.1.3 node-10.16 routing modules
BrowserAnimationsModule should be importable-able anywhere without issues
If this is a zone issue it should have been nailed down by now

Is there a clear solution/workaround? Or are we forced to restructure our routing modules and outlets to get around this?

@antontemchenko
Copy link

antontemchenko commented Oct 18, 2019

For sure problem occurs when you have multiple instances of BrowserModule - which is provided by BrowserAnimationModule too.

I've faced this issue by creating lazy loaded modules as angular libraries and one of library used AnimationBuilder service which requires BrowserAnimationModule.

My solution is to import BrowserAnimationModule only in core module, and injecting AnimationBuilder in library's directive using @Inject(AnimationBuilder) private animationBuilder: AnimationBuilder

hope this helps someone

@za77
Copy link

za77 commented Dec 27, 2019

Trying this solved the problem for me:
this.zone.run(() => { this.router.navigate(['/main']); });

Remember to include the import:

import { NgZone } from '@angular/core'; constructor(public zone: NgZone) { }

you saved my day !!!

@jasonc624
Copy link

jasonc624 commented May 16, 2020

Trying this solved the problem for me:
this.zone.run(() => { this.router.navigate(['/main']); });
Remember to include the import:
import { NgZone } from '@angular/core'; constructor(public zone: NgZone) { }

you saved my day !!!

Same, I don't like this hack but it works.

@antontemchenko although i haven't investigated, this started occurring when i implemented NGXS into my project. Perhaps there is a reference of BrowserModule in the library.

@atscott atscott added state: confirmed triage #1 and removed needs reproduction This issue needs a reproduction in order for the team to investigate further labels May 28, 2020
@atscott
Copy link
Contributor

atscott commented May 28, 2020

Confirmed in v9 with ivy: https://stackblitz.com/edit/router-outlet-bug-demo?file=src%2Fapp%2Froot%2Froot-routing.module.ts

@atscott
Copy link
Contributor

atscott commented Oct 8, 2021

Closing as duplicate of #19742. This is a symptom which is due to handling that affects all packages/parts of the framework rather than a router-specific problem.

@atscott atscott closed this as completed Oct 8, 2021
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Nov 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: animations area: router freq1: low P4 A relatively minor issue that is not relevant to core functions state: confirmed type: bug/fix
Projects
None yet
Development

No branches or pull requests