Attach current location and params to context.router

[gaearon]

Not sure if this is a great way to do it but let’s start here. Partially fixes #3325—at least for my use cases.

I’m not sure how to put route there because it depends on how deep you are. So I just limited it to params and location.

[gaearon]

Not a big fan of duplication but it doesn’t seem harmful yet. Can extract to a method later if necessary.

[taion]

I think we were imagining just creating a new this.router in componentWillUpdate every time, rather than keeping around a persistent one.

[gaearon]

I just tried this but this doesn’t work for the initial render.
setState called inside componentWillMount will not cause componentWillUpdate to run.

[taion]

We can do it in both then? I think part of the reason @ryanflorence wanted to defer this to 3.0 was so that we could implement it in this way, and get away from some of the requirements around keeping track of a stateful router object.

[taion]

We'd handle router.route when we got to per-route context providers for relative links.

[ayrton]

Would adding routes here make sense as well? I've been trying to add tracking to an app with redux & react-router, in some redux middleware I imagine seeing me use this like:

import { LOCATION_CHANGE } from 'react-router-redux';

export default function analytics({ getState }) {
  return (dispatch) => (action) => {
    switch (action.type) {
      case LOCATION_CHANGE:
        track({ name: 'visit', ...visitTrackOptions(action.payload) });
        break;
    }

    return dispatch(action);
  };
}

function visitTrackOptions({ location, routes }) {
  const routePaths = routes.map(route => route.path);
  const pattern = filter(routePaths).join('/').replace('//', '/');

  return {
    weblink_url_full: location.pathname, //=> `/posts/foo-bar`
    weblink_url_pattern: pattern, //=> `/posts/:slug`
  };
}

Is adding routes here something that can be considered? Reason why I ask is because we've been trying to add tracking to an app via redux & react-router

[gaearon]

Can you clarify how this is related to this PR? It only puts some stuff onto this.context.router so it is not related to react-router-redux or its actions.

[ayrton]

I thought this was somehow the leg work to get eg location in redux as well, if it isn't totally my bad 😁 and feel free to ignore. (context)

[taion]

I think it'd make sense to add routes here.

[gaearon]

No, the point I’m trying to make is you don’t need a special integrating library for React Router and Redux to work together. There are a few problems that need to be worked out for them to work fine without any libraries in the middle:

• Fix Link not updating due to connect() optimizations (Make <Link> work with static containers (take two) #3430)
• Make withRouter() more useful in deep hierarchies ([3.0] Put params, location, and route on context.router #3325)
• Ensure it also works with connect() optimizations (Make withRouter() update in response to context changes #3439)

Neither of these are about putting something in the store. Rather, they are about making props from React Router easy to inject into connect()ed components that can read them from ownProps and use them for state selection. I hope this helps!

[gaearon]

Update the PR to recreate the router object. I do it in the listen callback rather than componentWillUpdate so that it works the first time as well.

[taion]

I don't think we can do this in <Router> – we need the same logic for the router object created in match().

[gaearon]

Added the code to do this. It’s possible there is a nicer way to refactor this and split responsibilities between transitionManager and router differently but this can be done after shipping.

[taion]

Cool, thanks. I do think there's a nicer cleanup possible here but we can leave that for later.

[gaearon]

Cleaned it up a tiny bit in fbcbd94 because we only need the router once there.

[taion]

Can we make nextState an arg to createRouterObject? And just do createRouterObject(history, transitionManager, nextState) here and createRouterObject(history, transitionManager, this.state) in <Router>?

[gaearon]

Haha, just made that change. 👍

[gaearon]

@taion

I discovered a problem: changing the context object does not propagate well with the current ContextProvider / ContextSubscriber because this.context.stuff still points to the old one (React doesn’t update it). We need to either fix ContextUtils (more difficult for the general case) or keep updating the same router object (easy).

^^ one of the reasons extracting these hacks into something general is hard. We always know our own cases, but it’s hard to fix all possible edge cases.

[gaearon]

I’ll merge for now to unblock myself as I have little time to finish this. Please feel free to rearrange stuff as you see fit after me.

[taion]

Sure – I think we'll have to go back to modifying the same object in-place, then.

[gaearon]

#3443