Add previous/current route instances to RouterOnChangeArgs

[peabnuts123]

The nature of this change is to address an issue with RouterOnChangeArgs: the previous value is just a URL string, while current is a component instance. Not only is it a bit uncomfortable that these are different types (while representing the same concept), it's less useful that previous is just a string, when the previous route instance can be easily stored in the same manner.

As far as I know this is a simple, low impact change. I've added another state property (alongside previousUrl) called previousRoute - this is the instance of the previous route i.e. the last value of current. As far as I can tell, previousUrl should map to previousRoute 1:1 and so storing them both in the same manner should be correct behaviour.

In order to not break backwards compatibility with existing applications, I have not replaced previous with this change, but rather, added 2 new properties (which would ideally be referenced instead, and maybe one day previous and current can be removed?) called previousRoute and currentRoute. You'll notice currentRoute is exactly the same as current - this is to offer a migration path in the future, and to help the avoid the API feeling "clunky" (a-la Date.getFullYear()...).

Like I said I've deliberately kept this change as low-impact as possible. It could be done in better but more impactful ways, I can make these changes if you wish, just let me know.

Lastly, for some further context, I need this change in my app in order to detect when the user moves to/from a default route (i.e. I am comparing props.default in both previousRoute and currentRoute) - it's not possible to detect this without this change. This would also allow developers to do things like extract prop values from the route as you are leaving it.

[rschristian]

Can you elaborate on how you'd use this? Just curious.

[peabnuts123]

This is already what current is. This PR just brings current/previous in parity with one-another. Currently previous is a string, while current is a Route instance. Not only is this inconsistent, it makes it hard / impossible to implement logic that uses props from the previous route.

I am depending on these changes in a project of mine, where I use this data to determine whether the user is navigating to/from the default route: use-case.

[rschristian]

Ignoring the oddness of calling a browser refresh, why watch the props rather than the route?

Watching component props like this to me sounds like an anti-pattern. I'm not saying that this shouldn't get merged or anything but that there's almost certainly a better way to do what you want.

[peabnuts123]

I'd love to find a work around. I need to know information about the previous route, but I cannot resolve this from the URL. I could manually build up a dictionary of this data myself, keyed by URL, and try and do this lookup myself (including mapping URLs to dictionary keys i.e. trimming query params, etc.), but really it is the responsibility of the Router to give me this information (especially, in my use-case above, whether a Route is the default route or not).

[peabnuts123]

Thanks for taking the time to review @rschristian, I thought this would never be looked at