single-spa / single-spa-layout

Layout engine for single-spa applications
MIT License
78 stars 31 forks source link

cancelNavigation is not working properly when user tries to navigate to a page in another app #151

Open kiranmakamhub opened 2 years ago

kiranmakamhub commented 2 years ago

When cancelNavigationis called, it works fine when user tries to navigate to a page in the same app. It unmounts the app, when user tries to navigate to a page in another app.

Template:

<single-spa-router>
    <nav class="topnav">
      <application name="@organization/nav"></application>
    </nav>
    <div class="main-content">
      <route path="settings">
        <application name="@organization/settings"></application>
      </route>
      <route path="clients">
        <application name="@organization/clients"></application>
      </route>
    </div>
</single-spa-router>

Event handler

useEffect(() => {
    window.addEventListener('single-spa:before-routing-event', ({ detail: { cancelNavigation } }) => {
      cancelNavigation();
    });
}, []);

Steps to reproduce: User is on /settings/page1 (this page has the registered the event handler for single-spa:before-routing-event) Navigates to /settings/page2 Navigation gets cancelled, he stays on /settings/page1

Navigates to /clients/page1 (page in another app) Browser url changes to new-url, flips back to old-url Settings app is unmounted.

Versions used: single-spa: 5.9.3 single-spa-layout: 1.6.0

joeldenning commented 2 years ago

The bug here isn't very clear - "navigates" could refer to clicking on a link, pressing back/forward, or manually changing the browser url. The settings app could be unmounting due to an error in React code (react unmounts apps that don't have error boundaries and experience an error) or it could be due to single-spa's navigation.

Please provide a demonstration repository that shows the problem, as that is the easiest to diagnose.

The code you shared has a bug - the event listener for single-spa:before-routing-event is never cleaned up like it should be, so it will persist even after the settings app is unmounted.

tscislo commented 2 years ago

@joeldenning finally after one day of digging I came accross this isse that I'm currently facing. Basically using cancelNavigation in single-spa:before-routing-event works fine for me with single-spa 5.9.3 and both angular and react but when my root config does not use single-spa-layout manager. But when I use it it happens exactly what was described in this issue by @kiranmakamhub

So basically when we try to prevent navigation from within one AppA to AppB (with link click) using cancelNavigation, then AppA is left in some ambiguous state in which it is still mounted, but getAppState returns MOUNTED, however single-spa-layout removes this app form the DOM so at first glance it might look like it is unmounted but this is not the case.

This bug makes cancelNavigation not usable with single-spa-layout It would be great if you @joeldenning could help out please?

etokatlian commented 2 years ago

I can confirm we are also seeing this issue while using single-spa and single-spa-layout as described by @tscislo. There have been some suggested workarounds in another thread but they did not work for us.

Any suggestions/advice would be greatly appreciated! @joeldenning

Thank you!

b-ferrari commented 1 year ago

@joeldenning I'm so sorry to tag you in this, as I'm sure you're super busy.

Unfortunately, I'm also running into this issue with my project. cancelNavigation will cancel mounting the new app, but it also dismounts the old app leaving the user in a completely unmounted state as far as any modules being loaded from a URL are concerned.

I did notice up above you asked for a demo repository. I forked off of the coexisting-angular-microfrontends project, upgraded the single-spa being used to 5.9.4, the single-spa layout to 1.5.2, and was able to reproduce the issue by adding the following code into the app.component.ts of the navbar:

    window.addEventListener("single-spa:before-routing-event", (event: any) => {
      const detail = event.detail;

      if (detail.newUrl.includes("app1") && detail.oldUrl.includes("app2")) {
        console.log("CANCELLED");
        detail.cancelNavigation();
      }
    });

I cancel navigation whenever a user is on app2 and tries to navigate to app1, which causes the bug. That's all here, if you want to look at it at some point: https://github.com/b-ferrari/coexisting-angular-microfrontends.

If it helps at all as well, here is also a gif of the behavior I'm seeing: 2023-05-31_09-24-53