Platform: Router Store does not wait for deactivate guards to finish

[djhojd]

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

What is the current behavior?

The router state is updated immediately when you start a new router navigation.

Expected behavior:

The router state should be updated when navigation ends, maybe?! or at least wait for all your guards that delay or stop your navigation and check the result.

Minimal reproduction of the problem with instructions:

1. Open https://stackblitz.com/edit/angular-fa311h
2. Click Comp2
3. Click Comp1
4. Check router state - your state is updated with new location but your navigation was canceled because of the guard

[brandonroberts]

This is by design. Router store emits the action when navigation starts before guards and resolvers have run, giving you an opportunity to stop navigation before running those unnecessarily.

[bbaia]

OK for the action.
But the router state should be updated only after guards, no ?

[djhojd]

Well, maybe I misunderstood what router-store is all about. IMHO ngrx/router-store should help you move the angular router state into the ngrx/store so normally you would think that they are in sync. Your application is using the app state as the single source of truth and in my example that source is false because I didn't left that location so I cannot trust the router-store to be able to compute my data that needs to be displayed in my application. Maybe, a more appropriate behavior would be to include in the router state the currentState and also the nextState of the angular router?! and let people decide which of these they want to compute data from.

@bbaia I was thinking the same.

[bbaia]

Maybe @vsavkin has an opinion on this :)

[adrianfaciu]

Not sure why this would be as it is 'by design', as soon as we add guards to our routes it defeats the whole purpose of having it. State is already updated with the new route snapshot but the app is not, so one cannot reliably use it.

I would rather see this improved, than each of us implementing its own solution or a different lib.

[MattiJarvinen]

@adrianfaciu see https://stackblitz.com/edit/ngrx-router-reducer-can-deactivate?file=app%2Fservices%2Frouter-store-extension.service.ts it's hacky but might be of some use to you.

[mkurcius]

I have similar problem, in first view I am updating query params and has link to second view. When user navigate to second view, canActivate guard may prevent it and route is not changed, but router store has been changed.
In this scenario router and router store is out of sync.

Demo:
https://stackblitz.com/edit/ngrx-router-store-bug
(click "Navigate to component 2" to see effects)

[MattiJarvinen]

@mkurcius hmm... your stackblitz link doesn't seem to work.

[mkurcius]

I just check it and it works

[MattiJarvinen]

@mkurcius odd now the link works for me... maybe some glitch/delay in their system.

[trumbitta]

@brandonroberts

[...] giving you an opportunity to stop navigation before running those unnecessarily

Ok, but how? Is there a way to intercept ROUTER_NAVIGATION with an effect?

[trumbitta]

Of course there is, but the state still doesn't get updated and keeps showing the cancelled one.

[jamOne-]

You can always create an "ugly" meta-reducer that runs normal reducer first and then itself. It can remember previous router state and update it with every ROUTER_NAVIGATION action and then when ROUTER_CANCEL or ROUTER_ERROR occurs it can replace current router state with previous (remembered) one.

Note that ngrx/router reducer updates router state even with ROUTER_CANCEL, that's why meta-reducer is called after the normal one.

This solution works, however I don't like it. I think that ngrx/router should take care of routing cancels.

[jwerts]

Running into this issue as well and its very frustrating as it seems to go completely against what would be expected. This meta-reducer seems to work for correcting the state as @jamOne- suggested:

export function fixRouterStoreCancel(reducer: ActionReducer<AppState>): ActionReducer<AppState> {
  let preCancelState: RouterStateUrl;
  return function (state: AppState, action: any): AppState {
    let currentAction = action;
    if (state && state.router && action.type === ROUTER_NAVIGATION) {
      preCancelState = state.router.state;
    }
    if (action.type === ROUTER_CANCEL || action.type === ROUTER_ERROR) {
      currentAction = {
        ...action,
        payload: {
          ...action.payload,
          routerState: preCancelState
        }
      }
    }
    return reducer(state, currentAction);
  }
}

However, I'm still having issues with ROUTER_NAVIGATION updating the store and calling effects before my CanDeactivate guard ends which is causing functions to be called that should only be called if the component is deactivated. It seems like ngrx should be taking care of this kind of thing internally.

How can I tell if ROUTER_NAVIGATION is waiting for a guard to finish (confirm dialog in my case) to see if it should run the effect or not?

[creativeux]

I'm struggling with this as well. Needing to implement an inline hack to accomplish this rather than relying on ngrx to wait for the guard to reject the route. ROUTER_NAVIGATION fires, state updates, ROUTER_CANCEL fires. This is out of order.

[jwerts]

I ended up combining my effect w/ the NavigationEnd event to make sure it doesn't fire on the initial ROUTER_NAVIGATION that fires before the ROUTER_CANCEL. This kind of works except that it misses the initial loading of the page where I also wanted the effect to call, so I had to create a workaround there. All of this feels pretty hack-ish.

  @Effect({ dispatch: false })
  onNavigateBase$ = this._actions$.pipe(
    ofType(ROUTER_NAVIGATION),
    combineLatest(this._router.events),
    // make sure we don't process this until route guards have completed
    filter(([action, routerEvent]) => routerEvent instanceof NavigationEnd),
    withLatestFrom(this._store.select(s => s)),
    tap(([[action], state]: [[RouterNavigationAction<RouterStateUrl>, RouterEvent], AppState]) => {
      const params = action.payload.routerState.params;

      const routerRegionCode = getRegionCode(params.regionCode);
      if (routerRegionCode !== state.userState.regionCode) {
        this._store.dispatch(new SetRegionAction(routerRegionCode));
      }

      const routerJobPhase = getJobPhase(params.jobPhase);
      if (routerJobPhase !== state.userState.jobPhase) {
        this._store.dispatch(new SetJobPhaseAction(routerJobPhase));
      }
    })
  );

[dummdidumm]

@brandonroberts If I'm correct this issue can be closed, fixed via #1267

[brandonroberts]

Thanks @dummdidumm