-
-
Notifications
You must be signed in to change notification settings - Fork 79
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
URL Redirection Infinite Loop #113
Comments
I do not have time to work on this issue - outside contributions would be greatly appreciated. |
@AndrewGlazier I have fixed this issue by adding the onSameUrlNavigation: 'ignore' option to router module in angular. |
Oh wow thank you for sharing that @rknash18. It would be great if that workaround works for everyone else's issues they've been having, too. I've commented in #94 asking others to test if this works for them. If it does, let's look into incorporating this into single-spa-angular so that all benefit from it. |
Hi @joeldenning , @rknash18 , The solution provided in the comment does not work for me I m stilling using the changeDectection ref as an alternative. |
@ashuaggarwal94 I am not sure about the issue you are experiencing , I have fixed issue with router trying go back on forth when I clicked on a menu option. |
I identified that this loop starts in reroute.js file Edit 1: Also find out that it happens because of replaceState in navigation-event.js file, which calls reroute.js above methods.
Default behavior:
The conflict occurs between following routes |
My current guess is this line. I think the second line |
Thanks for helping to debug this - why would single-spa's code have the old url? Angular router should call |
What I meant was that single-spa reroute is happening during an router change, which causes the bug, but not sure about it at all. It feels like single spa is processing during another route change. |
Test it out for about 2 hours. I'm creating an repo with the bug! |
Created an repo that reproduces the bug @joeldenning |
I am not going to look at this bug anytime soon. I do not use Angular for any projects ever and this bug is a hard one when I tried to fix it in the past. The only path forward for this to be fixed is community contribution. |
Notice that the problem is in the following line @TheMcMurder (git blamed this code). I just need to know why is this code there. Because I not sure what might happen if I just delete it. And what you suggest, should I try to get why is pendingPromises making angular start a loop or just do something to fix this code (still don't know what)? |
Full disclosure: I don't use angular for any projects. I don't recall specifically why that line of code exists, but this is a really complicated interaction between two different libraries: angular-routing, and single-spa. They both operate with different ideas about how routing is to be accomplished and who is in control of routing. I'll try to take a look at the repo you created, but if I'm understanding correctly it's only consistently reproducible in IE which will make debugging extremely hard (I use linux as my daily driver). I wouldn't suggest deleting lines of code in single-spa as a solution. I know it's a slow response and I thank you for your continued patience. |
@TheMcMurder Actually is totally reproducible in Chrome / Firefox (using Windows at least). The bug happens 100% in my repo using the following settings: IE: I didn't tried. PS: I removed that line from single-spa and builded to test and works just fine |
Hi, I can trigger the loop in my apps with Chrome and Ubuntu too. |
Also can confirm this happens in several of our angular micro-apps, running both the latest chrome and firefox nightly. It doesn't happen on every navigation event, but when it does happen it will happen consistently for a particular one. This is something that I'm investigating as well, I'll post here if I figure anything out. |
the redirection infinite loop stills exist.any solutions? |
I try all solutions above,but failed. so i spend one day to solve the problem.luckily,it is fixed for me. |
@gubertcalixto I add ‘isNavGoOrBack‘ ,that just make a distinction between click back-forward in browser and normal route. because the popstate event will be triggered by doing a browser action such as a click on the back or forward button (or calling history.back() or history.forward() in JavaScript). |
@gubertcalixto I test your repo,it works after using my update. |
This issue only happens if user click on two different single spa link in a fraction of second. To fix this issue I disabled the links until any of the single page application is loading. When any application loads via single-spa, it runs below events at start and end.
|
@kisslove maybe you could create an PR with your correction for single spa. The bugs that I know/heard about was:
PS: If you don't do it I probably will do in the next month (on vacation right now 😎) |
@pankajparkar The cases where I see it is when the back button is pressed or when doing a single direct navigation from using the router (not triggered by user interaction). It's possible though that either of those are causing cascading navigation events (guards, redirects, etc), so the theory of "too many navigation's too quickly" is valid. I'm using the fork by @kisslove and it appears to be working, I did notice one time where the redirects started for a couple of bounces and then resolved themselves quickly, but that was single time out of several. So far I haven't seen any other negative side-effects or regressions within my apps from it. |
@joeldenning @arturovt After the version 3.4.1 fix of infinite redirection, it came back haunting again in our app. But this time it was weird, as it was not happening consistently but intermittently. Seeing the above comments as @DmitryBrus said it is happening when there are two close redirection routing in the angular app. Since in a more complex big app, it is quite visible and pertinent but I tried creating a small reproducible version of the problem here in this cloned template of coexisting-angular-frontend repo. In /app1 I have used Angular v8 and single-spa-angular v3.6.0 and added a route named /test1 & /test2 in it. I have provided the link to Test1 Component in app.component.html and in Test1 component I have added a router navigation to Test2 component under ngOnInit. Observe that it happens only in the first time of app start, that when you will click on Test 1 it will go to test2 then back to test 1 then back to test2. Also, if you put a debug point on ngOnInit router navigate command in Test1 Component, you will notice that it is being paused infinite times. Hope this helps and we can get a solution of this. |
Thanks for the repo demonstrating the issue. I do not plan on working on this issue, but welcome anybody who does to discuss it here and/or submit a pull request. |
Any updates on this? I'm still facing this issue which causes infinite loop between two routes. |
@plrthink You can try imports: [RouterModule.forRoot(routes, { onSameUrlNavigation: "ignore" })], in angular routing module it worked for me!! maybe you can give it a shot!!! |
Well, I don't think it would work since 'ignore' is the default value of the option onSameUrlNavigation. |
And it's indeed not working after testing. |
Hey guys, we have released the fix in 4.6.0 version. Could anyone check if it resolves your issue? @varora1406 could you also check in your example? |
Closing since the fix is published in 4.6.0 - we can reopen if that fix doesn't solve all cases. |
Hello, single spa crew. |
Could you paste your main.single-spa.ts? |
import { enableProdMode, NgZone } from '@angular/core'; import { platformBrowserDynamic } from '@angular/platform-browser-dynamic'; import { singleSpaAngular, getSingleSpaExtraProviders } from 'single-spa-angular'; import { AppModule } from './app/app.module'; if (environment.production) { const lifecycles = singleSpaAngular({ export const bootstrap = lifecycles.bootstrap; |
I see, could you add NavigationStart to options in the same way you pass NgZone and Router? |
@arturovt You are truly an everyday superhero. |
@arturovt Doesn't resolve this issue for our team too. For easier reproduction try setting CPU Throttling in Performance tab in developer settings. I've also cloned @varora1406 repo, updated packages, added NavigationStart to start method and i can still reproduce this. |
I've just cloned @varora1406 repo, updated packages, added NavigationStart to options and I can't reproduce it anymore (tho I could easily reproduce it earlier): Could you provide a minimal reproducible example? I also tried this script which clicks links 200 times automatically: function sleep() {
return new Promise(resolve => setTimeout(resolve));
}
window.tryToReproduce = async () => {
// Only app1 and app2
const links = [...document.querySelectorAll('navbar-primary-nav li a')].slice(1);
for (let i = 0; i < 200; i++) {
console.log('Navigating...');
links[0].click();
await sleep();
links[1].click();
await sleep();
}
}; And ran it from my browser via |
@arturovt Are you sure you're using issue-inifnite-url-loop branch? |
Hi guys, sorry I have been away for some time. I have actually switched from Angular to React from quite some time 🙂 @arturovt I can't see button added by me in your screenshot. Please confirm if you are checking at |
@arturovt My bad. I did everything again from the beginning, by the book on I've also solved our team issue with the loop. As we used angular sub app inside angular main app i had to also patch router in the main app using Thank you for your time, and again sorry for the fuzz. |
@kmiasko I'm glad that you've solved it! |
single-spa-anggular 4.x this question.how to fix infinite loop by single-spa-angular 3.x ? |
single-spa-angular@3 doesn't have it fully fixed - see https://github.com/single-spa/single-spa-angular/pull/235/files#diff-d41e9b77a99ac86d9908fc97b9df8578479c310a0fe85f632f375b2d9e95f39cR202. We need to make a corresponding change in the 3.x branch to fix it. Would you be interested in submitting a pull request with it? |
Hi @chenji336 @joeldenning I will submit a pull request sometime this week implementing the fix for the 3.x branch. Something of note is that this solution in single-spa-angular alone won't solve 100% of issues. I have a base project, which is an angular module, where singleSpa.start() is called. My parcels are subsequently mounted onto a component of this project. In the base project, during a navigation from my login component to another component, I was getting an infinite loop related to this issue. The infinite loop behaviour stopped when I removed the call to singleSpa.start(). I had to implement the non-imperative skipping logic in my base application as well. |
Adding
It is also mentioned in docs https://single-spa.js.org/docs/ecosystem-angular/#api-updates |
Issue seems to relate to this comment;
#94 (comment)
Creating a new ticket as I cannot reproduce using the browser back button.
This bug is reproducible in the vanilla example for coexisting angular micro-frontends found here:
https://github.com/joeldenning/coexisting-angular-microfrontends
Reproduction steps;
Using the navigation app, trigger a route change, and then quickly trigger another nav change.
Issue is reproducible on;
IE11 (Very common)
Firefox (Mediocre-Hard)
Chrome (Rare)
Notes;
In our application we have found that having more simultaneous angular apps running in parallel triggers the issue more commonly, however this bug is reproducible with only one app actively loaded by the single-spa (however with difficultly even in IE11). (Issue does not occur at all with the app running in angular standalone without the spa)
The issue appears to be also related to Router.forRoot in some way, upon removing this from an app, the issue becomes more difficult to reproduce even with IE11, a similar effect to whether the app wasn't running.
This seems to be related to the speed of the navigation. Navigating twice very quickly causes this effect.
If struggling to reproduce, we found that having a button which changes to swap between two pages and spam clicking it causes the bug to reproduce on IE11 every time.
The navigations do not need to be between separate apps, routing between two pages on the same app causes the issue.
The text was updated successfully, but these errors were encountered: