Link: listen to history for active state changes

[malte-wessel]

• subscribe to history changes
• don't use the context to pass data
• works with parents that implement shouldComponentUpdate
• solves ActiveState changes don't propagate if any single parent in hierarchy declares shouldComponentUpdate #470, Active link is not updating #1772

[gaearon]

4 spaces, should be 2

[gaearon]

You are my hero ❤️

@mjackson @ryanflorence Can we please get your eyes on this? I really really want this to get into 1.0.

[mjackson]

niiiiiiiice ;) I was hoping someone would take a stab at this. Thanks @malte-wessel ! I'll take a closer look tomorrow morning.

[malte-wessel]

@gaearon spaces are fixed, shouldComponentUpdate is in 👍

[mjackson]

Looks solid to me, @malte-wessel! Thanks for your thoughtful contribution here. It really takes some time to wrap your head around this codebase, and I really appreciate you taking the time to do it :)

[malte-wessel]

thank you guys for your awesome work!

[roundrobin]

Awesome thanks so much, now I can get rid of an entire store!

[mjackson]

Hmm ... turns out this approach causes another subtle bug. :/

In our tests, we now cannot assert active state on links. Here's what the flow looks like:

1. we get a new location
2. <Router> picks up the change and updates its state, causing a re-render
3. we assert that links have the correct active state (and fail)
4. <Link>s get the location change and update their active state

So, while I really like the fact that <Link>s now work as descendants of components that return false from shouldComponentUpdate, I'm afraid we're going to have to think of another solution for this particular problem. We have reverted these changes in 50372fe.

[gaearon]

@mjackson Can you expand a bit? I don't fully understand what's going on (particularly 2 and 3). Any way I can help?

[tajo]

Uh, I still don't now how to solve this annoying issue (working active Link inside a subtree with shouldComponentUpdate doing shallowCompare). I don't want to use any workarounds (like fiddling with this.context inside of shouldComponentUpdate or having my own UberLink component).

What about at least trying to get the history object from this.props.history if this.context.history does not exist? It's should be one-liner. Then I could just pass the history object as a prop. It is still kinda workaround but clean.

https://github.com/rackt/react-router/blob/master/modules/Link.js#L101

[taion]

You can follow along in the discussion on #470. For my money, though, I think you just shouldn't use shallowCompare on those subtrees. If you embed a Link, the parents of the Link are not really "pure".