useRoutes changes the meaning of history.listen

[timdorr]

In history, the listen method is for listening to when the location changes. Pretty simple.

However, in the router, useRoutes changes this definition to whatever it interprets the history state change as. Even worse, it changes the callback signature from history.

There's nothing wrong with that. We should have some sort of event listener for when the router is done doing its routing. However, it makes the API tough to reason around, because if you create a history, attach a listener, and then get it back out of context/renderProps, it's now changed the meaning of the function you've just used. Yikes!

What would likely make more sense is a separate method for listening to the route changes and leaving history.listen unaltered. @taion proposed routerHistory.listenRoutes in Discord, so that might be a good name for this.

[timdorr]

By the way, the reason this came up is for behaviors and libraries that want to perform some action before the state change. A useRoutes'ed history still has listenBefore on it, but the signature is different from the enhanced listen.

So, consistency here would be good for the API health and would avoid some confusion for anyone wanting to use listen

[taion]

My vote would be to rename this method to listenRoutes and add a deprecation warning to listen (to be removed in v2 of course).

[mjackson]

Now that 1.0 has shipped is like to take another look at this API. I'm not
fond of the idea of using a listenRoutes method because that just
introduces more API we have to maintain. Instead, I think the existing
listen method could possibly be changed to conform with other use*
functions.
On Tue, Nov 10, 2015 at 10:22 AM Jimmy Jia notifications@github.com wrote:

My vote would be to rename this method to listenRoutes and add a
deprecation warning to listen (to be removed in v2 of course).

—
Reply to this email directly or view it on GitHub
#2502 (comment)
.

[taion]

I don't know if that makes sense. The routing history doesn't just invoke its callback with a different signature. It invokes its callback at a different time - i.e. when all of the async routing stuff is resolved. Even if we merged the routing information onto the location object, I don't see how we could reconcile the question of the different callback invocation time.

[taion]

Questions of state aside, the difference is that one is a synchronous operation, while the other is an async operation that has some concept of possible failure. They're quite different.

[taion]

One more benefit to using listenRoutes instead of listen: this would allow <Router> to figure out whether the history already has routes set up, which would allow users to apply history enhancers after useRoutes. Currently this is impossible without replacing <Router> entirely.

[taion]

cc @jlongster @goatslacker

One implication is that right now, you can't actually hook into the routing state from outside of the <Router> itself. Something like alt-router or redux-simple-router as written do not have access to listen on the useRoutes-enhanced history, since <Router> creates that history, so you have no way to hook into the actual routing state changing (which happens after the history's location changes - and potentially asynchronously after when async routing is in play).

[mariocasciaro]

@taion Agreed, I'm currently using redux-simple-router and I had to patch react-router (renaming listen to something else) to get everything working. This change is really needed IMO.

[mjackson]

I think we're now having the same discussion in #2614. We're all converging around the same idea, anyway. Let's discuss there.