Remove context.location

[mjackson]

Right now v4 uses context.location to pass the current location down to <Link>s. However, context isn't great for communicating state updates because shouldComponentUpdate does not take context into account. This means that people who are using sCU (i.e. everyone using Redux) won't get these changes.

Instead, the router should rely on a listener/subscription mechanism. We can introduce this pretty easily using something like:

class Link extends React.Component {
  static contextTypes = {
    router: PropTypes.object
  }

  state = {
    location: null
  }

  componentWillMount() {
    this.context.router.addChangeListener(location => this.setState({ location }))
  }

  // ...
}

Note that addChangeListener is not the same as history.listen because it needs to fire once immediately.

[avocadowastaken]

Some helper like 'withLocation' will be great for start

[timdorr]

3.0 will get around this by using this neat mixin: https://github.com/ReactTraining/react-router/blob/v3/modules/ContextUtils.js
It's added here: https://github.com/ReactTraining/react-router/blob/v3/modules/withRouter.js#L15

You can probably do that without a mixin format. I know @taion was going to split that code out to a separate package, since it's something that affects a lot of context-using libraries. Not sure what the status of that effort is, but might want to check in with him to see if he's still planning on it.

[pshrmn]

Will this subscription apply to every Link or will it be conditional? From what I can tell, this is only being done to fix Link components that use activeClassName or activeStyle and get stuck behind a shouldComponentUpdate. However, removing context.location will also force Link components that use either of the active props but aren't stuck behind sCU to subscribe to location changes because they no longer have the context.location to refer to. Link components without either of those props wouldn't need to be subscribed since they don't care about the current location.

I think that it might make sense to preserve context.location and instead add a boolean attribute to the Link for which addChangeListener is only called when it is true. The downside of this is that it forces the user to explicitly state that the Link might not update when the location changes. However, this will only be necessary when their Link components aren't updating because of some other library (or an sCU of their own doing), so I don't think that it is an unreasonable expectation.

componentWillMount() {
  if ( this.props.listen ) {
    // don't need state anymore
    this.context.router.addChangeListener(() => { this.forceUpdate(); });
  }
}

<Link listen to='/faq' activeClassName='active'>FAQ</Link>

The Match component could also benefit from something like this since it also gets stuck behind sCU calls.

[timdorr]

@pshrmn We don't want to expose internal concerns to the end user. Adding a listener isn't a big deal, as React will batch together any concurrent setState's triggered by the listener. We do a similar thing in react-redux, so it's a good pattern.

[mjackson]

You're right though, @pshrmn. The only <Link>s that need to subscribe are those that need to know about "active" or not. This should actually help out a lot with perf.

[timdorr]

Done in ce59676

[jochenberger]

Are there any plans to export the LocationSubscriber component from index.js?

[sheepsteak]

@jochenberger you can do this at the moment:

import { LocationSubscriber } from 'react-router/locationBroadcast';

But I agree, it would be good to have this exported from index.js.

[jochenberger]

I'm in AMD environment, so unfortunately, I'm limited to what's exported there.