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

missing renavigate method at new angular-router #9669

Closed
gustavolira opened this issue Jun 28, 2016 · 39 comments
Closed

missing renavigate method at new angular-router #9669

gustavolira opened this issue Jun 28, 2016 · 39 comments
Labels
area: router feature: under consideration Feature request for which voting has completed and the request is now under consideration feature Issue that requests a new feature freq1: low
Milestone

Comments

@gustavolira
Copy link

gustavolira commented Jun 28, 2016

Hi,

In angular/router-deprecated we have Router.renavigate(), but at new component of router, I mean angular/router does not have something like Router.renavivate().
Would be nice if we have the same behavior a angular/router.

Thanks

@vicb vicb added feature Issue that requests a new feature area: router labels Jun 28, 2016
@zoechi
Copy link
Contributor

zoechi commented Jun 28, 2016

Do you have a concrete use case?

@gustavolira
Copy link
Author

@zoechi It's simple, just look for Router at angular/router-deprecated package and you will see renavigate method

@zoechi
Copy link
Contributor

zoechi commented Jun 28, 2016

@gustavolira I know it was there but some things are working differently in the new router and perhaps your concrete use case could be accomplished with existing features, and if not it would still be interesting to know your use case to ensure renavigate does what you expect, otherwise it might be sufficient to add a renavigate() method with an empty body ;-)

@PEsteves8
Copy link

Personally I wanted this, because when I use canActivate with a guard, sometimes instead of redirecting to somewhere specific I would prefer to keep the exact same url. If I just return false, the user stays in the same place as expected, but the url changes stays as whatever was inserted (the unaccessible url)

@awerlang
Copy link
Contributor

awerlang commented Jul 1, 2016

@PEsteves8 isn't this a bug then? the route url should reflect the current state in the outlet's.

@PEsteves8
Copy link

Well, if the point is to have the url stay what it was before the blocked route access attempt, then I suppose it it.

@janerist
Copy link

My use case for this is an edit form where you want to reset the form state after saving, and also make sure that your form is up to date with data from the server. Sure, I can use router.navigate(...) and set up an observable and react to changed data and update my form that way, but that introduces a lot of unnecessary complexity when just reloading the current route would achive the same thing a lot simpler.

@damiandennis
Copy link

same problem here, why cant we renavigate anymore? it was a quick solution to reloading all state of a path. Yes I can use observables but then everything has to be hooked up to observables and with a lot of interactions on a page this can get difficult to manage. A simple renavigate made this simple. I think the method reload makes more sense as well.

@0cv
Copy link

0cv commented Jul 19, 2016

Similarly, the point of renavigate is to avoid all kind of boilerplate. I have an edit form, which can be called from multiple places in the application. Using renavigate when the user clicks save (or cancel!) redirects exactly to the place where the user was before. It's possible now to do the same, somehow, but it's (much) more complicated, etc.

@zoechi
Copy link
Contributor

zoechi commented Jul 19, 2016

@Krisa the new forms got a reset() method recently AFAIK

@0cv
Copy link

0cv commented Jul 19, 2016

on the FormGroup or FormControl, something like this

g.reset({'one': 'initial value', 'two': ''});
?

This is not yet in the current RC4, but more particularly, my use case is actually not really about that. I'm looking for a way to navigate back to the last visited url within the application. Kind of history.back() but within Angular2 mechanism (and additionally I don't want to navigate back in the history, but to move forward even so the url will be the same).

@roben
Copy link

roben commented Oct 4, 2016

Same for me. I have a rather complex application which components react on series of global events. If the event history is changed (i.e. undo), it is waay more complicated to do atomic undos than just reload everything with the new event history.

Currently i am working around this by simply calling location.reload();

@QuentinFchx
Copy link
Contributor

I also have the following use-case:
I have an app where you can switch between several "accounts".
Each accounts has different permission, and some routes should be inaccessible to some accounts.

Let's say userA can access routeA, but not userB.
The routeA is protected by a guard redirecting to routeX any unauthorized user.

If I'm on routeA as userA, and I switch to userB, I would like to re-run the guards stack and redirect userB to routeX.

@kblestarge
Copy link

What is the status of this feature request?

@msieurtoph
Copy link

Same usecase as @QuentinFchx ...
I do not understand why renavigate() has been removed.
location.reload() seems to me to be a (way too much) radical solution.

@flensrocker
Copy link

My usecase is the same as @janerist (reload an edit-form after save to reflect the new data).

The form creation is hooked up to the route.params observable, but now I do a combineLatest with some BehaviorSubject<boolean>(true) to trigger an refresh with a call to its next method. That makes the parameters of the map of the Params a lot uglier.

If someone has a nicer way of reloading, please let me know...

export class MyComponent {
  public form: Observable<FormGroup>;
  private _reload: BehaviorSubject<boolean> = new BehaviorSubject<boolean>(true);

  constructor(route: ActivatedRoute) {
    this.form = route.Params
      .combineLatest(this._reload)
      .map((data: [Params, boolean]) => {
        let params: Params = data[0];
        ...
      });
  }

  renavigate(): void {
    this._reload.next(true);
  }
}

@intellix
Copy link

intellix commented Sep 18, 2017

Common use-case: Route has an AuthGuard and when you click Logout in the header, you'd like to re-run guards to know if the user should be on his current page. You need to re-run the guard and currently it requires repeating the code here to find guards, inject and run them:

private runCanActivate(future: ActivatedRouteSnapshot): Observable<boolean> {
const canActivate = future._routeConfig ? future._routeConfig.canActivate : null;
if (!canActivate || canActivate.length === 0) return of (true);
const obs = map.call(from(canActivate), (c: any) => {
const guard = this.getToken(c, future);
let observable: Observable<boolean>;
if (guard.canActivate) {
observable = wrapIntoObservable(guard.canActivate(future, this.future));
} else {
observable = wrapIntoObservable(guard(future, this.future));
}
return first.call(observable);
});
return andObservables(obs);
}

Not saying we need a renavigate API exposed, but perhaps the Guard running API should be exposed so we can just tell them to re-run

It's not enough to just inject the AuthGuard and run it because the user may be allowed on the current page

Workaround 1:

// Route
{
  path: 'account',
  canActivate: [ AuthGuard ],
  runGuardsAndResolvers: 'always',
}

// RouteService
refresh() {
  const queryParams = { reload: 1 };
  this.router.navigate(this.route.snapshot.url, { queryParams });
  setTimeout(() => this.router.navigate(this.route.snapshot.url));
}

Workaround 2:

export class RouteService {

  constructor(private route: ActivatedRoute, private router: Router, private injector: Injector) { }

  recursivelyGetGuardedRoutes(route: ActivatedRoute): ActivatedRoute[] {
    const config = route.routeConfig;
    const returned = config && config.canActivate && config.canActivate.length > 0 ? [route] : [];
    return route.children
      ? returned.concat(route.children.reduce((acc, r) => acc.concat(this.recursivelyGetGuardedRoutes(r)), []))
      : returned;
  }

  getGuardChain(): Observable<boolean> {
    const routes = this.recursivelyGetGuardedRoutes(this.route);
    const obs = routes.reduce((acc: (Observable<boolean> | Promise<boolean>)[], route) => {
      return acc.concat(route.routeConfig.canActivate.map((token: InjectionToken<CanActivate>) => {
        const guard = this.injector.get(token);
        const value = guard.canActivate(route.snapshot, this.router.routerState.snapshot);
        return typeof value === 'boolean' ? Observable.of(value) : value;
      }));
    }, []);
    // Loop through each guard whilst they return truthy
    return Observable.from(obs).mergeMap(o => o).takeWhile(v => !!v);
  }

}

// Usage
userService.logout()
  .switchMap(() => routeService.getGuardChain())
  .subscribe();

@cwalcott
Copy link

The use case I have are components with resolved data. Sometimes I want the resolved data to be reloaded in response to a user action.

For example, a component that lists users and includes the ability to delete a user. After a user is deleted, I want the component to receive an updated set of users without the deleted one.

For now I've been using something similar to "Workaround 1" in intellix's comment though it requires incrementing the "reload" parameter if it exists (to handle multiple refreshes).

@awerlang
Copy link
Contributor

awerlang commented Sep 27, 2017

@cwalcott This is already supported by resolvers. Resolver data is available in the .data field as an observable as you should be already aware. Don't take only the first item, keep a subscription to it. When it's time to reload, make the resolver .next() another item. Of course it takes some more work, but it doesn't need any workaround to do so, it's how it is designed.

@cwalcott
Copy link

@awerlang Don't observables from resolvers have to complete before the component gets created? I tried making a resolver that reloads items based on mapping over a subject, but the component never loads, I think because the resolver's observable never completes:

@Injectable()
export class UsersResolver implements Resolve<User[]> {
  private reloadSubject = new BehaviorSubject(true);

  constructor(private backendService: BackendService) {}

  reload(): void {
    this.reloadSubject.next(true);
  }

  resolve(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable<User[]> {
    return this.reloadSubject.switchMap(() => this.backendService.getUsers())
  }
}

@awerlang
Copy link
Contributor

@cwalcott you're right, sorry for misleading you.
For CanActivate it was the same thing, until I sent a PR to fix #9613.

So what I described earlier is how I pictured it in my mind. I think it needs to work that way. I fail to understand why consume data as an observable in the current approach, it could be the value itself. Probably I'm missing something. There weren't much prior art for observable-based routers, so this must have been a slip in the design.

I found this PR #19101, although I don't think it's a good solution for resolve (i.e. take the first value).

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

Any update on this?

@flensrocker
Copy link

Another workaround we're using, is

import { Component } from '@angular/core';
import { Router, ActivatedRoute } from '@angular/router';

@Component({
    template: '<p>redirecting...</p>'
})
export class RedirectComponent {
    private returnUrl: string;

    constructor(
        private _router: Router,
        private _route: ActivatedRoute,
    ) {
        if (this._route.snapshot.paramMap.has("returnUrl")) {
            this.returnUrl = decodeURIComponent(this._route.snapshot.paramMap.get("returnUrl"));
        }
    }

    ngOnInit() {
        if (this.returnUrl) {
            this._router.navigateByUrl(this.returnUrl, { skipLocationChange: true });
        }
    }
}

...

const appRoutes: Routes = [
    { path: "redirect", component: RedirectComponent },
];

...

this._router.navigate(["/redirect", { returnUrl: this._router.url }], { skipLocationChange: true });

It's usally so fast, that you don't see the content of the template.

@mlc-mlapis
Copy link
Contributor

... there is onSameUrlNavigation for that ... you can read #21115

@alex7egli
Copy link

Any updates on this? I have a global state that affects all the data on my current route, and if the state changes then all components need to refresh all their data. It would be a huge burden to add code to every single component to listen for this state change and reload everything, as opposed to how it worked in angularJs where I could just do $route.reload().

@mlc-mlapis
Copy link
Contributor

mlc-mlapis commented Aug 9, 2018

@alex7egli ... do you expect to create new components instances?

@alex7egli
Copy link

@mlc-mlapis Yes, I want the same functionality as if I went to a different route, then came back to the current one. $route.reload() worked great in angularJS and is very much needed in Angular. I need to refresh my UI from cached data I already have, so I don't want to actually reload the window because that would take much longer and is overkill when I just want to re-render my components.

You can see from googling this issue that many other people have the same use case as I do: a global state change occurs that affects all displayed information, so we have to re-render everything but don't actually need to retrieve new data. There are a few workarounds presented but none are very good. They all involve manually listening for a change and invoking a refresh function in every component that needs to refresh. Not only is this a ton of boilerplate for something that should be simple, it's very likely to produce bugs as you have to remember everytime you write a component whether it should be listening for this global state change or not.

@mlc-mlapis
Copy link
Contributor

@alex7egli ... can you describe more closely how do you pass that global state to those components ... because it's unclear for me ... related to your point of view for the next re-initialization.

@alex7egli
Copy link

@mlc-mlapis I have a local data cache and the various components query the cache for data as needed, based on the component. On initialization most components check a global service for the current state. I don't want to go into too much detail because it isn't relevant to this issue, which is that Angular Router should have a reload type method that reloads the current route.

@mlc-mlapis
Copy link
Contributor

mlc-mlapis commented Aug 9, 2018

@alex7egli ... yep, that's the difference. You see it as a problem and others not because they suppose from the very beginning that the state could be async (thinking that everything is async in fact). So from the very beginning, there is a subscription to it ... and any state change means automatic re-initialization (or any other type of reaction) and from that point of view, no problem exists. Destroying and re-creating the instances is also a performance hit of course. So that's the answer why such a method like .reload() doesn't exist.

@alex7egli
Copy link

@mlc-mlapis So if you don't want to implement this feature because you can't understand the need for it, why is this ticket in the backlog and not closed? It's not like devs stuck using Angular aren't used to the response "Angular just doesn't work that way, switch to React".

@mlc-mlapis
Copy link
Contributor

@alex7egli ... I was talking about the current state. As I understand ... there are some priorities (as Ivy renderer #1 actually) and the final decision has not yet happened ... so this feature issue is waiting for it ...
.renavigate() method and also .reload() (simplify programmatic reloading of the same route ... which keeps the component instance but could re-run its guards and resolvers).

@MatthiasKunnen
Copy link
Contributor

MatthiasKunnen commented Nov 28, 2018

I've updated @intellix's workaround for Angular 7

import { Injectable, InjectionToken, Injector } from '@angular/core';
import { ActivatedRoute, CanActivate, Router, UrlTree } from '@angular/router';

import { from, Observable, of } from 'rxjs';
import { map, mergeMap, takeWhile } from 'rxjs/operators';

// Todo Update when https://github.com/angular/angular/issues/9669 is resolved.
@Injectable()
export class RouteService {
    constructor(
        private injector: Injector,
        private route: ActivatedRoute,
        private router: Router,
    ) {
    }

    recursivelyGetGuardedRoutes(route: ActivatedRoute): ActivatedRoute[] {
        const config = route.routeConfig;

        // Check if current route has a canActivate guard
        const result = config && config.canActivate && config.canActivate.length > 0
            ? [route]
            : [];

        if (!route.children) {
            return result;
        }

        return result.concat(route.children
            .reduce((acc: Array<ActivatedRoute>, r: ActivatedRoute) => {
                return acc.concat(this.recursivelyGetGuardedRoutes(r));
            }, []));
    }

    /**
     * This returns an observable which will run all the canActivate guards of
     * the activated route and its children. The observable emits the first
     * false value or true.
     * @returns {Observable<boolean>}
     */
    runGuardChain(): Observable<boolean> {
        const routes = this.recursivelyGetGuardedRoutes(this.route);
        const obs = routes.reduce((
            acc: Array<Observable<boolean | UrlTree>>,
            route: ActivatedRoute,
        ) => {
            if (!route.routeConfig || !route.routeConfig.canActivate) {
                return acc;
            }

            return acc.concat(route.routeConfig.canActivate
                .map((token: InjectionToken<CanActivate>) => {
                    const guard = this.injector.get(token);
                    const value = guard.canActivate(
                        route.snapshot,
                        this.router.routerState.snapshot,
                    );

                    if (typeof value === 'boolean' || value instanceof UrlTree) {
                        return of(value);
                    } else {
                        return from(value);
                    }
                }));
        }, []);

        // Loop through each guard whilst they return thruthy
        return from(obs).pipe(
            mergeMap(o => o),
            map(v => v === true), // When a UrlTree is passed, treat as if false was passed
            takeWhile(v => v),
        );
    }
}

@gund
Copy link

gund commented Jun 28, 2019

This feature is really required for the next use case:

Suppose we have a subsection (a feature) in app that describes RealEstate and everything related to it.

So the url will be: /realestate/:id.

We will have a RealEstateResolver that will sit on path /realestate/:id and fetch data from network.

Then we have multiple subsections where we:

  1. /realestate/:id/data - Display and edit RE data
  2. /realestate/:id/other - Display something relevant to RE (ads, brokers, etc.)

But when we edit our RE the new data will not be reflected across the sections because the resolver is stuck and there is no way to refresh it (apart from going briefly outside of /realestate/:id path and then back).

Also note that none of runGuardsAndResolvers options can help here - since we do not want to refetch RE between every sections navigation, but only when we do specific action (editing of RE).

This is the perfect use-case of Router#renavigate() method.

@pauldraper
Copy link
Contributor

pauldraper commented Jul 8, 2019

There are several workarounds to achieve selectively reloadable values.

None of them look like the router was meant for it.

export class Reloadable<T> {
  private readonly reload$ = new BehaviorSubject(undefined);
  readonly value$ = this.reload$.pipe(switchMap(this.f));
  constructor(private readonly f: () => Observable<T>)  {}
  reload() {
    this.reload$.next(undefined);
  }
}

@Injectable()
export class MyResolver implements Resolve<Reloadable<number>> {
  resolve(route, state) {
    const reloadable = new Reloadable(() => of(Date.now()).pipe(delay(1000)));
    return reloadable;
    // or, if you need the value to exist immediately the first time
    // return reloadable.value$.pipe(mapTo(reloadable), take(1));
  }
}

@Injectable()
export class MyConsumer implements OnDestroy {
  private readonly destroy = new Subject();
  private readonly myReloadable = this.route.data.pipe(pluck('key'));

  readonly value$ = myReloadable.value$;
  
  constructor(private readonly route: ActivatedRoute) {
    someChange$.pipe(
      withLatestFrom(this.myReloadable), 
      takeUntil(this.destroy)
    ).subscribe(([_, reloadable]) => reloadable.reload());
  }

  ngOnDestroy() {
    this.destroy.next();
  }
}

Ideally, the router would be changed so that resolver Observables could emit multiple values, routing would happen after first emitted value, and other values would trigger .data changes.

(Currently, the returned Observable must complete before routing happens.)

Honestly, I think the router is unusable enough that I don't try to use it at all for loading data, and instead use DI injected stateful services provided in the component tree, plus defaults/empty state.

@theLeftTenant
Copy link

I like the resolver/guard concept, but without being able to reevaluate after conditions change, you're at a loss in common scenarios such as details-edit.

What if we could control this declaratively in the route config, which seems most conventional, instead of with the imperative AngularJS-style router.reload()?

{
    path: 'widgets/:id',
    resolve: { widget: WidgetResolver },
    children: [
        {
            path: 'details',
            component: WidgetDetailsComponent,
            children: [
                {
                     path: 'gizmos',
                     component: GizmosPickerComponent,
                     rerunResolver: {
                         atPath: '../../', // relative or absolute
                         upon: 'leave'     // 'leave' || 'enter' etc.
                     }
                }
            ]
        },
        {
            path: 'edit',
            component: WidgetFormComponent,
            rerunResolver: {
                atPath: '../',  // relative or absolute
                upon: 'leave'   // 'leave' || 'enter' etc.
            }
        }
    ]
}

@atscott
Copy link
Contributor

atscott commented Jun 19, 2020

I believe this could be resolved if #23152 were merged, right? You would also need to provide a custom RouteReuseStrategy if you wanted components to reload (i.e. shouldReuseRoute returning false).

@angular-robot angular-robot bot added the feature: under consideration Feature request for which voting has completed and the request is now under consideration label May 21, 2021
@atscott
Copy link
Contributor

atscott commented Jul 23, 2021

Closing since it's unclear to me exactly what this feature request is for since the available configurations and features in the Router have changed significantly since 2016. The relevant items in the router could be onSameUrlNavigation, RouteReuseStrategy, RunGuardsAndResolvers, returning navigation extras when returning a UrlTree from a guard (#27148), or just simply triggering a new navigation from within a guard/resolver.

If there is no existing feature or feature request that covers some needed use-case here (potentially reloading on an individual navigation call as was in #23152), please file a new issue so it can be re-prioritized.

@atscott atscott closed this as completed Jul 23, 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 Aug 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: router feature: under consideration Feature request for which voting has completed and the request is now under consideration feature Issue that requests a new feature freq1: low
Projects
None yet
Development

No branches or pull requests