single-spa / single-spa-angular

Helpers for building single-spa applications which use Angular
Apache License 2.0
203 stars 78 forks source link

SkipNextPopState = true is preventing initial pushState to take place #526

Closed greg-signi closed 2 months ago

greg-signi commented 2 months ago

Question

The first history.pushState(..) dispatched by other "mfe" is not being consumed by my angular spa, because of this snippet since skipNextPopState is set to true. After that is false and I can already grab the same history.pushState(..) which is triggered by a Nav button.

const onPopStateListener = (event) => {
            // The `LocationChangeEvent` doesn't have the `singleSpa` property, since it's added
            // by `single-spa` starting from `5.4` version. We need this check because we want
            // to skip "unnatural" PopStateEvents, the one caused by `single-spa`.
            const popStateEventWasDispatchedBySingleSpa = !!event
                .singleSpa;
            if (this.skipNextPopState && popStateEventWasDispatchedBySingleSpa) {
                // Enters here!
                this.skipNextPopState = false;
            }
            else {
                fn(event);
            }
        };

What can I do to stop prevent this behaviour ?

Environment


Libs:
- @angular/core version: 17.2.0
- single-spa-angular version: 9.0.1            
greg-signi commented 2 months ago

Sorry for disturbing you @arturovt , but you seem to be the only help still around here 😨

arturovt commented 2 months ago

Is there a way to reproduce it in some playground?

greg-signi commented 2 months ago

I could try and set it up but I don't have time now.. but I guess my question is why is this skipNextPopState needed ? Also this variable popStateEventWasDispatchedBySingleSpa does it mean it was dispatched by my angular-spa ? Because if it is that's wrong since it was another spa that holds the nav buttons that promotes the history.pushState()..

arturovt commented 2 months ago

This was done to prevent infinite navigation loops because both single-spa and Angular listen to the popstate event. This simultaneous listening would trigger their internal navigation processes and history calls, potentially causing an infinite loop.

greg-signi commented 2 months ago

Okay, but how do you know you are not ignoring valid navigation events ? Should we just discard the first navigation event always ?

arturovt commented 2 months ago

No, this is only ignoring events dispatched by single-spa, and not the actual browser.

greg-signi commented 2 months ago

Well I managed to find a workaround with some insight found on this thread , but I still don't understand why is this was necessary in the first place...

So I added the withDisabledInitialNavigation() to stop the initial navigation that was triggering the replaceState

providers: [
    provideRouter(APP_ROUTES, withDisabledInitialNavigation()),
    provideHttpClient(withInterceptors([antiforgeryTokenInterceptorFn]))
  ]

And for my initial navigation event I refused to replace the history state, which avoided the SkipNextPopState = true; scenario I was having issues with:

this.router.navigate(['/'], { replaceUrl: false, skipLocationChange: true });

Now I wouldn't have issue with replaceState if not the case of SkipNextPopState = true and causing to always ignore the navigation event it would follow after..

Any thoughts on the matter ? Why did I have to fallback to this ?

Thanks for your time!!