Transition hooks not called for redirects

[davidminor]

I notice that if a route's transition hooks return a redirect state, the hooks for the redirect state's route aren't called. Is that intentional?

[dkolarev]

Have the same issue with this since and caused some serious problems since my core logic requires that hooks are called. Problem is with this line where a new state is changed but transition hooks don't get called (the same is for all hooks). I'm using the latest 6.0.0-beta.1 for Mobx 6. Can you confirm that is a bug or by design so we could plan what needs to be changed.

[nareshbhatia]

@davidminor, @dkolarev, confirming that I have seen your messages. I will try to investigate and get you an answer by the weekend.

[nareshbhatia]

Hi @davidminor and @dkolarev, I took a quick look at the related code and concluded that the hooks were meant to be a simple mechanism to solve the following use cases:

1. Check if it is ok to enter the state requested by the user. This is the beforeEnter hook. See Redirecting to the Sign In page.
2. Do something just before entering a state. This is the onEnter hook. See Fetching Data

The beforeExit and onExit hooks were thrown in for completeness.

Implementing your request would mean introducing recursion into the transition algorithm, essentially the transition function would become recursive. Also, it is not clear which state would be specified as the from state.

Do you have a compelling use case that would justify adding this kind of complication? Can your use case be simplified to achieve the desired results using the existing implementation?

[dkolarev]

Hi @nareshbhatia, first thank you for your time. I am aware of the recursion issue but that should be on the developer since his actions can cause stack overflow. There is a lot of existing use cases in various frameworks, where badly written code could lead to undesired behavior. My general issues are data fetching and authorization processing that fails in specific cases. As you said, hooks are a mechanism to check if it's ok to enter the requested route. The problem is if they are not called every time, they are not much of a benefit.

[davidminor]

Hi @nareshbhatia -

Thank you for looking into this.

In our case we're using the 2nd use case fairly extensively, either fetching the data or kicking off the fetching of data when entering a route (seems like with react moving to suspense this becomes a very common one). When we tried adding a redirect to one of our routes we found that the data wasn't being fetched due to onEnter not being called for that route.

One possible workaround is to have the redirecting route fetch the data for the route it is redirecting to. This seems not ideal to have the routes know how each other works. This also might fail if the 2nd had some case where it was redirecting as well.

Another possible workaround would be to have the redirecting route call the hooks of the route it is redirecting to, but then it feels like the routes are doing the work of the router.

I agree adding recursion complicates the router implementation which is currently very clean. I imagine you'd want to consider a maxRedirects option to keep devs from shooting themselves in the foot.

I'm not sure what fromState should be. In our app we're only using fromState to prevent the user from leaving a page if they have unsaved changes, so it would be unaffected by this. Possibly the API could allow the user to choose it if necessary? Or perhaps the API could expose the original route and the redirecting route?

[nareshbhatia]

Ok, I can see your use case now. Perhaps we can try the recursion. That seems to be the cleanest approach to avoid repeated code. I would also suggest keeping the fromState the same as the original fromState - the reason being, the router never went into the redirectState. Do you want to submit a PR? We will need to make sure that tests don't break + additional tests for the new feature.

[davidminor]

@nareshbhatia if fromState remains the same, what are your thoughts on whether beforeExit and onExit should be called once, or each time?

E.g. if we transition from state A to state B and A.onExit redirects to state C, should we again call the beforeExit and onExit hooks for A when transitioning to C? Or only call C's beforeEnter and onEnter hooks?

[dkolarev]

@nareshbhatia if fromState remains the same, what are your thoughts on whether beforeExit and onExit should be called once, or each time?

E.g. if we transition from state A to state B and A.onExit redirects to state C, should we again call the beforeExit and onExit hooks for A when transitioning to C? Or only call C's beforeEnter and onEnter hooks?

@davidminor Calling exit hooks of the same fromState will cause infinite loops. You can check my implementation #119 where I'm doing it only once, which is somehow natural.

[nareshbhatia]

Thanks @dkolarev. I will take a look at your PR over the weekend.

[davidminor]

@nareshbhatia if fromState remains the same, what are your thoughts on whether beforeExit and onExit should be called once, or each time? E.g. if we transition from state A to state B and A.onExit redirects to state C, should we again call the beforeExit and onExit hooks for A when transitioning to C? Or only call C's beforeEnter and onEnter hooks?

@davidminor Calling exit hooks of the same fromState will cause infinite loops. You can check my implementation #119 where I'm doing it only once, which is somehow natural.

I think this could be avoided by checking that the redirectState and toState are not equal.

I agree though, it seems more logical to have beforeExit and onExit only called a single time.

[dkolarev]

Thanks @dkolarev. I will take a look at your PR over the weekend.

@nareshbhatia Hi, did you maybe have time to check the pull request related to this issue?

[nareshbhatia]

Implemented via https://github.com/nareshbhatia/mobx-state-router/pull/119