Please improve migration guide and describe what is the replacement of go, show, replace, back actions, because they are not in router-store anymore.
I would also like to know what the migration path is for code that uses these v2 actions. Replace with usages of Router everywhere? Even in effects?
There isn't a replacement for those actions in V4 and that's for consistency. You should use the router navigation syntax as normal. If you're using the actions in effects, you would inject the Router and use it to navigate rather than dispatching an action that tells the router to navigate.
If you're not able to migrate these at once, another option would be to copy the actions into your project and set up an effect that does the navigation. I will add a path to V4 in the migration guide.
brandonroberts, in such case, as StevenLiekens asked, how about navigating inside effect? Previously I was using:
.mergeMap(payload => {
// ....
return Observable.from([
new Action1(),
new Action2(),
go("/")
]);
})
Now, as you recommended this.router.navigate(["/"]) returns promise, not an action.
Will it work the same if used like this with v4?:
.mergeMap(payload => {
// ....
this.router.navigate(["/"]);
return Observable.from([
new Action1(),
new Action2()
]);
})
@zygimantas I switched to Actions.ofType(whatever).do(navigation here) in v4. I think you should too.
.mergeMap(payload => {
// ...
return Observable.from([
new Action1(),
new Action2()
]);
})
.do(() => this.router.navigate(["/"]))
I can't tell if this code runs the reducers first or the navigation first, but at least you don't have an impure mergeMap when you move the navigation to do().
@StevenLiekens suggestion is a good one
Wasn't the whole point of using the go, back, etc Actions to enable time traveling with the redux dev tools extension?
If everyone is adding these work arounds back in manually, does it not make sense for them to be provided by ngrx? Adding the workarounds isn't that hard, I'm just wondering what it means that they were removed for "consistency"?
@Matmo10 @brandonroberts @StevenLiekens @zygimantas
I think the old actions are nicer as you don't need to inject the router into all your effects, you just return a new action. This helps with testing as it cuts down on the number of mocks you need. On a project I am working on we have an notification, error, and toaster services that we want to convert, to mimic the way the old router actions worked, for that exact reason -> removing dependencies from the effects. It's much nicer when they just contain just the services they get data from IMHO.
@brandonroberts I appreciate you want consistency but I feel the old way where you always just dispatch an action as the result of your effect, is a better more consistent pattern.
.do(() => this.router.navigate)
feels like a bit of an anti pattern to me
Thoughts? Happy to be convinced as to why I might be wrong what angle I am missing.
@vespertilian I understand where you're coming from as the action creators are nice to have to hide away the router from your effects, but as I mentioned its at the cost of consistency and maintenance. On the consistency side, you have any extra layer on top of the router syntax to navigate with. From the package's perspective, wrapping the router navigation APIs didn't provide enough value versus the overhead of having to update them to keep up with the router navigation APIs.
@Matmo10 you can still use the Devtools to time-travel with router-store if you use the provided routerReducer.
@brandonroberts Thank's for the in depth explanation. I will create my own wrapper.
I used go when handling errors to navigate to an error page, but I would also pass along some error details by doing something like
map(err -> {
return go(['/error'], { error: err })]
})
The payload isn't passed to the do function, only the original action. So what is the preferred approach here? Only though I have is to manage the error state in the store and dispatch an error action which stores the details and then retrieve them in the error page. Is this the best approach or is there something better?
@swargoletCts The do/`tap operator does get the emitted item:
this.actions$.ofType('example').do(action => { ... })
@MikeRyanDev Thanks! Had a complete brain fart, not sure why I didn't even think to move the do up so it would receive the error and pass it along.
@StevenLiekens #
In your code snippet
.mergeMap(payload => {
// ...
return Observable.from([
new Action1(),
new Action2()
]);
})
.do(() => this.router.navigate(["/"]))
The "navigate" will get called twice because you have two actions in the observable, even though the route is the same . Is there a way to "do" the navigation only once ?
Good point. I think you can use finally instead of do. The difference is that finally only gets called once at the end of effect instead of once for each continuation action.
.mergeMap(payload => {
// ...
return Observable.from([
new Action1(),
new Action2()
]);
})
.finally(() => this.router.navigate(["/"]))
(untested)
Hi Steven,
It works perfectly, thank you!
On Mon, Jan 8, 2018 at 7:50 AM, Steven Liekens notifications@github.com
wrote:
Good point. I think you can use finally instead of do. The difference is
that finally only gets called once at the end of effect instead of once
for each continuation action..mergeMap(payload => {
// ...
return Observable.from([
new Action1(),
new Action2()
]);
})
.finally(() => this.router.navigate(["/"]))—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/ngrx/platform/issues/107#issuecomment-355958103, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ABkBqAU9q6VFdQKr5y-_Z7Ap2WguNuQ4ks5tIg8hgaJpZM4OcmOk
.
--
Sincerely,
Ningze Dai
email:[email protected]
tel: 347 404 0985
skype:dnzyt1986
Most helpful comment
@Matmo10 @brandonroberts @StevenLiekens @zygimantas
I think the old actions are nicer as you don't need to inject the router into all your effects, you just return a new action. This helps with testing as it cuts down on the number of mocks you need. On a project I am working on we have an notification, error, and toaster services that we want to convert, to mimic the way the old router actions worked, for that exact reason -> removing dependencies from the effects. It's much nicer when they just contain just the services they get data from IMHO.
@brandonroberts I appreciate you want consistency but I feel the old way where you always just dispatch an action as the result of your effect, is a better more consistent pattern.
.do(() => this.router.navigate)
feels like a bit of an anti pattern to me
Thoughts? Happy to be convinced as to why I might be wrong what angle I am missing.