From 81ddb45250e60671e7c9b19c7df9af2b167d507c Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Thu, 5 May 2016 00:15:45 +0100 Subject: [PATCH] Make work with static containers Fixes #470 --- modules/Link.js | 94 +++++++++++++++++++++++++----- modules/RouterUtils.js | 1 + modules/__tests__/Link-test.js | 41 +++++++++++++ modules/createTransitionManager.js | 82 +++++++++++++++++++------- 4 files changed, 183 insertions(+), 35 deletions(-) diff --git a/modules/Link.js b/modules/Link.js index 38b385ccdf..a815fdc715 100644 --- a/modules/Link.js +++ b/modules/Link.js @@ -20,6 +20,21 @@ function isEmptyObject(object) { return true } +function isLinkActive(router, props) { + // Ignore if rendered outside the context of router, simplifies unit testing. + if (!router) { + return false + } + + // Ignore if the render output is unaffected by the active state. + const { to, activeClassName, activeStyle, onlyActiveOnIndex } = props + if (!activeClassName && (activeStyle == null || isEmptyObject(activeStyle))) { + return false + } + + return router.isActive(to, onlyActiveOnIndex) +} + /** * A is used to create an element that links to a route. * When that route is active, the link gets the value of its @@ -62,6 +77,60 @@ const Link = React.createClass({ } }, + getInitialState() { + return { + isActive: isLinkActive(this.context.router, this.props) + } + }, + + componentWillMount() { + this._isActive = this.state.isActive + }, + + componentDidMount() { + const { router } = this.context + if (router) { + this._unlisten = router.listen(() => { + if (this._unlisten) { + this.updateIsActive() + } + }) + } + }, + + componentWillReceiveProps(nextProps) { + const { router } = this.context + if (router) { + if ( + nextProps.to !== this.props.to || + nextProps.onlyActiveOnIndex !== this.props.onlyActiveOnIndex || + nextProps.activeClassName !== this.props.activeClassName || + nextProps.activeStyle !== this.props.activeStyle + ) { + this.updateIsActive(nextProps) + } + } + }, + + componentWillUnmount() { + if (this._unlisten) { + this._unlisten() + this._unlisten = null + } + }, + + updateIsActive(props = this.props) { + const { router } = this.context + const isActive = isLinkActive(router, props) + + // The code is written this way to avoid wasted + // setState() calls that get expensive in large trees. + if (isActive !== this._isActive) { + this._isActive = isActive + this.setState({ isActive: this._isActive }) + } + }, + handleClick(event) { let allowTransition = true @@ -93,26 +162,25 @@ const Link = React.createClass({ }, render() { - const { to, activeClassName, activeStyle, onlyActiveOnIndex, ...props } = this.props - // Ignore if rendered outside the context of router, simplifies unit testing. + const { to, activeClassName, activeStyle, ...props } = this.props const { router } = this.context + const { isActive } = this.state + // Ignore if rendered outside the context of router, simplifies unit testing. if (router) { props.href = router.createHref(to) - if (activeClassName || (activeStyle != null && !isEmptyObject(activeStyle))) { - if (router.isActive(to, onlyActiveOnIndex)) { - if (activeClassName) { - if (props.className) { - props.className += ` ${activeClassName}` - } else { - props.className = activeClassName - } + if (isActive) { + if (activeClassName) { + if (props.className) { + props.className += ` ${activeClassName}` + } else { + props.className = activeClassName } - - if (activeStyle) - props.style = { ...props.style, ...activeStyle } } + + if (activeStyle) + props.style = { ...props.style, ...activeStyle } } } diff --git a/modules/RouterUtils.js b/modules/RouterUtils.js index 246217574d..39f618954a 100644 --- a/modules/RouterUtils.js +++ b/modules/RouterUtils.js @@ -1,6 +1,7 @@ export function createRouterObject(history, transitionManager) { return { ...history, + listen: transitionManager.listen, setRouteLeaveHook: transitionManager.listenBeforeLeavingRoute, isActive: transitionManager.isActive } diff --git a/modules/__tests__/Link-test.js b/modules/__tests__/Link-test.js index 39e78c1b28..249fd573f0 100644 --- a/modules/__tests__/Link-test.js +++ b/modules/__tests__/Link-test.js @@ -314,6 +314,47 @@ describe('A ', function () { ), node, execNextStep) }) + + it('changes active state inside static components', function (done) { + class LinkWrapper extends Component { + shouldComponentUpdate() { + return false + } + + render() { + return ( +
+ Link + {this.props.children} +
+ ) + } + } + + let a + const history = createHistory('/goodbye') + const steps = [ + function () { + a = node.querySelector('a') + expect(a.className).toEqual('') + history.push('/hello') + }, + function () { + expect(a.className).toEqual('active') + } + ] + + const execNextStep = execSteps(steps, done) + + render(( + + + + + + + ), node, execNextStep) + }) }) describe('when clicked', function () { diff --git a/modules/createTransitionManager.js b/modules/createTransitionManager.js index b81c891686..4e188811e6 100644 --- a/modules/createTransitionManager.js +++ b/modules/createTransitionManager.js @@ -209,35 +209,73 @@ export default function createTransitionManager(history, routes) { } } + let ownListeners = [] + let unlistenHistory + let lastHistoryLocation + + function handleHistoryLocationChange(location, callback) { + if (state.location === location) { + callback(null, state) + } else { + match(location, function (error, redirectLocation, nextState) { + if (error) { + callback(error) + } else if (redirectLocation) { + history.transitionTo(redirectLocation) + } else if (nextState) { + callback(null, nextState) + } else { + warning( + false, + 'Location "%s" did not match any routes', + location.pathname + location.search + location.hash + ) + } + }) + } + } + + function notifyOwnListeners(error, nextState) { + ownListeners.forEach(listener => listener(error, nextState)) + } + /** * This is the API for stateful environments. As the location * changes, we update state and call the listener. We can also * gracefully handle errors and redirects. */ function listen(listener) { - // TODO: Only use a single history listener. Otherwise we'll - // end up with multiple concurrent calls to match. - return history.listen(function (location) { - if (state.location === location) { - listener(null, state) - } else { - match(location, function (error, redirectLocation, nextState) { - if (error) { - listener(error) - } else if (redirectLocation) { - history.transitionTo(redirectLocation) - } else if (nextState) { - listener(null, nextState) - } else { - warning( - false, - 'Location "%s" did not match any routes', - location.pathname + location.search + location.hash - ) - } - }) + if (!unlistenHistory) { + // Set up a shared subscription to history + // when the first own listener subscribes. + unlistenHistory = history.listen(location => { + lastHistoryLocation = location + handleHistoryLocationChange(location, notifyOwnListeners) + }) + } + + ownListeners.push(listener) + + // Since history.listen() only happens once, + // we manually call the new listener synchronously. + handleHistoryLocationChange(lastHistoryLocation, listener) + + return () => { + const index = ownListeners.indexOf(listener) + if (index === -1) { + // This listener has been unsubscribed before + return } - }) + + ownListeners.splice(index, 1) + + // Tear down the shared subscription to history + // when the last own listener unsubscribes. + if (!ownListeners.length) { + unlistenHistory() + unlistenHistory = null + } + } } return {