add refresh method to router service

[Gaurav0]

Part of issue #629

Rendered

[mehulkar]

Should this also deprecate Route#refresh()? And/Or change its implementation to delegate to RouterService?

[rwjblue]

@mehulkar Route.prototype.refresh is a tad different than this new method being proposed. It refreshes whatever route you call it on and all routes “below” it in the route hierarchy. The method proposed here always refreshes all routes.

That being said, I think it would be good to mention the service method from the route documentation, to make it super easy for folks to see there is a service mechanism to do this also.

[acorncom]

It refreshes whatever route you call it on and all routes “below” it in the route hierarchy. The method proposed here always refreshes all routes.

Would we want to be opting for different behavior and refreshing all routes or is the implicit goal here to refresh only the current route (as defined from the perspective of the caller)? The “defined from the perspective of the caller” part seems quite tricky to implement well (and consistently) ...

[Gaurav0]

Should this also deprecate Route#refresh()? And/Or change its implementation to delegate to RouterService?

In my opinion, no. We should keep it around until we are sure we have filled in all the gaps. There may be a use case for refreshing an only some of the active routes, possibly.

[Gaurav0]

@rwjblue revised.

[mike183]

@rwjblue @acorncom I think it would be useful to have a method on the router service for refreshing the current route (including child routes).

I had a recent use case for providing a “Try Again” button when a route failed to load due to a timeout.

I did look for some method of refreshing the route but ended up resorting to just redirecting to a parent route which so happened to redirect back to the route that I wanted to refresh.

[Gaurav0]

@mike183 How about adding an optional routeName argument as proposed in #592 ?

[mike183]

@mike183 How about adding an optional routeName argument as proposed in #592 ?

That seems like it would work 👍

[Gaurav0]

@mike183 @rwjblue Updated. Please rereview.

[Gaurav0]

@rwjblue Thanks again for the fast review.

[mike183]

Apologies for the delayed response.

The updates all look good to me, nice work 👍

[mehulkar]

Re-reading this after a while, does my previous question/suggestion apply more, now that pivotRouteName is introduced? In other words, could Route.prototype.refresh be implemented as:

{
  refresh() {
    routerService.refresh(this.fullName)
  }
}

[rwjblue]

Re-reading this after a while, does my previous question/suggestion apply more, now that pivotRouteName is introduced? In other words, could Route.prototype.refresh be implemented as:

{

  refresh() {

    routerService.refresh(this.fullName)

  }

}

Ya, I think that makes sense, though I'm not sure it really matters from an RFC perspective (since it doesn't really change any existing API surface, it just makes the implementation make more sense).

[MariannaAtPlay]

This is a great idea! Wondering what's the timeline for implementing this in Ember core?

[snewcomer]

@MariannaAtPlay This was implemented in 4.1.0-beta.1.!

https://github.com/emberjs/ember.js/blob/master/CHANGELOG.md#v410-beta1-november-19-2021