Links apathetic to active don't subscribe (#3985) (#3986)

* Links apathetic to active don't subscribe

* add tests to check if Link subscribes

Author: pshrmn

modules/Link.js

@@ -51,23 +51,41 @@ class Link extends React.Component {
   }
 
   render() {
+    const { router } = this.context
+    const {
+      to,
+      style,
+      activeStyle,
+      className,
+      activeClassName,
+      location: propLocation,
+      isActive: getIsActive,
+      activeOnlyWhenExact, // eslint-disable-line
+      replace, // eslint-disable-line
+      ...rest
+    } = this.props
+
+    // If there are no props that require location information
+    // the LocationSubscriber is unnecessary.
+    if (
+      activeClassName === '' &&
+      Object.keys(activeStyle).length === 0 &&
+      typeof rest.children !== 'function'
+    ) {
+      return (
+        <a
+          {...rest}
+          href={router ? router.createHref(to) : to}
+          onClick={this.handleClick}
+          style={style}
+          className={className}
+        />
+      )
+    }
+
     return (
       <LocationSubscriber>
         {(contextLocation) => {
-          const { router } = this.context
-          const {
-            to,
-            style,
-            activeStyle,
-            className,
-            activeClassName,
-            location: propLocation,
-            isActive: getIsActive,
-            activeOnlyWhenExact, // eslint-disable-line
-            replace, // eslint-disable-line
-            ...rest
-          } = this.props
-
           const currentLocation = propLocation || contextLocation
 
           const isActive = getIsActive(

modules/__tests__/Link-test.js

@@ -1,8 +1,9 @@
-import expect from 'expect'
+import expect, { spyOn, restoreSpies } from 'expect'
 import React, { PropTypes } from 'react'
 import Link from '../Link'
 import { render } from 'react-dom'
 import { LocationBroadcast } from '../Broadcasts'
+import * as broadcasters from '../Broadcasts'
 
 describe('Link', () => {
 
@@ -287,4 +288,59 @@ describe('Link', () => {
       expect(a.className).toEqual('active')
     })
   })
+
+  describe('knowledge of location', () => {
+
+    beforeEach(() => {
+      spyOn(broadcasters, 'LocationSubscriber').andCallThrough()
+    })
+
+    afterEach(() => {
+      restoreSpies()
+    })
+
+    it('renders <LocationSubscriber> when it has an activeClassName', () => {
+      const div = document.createElement('div')
+      render((
+          <Link
+            to='/foo'
+            activeClassName='active'
+            location={{ pathname: '/bar' }}
+          />
+      ), div)
+      expect(broadcasters.LocationSubscriber).toHaveBeenCalled()
+    })
+
+    it('renders <LocationSubscriber> when it has an activeStyle', () => {
+      const div = document.createElement('div')
+      render((
+        <Link
+          to='/foo'
+          activeStyle={{ color: 'red' }}
+          location={{ pathname: '/bar' }}
+        />
+      ), div)
+      expect(broadcasters.LocationSubscriber).toHaveBeenCalled()
+    })
+
+    it('renders <LocationSubscriber> when props.children is a function', () => {
+      const div = document.createElement('div')
+      render((
+        <Link to='/foo' location={{ pathname: '/bar' }}>
+          {
+            ({isActive}) => <a className={isActive ? 'active' : ''}>Test!</a>
+          }
+        </Link>
+      ), div)
+      expect(broadcasters.LocationSubscriber).toHaveBeenCalled()
+    })
+
+    it('does not render <LocationSubscriber> when it has no active props', () => {
+      const div = document.createElement('div')
+      render((
+        <Link to='/foo'>Foo</Link>
+      ), div)
+      expect(broadcasters.LocationSubscriber).toNotHaveBeenCalled()
+    })
+  })
 })