preact-router icon indicating copy to clipboard operation
preact-router copied to clipboard

Add previous/current route instances to RouterOnChangeArgs

Open peabnuts123 opened this issue 3 years ago • 2 comments

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.

peabnuts123 avatar Oct 15 '20 05:10 peabnuts123

Unrelatedly, I was wondering if y'all wanted to chuck the Hacktoberfest topic on this repo? This would make this PR count towards my "score" on there, and might bring in some people to make some PRs here. But I want to make this change regardless 😄

peabnuts123 avatar Oct 15 '20 05:10 peabnuts123

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

peabnuts123 avatar Dec 13 '20 05:12 peabnuts123