From 2c28b9d8bb4b38af09417e42da7de411a5936f86 Mon Sep 17 00:00:00 2001 From: Nikita Khomyakov Date: Thu, 9 Feb 2017 13:43:34 +0000 Subject: [PATCH 1/7] Add React Router v4 support --- package.json | 3 +- src/IndexLinkContainer.js | 2 +- src/LinkContainer.js | 173 ++++++++++++++++---------------- test/IndexLinkContainer.spec.js | 31 +++--- test/LinkContainer.spec.js | 130 ++++++++++++------------ 5 files changed, 171 insertions(+), 168 deletions(-) diff --git a/package.json b/package.json index 9508d41..091d60d 100644 --- a/package.json +++ b/package.json @@ -73,9 +73,10 @@ "lodash": "^4.14.0", "mocha": "^2.5.3", "react": "^15.2.1", + "react-addons-test-utils": "^15.4.2", "react-bootstrap": "^0.30.0", "react-dom": "^15.2.1", - "react-router": "^2.6.0", + "react-router": "4.0.0-beta.5", "release-script": "^1.0.2", "rimraf": "^2.5.4", "shelljs": "^0.7.2", diff --git a/src/IndexLinkContainer.js b/src/IndexLinkContainer.js index b46946f..33e5498 100644 --- a/src/IndexLinkContainer.js +++ b/src/IndexLinkContainer.js @@ -7,7 +7,7 @@ import LinkContainer from './LinkContainer'; export default class IndexLinkContainer extends React.Component { render() { return ( - + ); } } diff --git a/src/LinkContainer.js b/src/LinkContainer.js index 3225df0..c6f296f 100644 --- a/src/LinkContainer.js +++ b/src/LinkContainer.js @@ -1,61 +1,43 @@ -// This is largely taken from react-router/lib/Link. - -import React from 'react'; - -function isLeftClickEvent(event) { - return event.button === 0; -} - -function isModifiedEvent(event) { - return !!( - event.metaKey || - event.altKey || - event.ctrlKey || - event.shiftKey - ); -} - -function createLocationDescriptor(to, query, hash, state) { - if (query || hash || state) { - return { pathname: to, query, hash, state }; - } - - return to; -} - -const propTypes = { - onlyActiveOnIndex: React.PropTypes.bool.isRequired, - to: React.PropTypes.oneOfType([ - React.PropTypes.string, - React.PropTypes.object, - ]).isRequired, - query: React.PropTypes.string, - hash: React.PropTypes.string, - state: React.PropTypes.object, - action: React.PropTypes.oneOf([ - 'push', - 'replace', - ]).isRequired, - onClick: React.PropTypes.func, - active: React.PropTypes.bool, - target: React.PropTypes.string, - children: React.PropTypes.node.isRequired, -}; +import React, { Component, PropTypes } from 'react'; +import { Route } from 'react-router'; + +const isModifiedEvent = (event) => + !!(event.metaKey || event.altKey || event.ctrlKey || event.shiftKey); + +export default class LinkContainer extends Component { + static contextTypes = { + router: PropTypes.shape({ + push: PropTypes.func.isRequired, + replace: PropTypes.func.isRequired, + createHref: PropTypes.func.isRequired, + }).isRequired, + }; -const contextTypes = { - router: React.PropTypes.object, -}; + static propTypes = { + children: PropTypes.element.isRequired, + onClick: PropTypes.func, + target: PropTypes.string, + replace: PropTypes.bool, + to: PropTypes.oneOfType([ + PropTypes.string, + PropTypes.object, + ]).isRequired, + exact: PropTypes.bool, + strict: PropTypes.bool, + className: PropTypes.string, + activeClassName: PropTypes.string, + style: PropTypes.object, + activeStyle: PropTypes.object, + isActive: PropTypes.func, + }; -const defaultProps = { - onlyActiveOnIndex: false, - action: 'push', -}; + static defaultProps = { + replace: false, + activeClassName: 'active', + }; -class LinkContainer extends React.Component { - onClick = (event) => { - const { - to, query, hash, state, children, onClick, target, action, - } = this.props; + handleClick = (event) => { + const { children, onClick } = this.props; if (children.props.onClick) { children.props.onClick(event); @@ -66,42 +48,63 @@ class LinkContainer extends React.Component { } if ( - target || - event.defaultPrevented || - isModifiedEvent(event) || - !isLeftClickEvent(event) + !event.defaultPrevented && // onClick prevented default + event.button === 0 && // ignore right clicks + !this.props.target && // let browser handle "target=_blank" etc. + !isModifiedEvent(event) // ignore clicks with modifier keys ) { - return; - } + event.preventDefault(); - event.preventDefault(); + const { router } = this.context; + const { replace, to } = this.props; - this.context.router[action]( - createLocationDescriptor(to, query, hash, state) - ); - }; + if (replace) { + router.replace(to); + } else { + router.push(to); + } + } + } render() { - const { router } = this.context; - const { onlyActiveOnIndex, to, children, ...props } = this.props; - - props.onClick = this.onClick; - - // Ignore if rendered outside Router context; simplifies unit testing. - if (router) { - props.href = router.createHref(to); + const { + children, + replace, // eslint-disable-line no-unused-vars + to, + exact, + strict, + activeClassName, + className, + activeStyle, + style, + isActive: getIsActive, + ...props, + } = this.props; - if (props.active == null) { - props.active = router.isActive(to, onlyActiveOnIndex); - } - } + const href = this.context.router.createHref( + typeof to === 'string' ? { pathname: to } : to + ); - return React.cloneElement(React.Children.only(children), props); + return ( + { + const isActive = !!(getIsActive ? getIsActive(match, location) : match); + + return React.cloneElement( + React.Children.only(children), + { + ...props, + className: (className || '') + (isActive ? activeClassName : ''), + style: isActive ? { ...style, ...activeStyle } : style, + href, + onClick: this.handleClick, + } + ); + }} + /> + ); } } - -LinkContainer.propTypes = propTypes; -LinkContainer.contextTypes = contextTypes; -LinkContainer.defaultProps = defaultProps; - -export default LinkContainer; diff --git a/test/IndexLinkContainer.spec.js b/test/IndexLinkContainer.spec.js index daacc13..60696a3 100644 --- a/test/IndexLinkContainer.spec.js +++ b/test/IndexLinkContainer.spec.js @@ -1,8 +1,8 @@ import React from 'react'; -import ReactTestUtils from 'react/lib/ReactTestUtils'; +import ReactTestUtils from 'react-addons-test-utils'; import * as ReactBootstrap from 'react-bootstrap'; -import ReactDOM from 'react-dom'; -import { createMemoryHistory, IndexRoute, Route, Router } from 'react-router'; +import { findDOMNode } from 'react-dom'; +import { Route, MemoryRouter as Router } from 'react-router'; import IndexLinkContainer from '../src/IndexLinkContainer'; @@ -19,25 +19,24 @@ describe('IndexLinkContainer', () => { describe('active state', () => { function renderComponent(location) { const router = ReactTestUtils.renderIntoDocument( - - ( - - Root - - )} - > - - - + +
+ ( + + Root + + )} + /> +
); const component = ReactTestUtils.findRenderedComponentWithType( router, Component ); - return ReactDOM.findDOMNode(component); + return findDOMNode(component); } it('should be active on the index route', () => { diff --git a/test/LinkContainer.spec.js b/test/LinkContainer.spec.js index b4c2fd4..fee142d 100644 --- a/test/LinkContainer.spec.js +++ b/test/LinkContainer.spec.js @@ -1,8 +1,8 @@ import React from 'react'; -import ReactTestUtils from 'react/lib/ReactTestUtils'; +import ReactTestUtils from 'react-addons-test-utils'; import * as ReactBootstrap from 'react-bootstrap'; -import ReactDOM from 'react-dom'; -import { createMemoryHistory, Route, Router } from 'react-router'; +import { findDOMNode } from 'react-dom'; +import { Route, MemoryRouter as Router } from 'react-router'; import LinkContainer from '../src/LinkContainer'; @@ -18,14 +18,14 @@ describe('LinkContainer', () => { it('should make the correct href', () => { const router = ReactTestUtils.renderIntoDocument( - + ( + render={() => ( @@ -44,10 +44,10 @@ describe('LinkContainer', () => { it('should not add extra DOM nodes', () => { const router = ReactTestUtils.renderIntoDocument( - + ( + render={() => ( { router, Component ); - expect(ReactDOM.findDOMNode(container)) - .to.equal(ReactDOM.findDOMNode(component)); + expect(findDOMNode(container)) + .to.equal(findDOMNode(component)); }); describe('when clicked', () => { it('should transition to the correct route', () => { const router = ReactTestUtils.renderIntoDocument( - - ( - - Target - - )} - /> -
} - /> + +
+ ( + + Target + + )} + /> +
} + /> +
); @@ -107,19 +109,21 @@ describe('LinkContainer', () => { const childOnClick = sinon.spy(); const router = ReactTestUtils.renderIntoDocument( - - ( - - Foo - - )} - /> -
} - /> + +
+ ( + + Foo + + )} + /> +
} + /> +
); @@ -136,25 +140,22 @@ describe('LinkContainer', () => { describe('active state', () => { function renderComponent(location) { const router = ReactTestUtils.renderIntoDocument( - + ( + render={() => ( Foo )} - > - - - + /> ); const component = ReactTestUtils.findRenderedComponentWithType( router, Component ); - return ReactDOM.findDOMNode(component); + return findDOMNode(component); } it('should be active when on the target route', () => { @@ -167,25 +168,22 @@ describe('LinkContainer', () => { it('should respect explicit active prop on container', () => { const router = ReactTestUtils.renderIntoDocument( - + ( + render={() => ( Bar )} - > - - - + /> ); const component = ReactTestUtils.findRenderedComponentWithType( router, Component ); - expect(ReactDOM.findDOMNode(component).className) + expect(findDOMNode(component).className) .to.match(/\bactive\b/); }); }); @@ -195,19 +193,21 @@ describe('LinkContainer', () => { beforeEach(() => { router = ReactTestUtils.renderIntoDocument( - - ( - - Target - - )} - /> -
} - /> + +
+ ( + + Target + + )} + /> +
} + /> +
); }); @@ -218,7 +218,7 @@ describe('LinkContainer', () => { const component = ReactTestUtils.findRenderedComponentWithType( router, Component ); - ReactTestUtils.Simulate.click(ReactDOM.findDOMNode(component), + ReactTestUtils.Simulate.click(findDOMNode(component), { button: 0 } ); @@ -233,7 +233,7 @@ describe('LinkContainer', () => { const component = ReactTestUtils.findRenderedComponentWithType( router, Component ); - expect(ReactDOM.findDOMNode(component).className) + expect(findDOMNode(component).className) .to.match(/\bdisabled\b/); }); }); From e986ececf03708eb342a2cca10c4a60979a6d8c3 Mon Sep 17 00:00:00 2001 From: Nikita Khomyakov Date: Mon, 20 Mar 2017 10:22:56 +0000 Subject: [PATCH 2/7] Fix by explicitly setting `exact` prop to true --- src/IndexLinkContainer.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/IndexLinkContainer.js b/src/IndexLinkContainer.js index 33e5498..de632a9 100644 --- a/src/IndexLinkContainer.js +++ b/src/IndexLinkContainer.js @@ -7,7 +7,7 @@ import LinkContainer from './LinkContainer'; export default class IndexLinkContainer extends React.Component { render() { return ( - + ); } } From 593b0558e32c325fe82fd136809c9a030b14f4da Mon Sep 17 00:00:00 2001 From: Nikita Khomyakov Date: Mon, 20 Mar 2017 10:24:59 +0000 Subject: [PATCH 3/7] Use stable release of React Router v4 Apparently, it now uses react-router-dom package, which is designed to be used within browser environments --- package.json | 2 +- src/LinkContainer.js | 21 +++++++++++---------- test/IndexLinkContainer.spec.js | 2 +- test/LinkContainer.spec.js | 2 +- 4 files changed, 14 insertions(+), 13 deletions(-) diff --git a/package.json b/package.json index 091d60d..70ae0eb 100644 --- a/package.json +++ b/package.json @@ -76,7 +76,7 @@ "react-addons-test-utils": "^15.4.2", "react-bootstrap": "^0.30.0", "react-dom": "^15.2.1", - "react-router": "4.0.0-beta.5", + "react-router-dom": "^4.0.0", "release-script": "^1.0.2", "rimraf": "^2.5.4", "shelljs": "^0.7.2", diff --git a/src/LinkContainer.js b/src/LinkContainer.js index c6f296f..bded5f3 100644 --- a/src/LinkContainer.js +++ b/src/LinkContainer.js @@ -1,5 +1,5 @@ import React, { Component, PropTypes } from 'react'; -import { Route } from 'react-router'; +import { Route } from 'react-router-dom'; const isModifiedEvent = (event) => !!(event.metaKey || event.altKey || event.ctrlKey || event.shiftKey); @@ -7,9 +7,11 @@ const isModifiedEvent = (event) => export default class LinkContainer extends Component { static contextTypes = { router: PropTypes.shape({ - push: PropTypes.func.isRequired, - replace: PropTypes.func.isRequired, - createHref: PropTypes.func.isRequired, + history: PropTypes.shape({ + push: PropTypes.func.isRequired, + replace: PropTypes.func.isRequired, + createHref: PropTypes.func.isRequired + }).isRequired }).isRequired, }; @@ -50,18 +52,17 @@ export default class LinkContainer extends Component { if ( !event.defaultPrevented && // onClick prevented default event.button === 0 && // ignore right clicks - !this.props.target && // let browser handle "target=_blank" etc. !isModifiedEvent(event) // ignore clicks with modifier keys ) { event.preventDefault(); - const { router } = this.context; + const { history } = this.context.router; const { replace, to } = this.props; if (replace) { - router.replace(to); + history.replace(to); } else { - router.push(to); + history.push(to); } } } @@ -81,7 +82,7 @@ export default class LinkContainer extends Component { ...props, } = this.props; - const href = this.context.router.createHref( + const href = this.context.router.history.createHref( typeof to === 'string' ? { pathname: to } : to ); @@ -97,7 +98,7 @@ export default class LinkContainer extends Component { React.Children.only(children), { ...props, - className: (className || '') + (isActive ? activeClassName : ''), + className: isActive ? [className, activeClassName].join(' ') : className, style: isActive ? { ...style, ...activeStyle } : style, href, onClick: this.handleClick, diff --git a/test/IndexLinkContainer.spec.js b/test/IndexLinkContainer.spec.js index 60696a3..f2e005a 100644 --- a/test/IndexLinkContainer.spec.js +++ b/test/IndexLinkContainer.spec.js @@ -2,7 +2,7 @@ import React from 'react'; import ReactTestUtils from 'react-addons-test-utils'; import * as ReactBootstrap from 'react-bootstrap'; import { findDOMNode } from 'react-dom'; -import { Route, MemoryRouter as Router } from 'react-router'; +import { Route, MemoryRouter as Router } from 'react-router-dom'; import IndexLinkContainer from '../src/IndexLinkContainer'; diff --git a/test/LinkContainer.spec.js b/test/LinkContainer.spec.js index fee142d..4e9ecab 100644 --- a/test/LinkContainer.spec.js +++ b/test/LinkContainer.spec.js @@ -2,7 +2,7 @@ import React from 'react'; import ReactTestUtils from 'react-addons-test-utils'; import * as ReactBootstrap from 'react-bootstrap'; import { findDOMNode } from 'react-dom'; -import { Route, MemoryRouter as Router } from 'react-router'; +import { Route, MemoryRouter as Router } from 'react-router-dom'; import LinkContainer from '../src/LinkContainer'; From 016eb9d9d2314d8bc95ca0a0941be94168acae63 Mon Sep 17 00:00:00 2001 From: Nikita Khomyakov Date: Mon, 20 Mar 2017 10:25:43 +0000 Subject: [PATCH 4/7] Update visual tests to use react-router-dom --- test/visual/ButtonVisual.js | 4 ++-- test/visual/Home.js | 2 +- test/visual/ListGroupItemVisual.js | 2 +- test/visual/MenuItemVisual.js | 2 +- test/visual/NavItemVisual.js | 2 +- test/visual/index.js | 37 +++++++++++------------------- 6 files changed, 19 insertions(+), 30 deletions(-) diff --git a/test/visual/ButtonVisual.js b/test/visual/ButtonVisual.js index da54a81..f4c45eb 100644 --- a/test/visual/ButtonVisual.js +++ b/test/visual/ButtonVisual.js @@ -1,13 +1,13 @@ import React from 'react'; import ButtonToolbar from 'react-bootstrap/lib/ButtonToolbar'; import Button from 'react-bootstrap/lib/Button'; -import { Link } from 'react-router'; +import { Link } from 'react-router-dom'; import LinkContainer from '../../src/LinkContainer'; export default () => (
- Back to Index + Back to Index

Button

Baseline

diff --git a/test/visual/Home.js b/test/visual/Home.js index f1e4e76..00af682 100644 --- a/test/visual/Home.js +++ b/test/visual/Home.js @@ -1,5 +1,5 @@ import React from 'react'; -import { Link } from 'react-router'; +import { Link } from 'react-router-dom'; export default () => (
diff --git a/test/visual/ListGroupItemVisual.js b/test/visual/ListGroupItemVisual.js index 75f2ff1..66d555c 100644 --- a/test/visual/ListGroupItemVisual.js +++ b/test/visual/ListGroupItemVisual.js @@ -1,7 +1,7 @@ import React from 'react'; import ListGroup from 'react-bootstrap/lib/ListGroup'; import ListGroupItem from 'react-bootstrap/lib/ListGroupItem'; -import { Link } from 'react-router'; +import { Link } from 'react-router-dom'; import LinkContainer from '../../src/LinkContainer'; diff --git a/test/visual/MenuItemVisual.js b/test/visual/MenuItemVisual.js index 0359f40..1d9229c 100644 --- a/test/visual/MenuItemVisual.js +++ b/test/visual/MenuItemVisual.js @@ -2,7 +2,7 @@ import React from 'react'; import ButtonToolbar from 'react-bootstrap/lib/ButtonToolbar'; import MenuItem from 'react-bootstrap/lib/MenuItem'; import SplitButton from 'react-bootstrap/lib/SplitButton'; -import { Link } from 'react-router'; +import { Link } from 'react-router-dom'; import LinkContainer from '../../src/LinkContainer'; diff --git a/test/visual/NavItemVisual.js b/test/visual/NavItemVisual.js index 3b957b7..6139762 100644 --- a/test/visual/NavItemVisual.js +++ b/test/visual/NavItemVisual.js @@ -1,7 +1,7 @@ import React from 'react'; import Nav from 'react-bootstrap/lib/Nav'; import NavItem from 'react-bootstrap/lib/NavItem'; -import { Link } from 'react-router'; +import { Link } from 'react-router-dom'; import LinkContainer from '../../src/LinkContainer'; diff --git a/test/visual/index.js b/test/visual/index.js index 9c1a3c1..15b1ddd 100644 --- a/test/visual/index.js +++ b/test/visual/index.js @@ -1,7 +1,7 @@ import React from 'react'; import Grid from 'react-bootstrap/lib/Grid'; import ReactDOM from 'react-dom'; -import { hashHistory, IndexRedirect, Route, Router } from 'react-router'; +import { HashRouter as Router, Route, Redirect } from 'react-router-dom'; import ButtonVisual from './ButtonVisual'; import Home from './Home'; @@ -11,33 +11,22 @@ import NavItemVisual from './NavItemVisual'; import 'bootstrap/less/bootstrap.less'; -const propTypes = { - children: React.PropTypes.node.isRequired, -}; - -const App = ({ children }) => ( - -

React-Router-Bootstrap Module Visual Test

- {children} - -); - -App.propTypes = propTypes; - const mountNode = document.createElement('div'); document.body.appendChild(mountNode); ReactDOM.render( - - - - - - - - - - + + +

React-Router-Bootstrap Module Visual Test

+ + } /> + + + + + + + , mountNode ); From 946e8c665af9493e6133fd3de0ee85c1f3f726b6 Mon Sep 17 00:00:00 2001 From: Nikita Khomyakov Date: Mon, 20 Mar 2017 10:43:24 +0000 Subject: [PATCH 5/7] Fix code style --- src/IndexLinkContainer.js | 2 +- src/LinkContainer.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/IndexLinkContainer.js b/src/IndexLinkContainer.js index de632a9..b2a3f3b 100644 --- a/src/IndexLinkContainer.js +++ b/src/IndexLinkContainer.js @@ -7,7 +7,7 @@ import LinkContainer from './LinkContainer'; export default class IndexLinkContainer extends React.Component { render() { return ( - + // eslint-disable-line react/jsx-boolean-value ); } } diff --git a/src/LinkContainer.js b/src/LinkContainer.js index bded5f3..edce5ad 100644 --- a/src/LinkContainer.js +++ b/src/LinkContainer.js @@ -10,8 +10,8 @@ export default class LinkContainer extends Component { history: PropTypes.shape({ push: PropTypes.func.isRequired, replace: PropTypes.func.isRequired, - createHref: PropTypes.func.isRequired - }).isRequired + createHref: PropTypes.func.isRequired, + }).isRequired, }).isRequired, }; From 94062a09ef91e725e6110ee4cfac8ca6a1cdf0c8 Mon Sep 17 00:00:00 2001 From: Nikita Khomyakov Date: Mon, 20 Mar 2017 18:34:29 +0000 Subject: [PATCH 6/7] Revert change made to prop --- src/IndexLinkContainer.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/IndexLinkContainer.js b/src/IndexLinkContainer.js index b2a3f3b..33e5498 100644 --- a/src/IndexLinkContainer.js +++ b/src/IndexLinkContainer.js @@ -7,7 +7,7 @@ import LinkContainer from './LinkContainer'; export default class IndexLinkContainer extends React.Component { render() { return ( - // eslint-disable-line react/jsx-boolean-value + ); } } From 32f9e442817cf4deaabd4640bb6d0686045ea3e3 Mon Sep 17 00:00:00 2001 From: Nikita Khomyakov Date: Tue, 11 Apr 2017 12:16:45 +0100 Subject: [PATCH 7/7] Set default values for `exact` and `strict` props --- src/LinkContainer.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/LinkContainer.js b/src/LinkContainer.js index edce5ad..1bb7c4c 100644 --- a/src/LinkContainer.js +++ b/src/LinkContainer.js @@ -18,7 +18,6 @@ export default class LinkContainer extends Component { static propTypes = { children: PropTypes.element.isRequired, onClick: PropTypes.func, - target: PropTypes.string, replace: PropTypes.bool, to: PropTypes.oneOfType([ PropTypes.string, @@ -35,6 +34,8 @@ export default class LinkContainer extends Component { static defaultProps = { replace: false, + exact: false, + strict: false, activeClassName: 'active', };