From 048be894928051047f526cf103f20f697384976f Mon Sep 17 00:00:00 2001 From: Thomas Florin Date: Wed, 17 Aug 2016 23:54:00 -0500 Subject: [PATCH 1/8] withRouter now supports withRef as an option, to better support getting the wrapped instance --- modules/__tests__/withRouter-test.js | 25 ++++++++++++++-- modules/withRouter.js | 43 +++++++++++++++++++--------- 2 files changed, 52 insertions(+), 16 deletions(-) diff --git a/modules/__tests__/withRouter-test.js b/modules/__tests__/withRouter-test.js index c083bd74bf..42c3e2718d 100644 --- a/modules/__tests__/withRouter-test.js +++ b/modules/__tests__/withRouter-test.js @@ -6,12 +6,14 @@ import Route from '../Route' import Router from '../Router' import routerShape from '../PropTypes' import withRouter from '../withRouter' - describe('withRouter', function () { class App extends Component { propTypes: { router: routerShape.isRequired } + testFunction() { + return 'hello from the test function' + } render() { expect(this.props.router).toExist() return

App

@@ -28,7 +30,8 @@ describe('withRouter', function () { }) it('puts router on context', function (done) { - const WrappedApp = withRouter(App) + + const WrappedApp = withRouter()(App) render(( @@ -40,7 +43,7 @@ describe('withRouter', function () { }) it('still uses router prop if provided', function (done) { - const Test = withRouter(function (props) { + const Test = withRouter()(function (props) { props.test(props) return null }) @@ -59,4 +62,20 @@ describe('withRouter', function () { render(, node, done) }) + + it('should support withRefs as a parameter',function (done) { + const WrappedApp = withRouter({ withRef:true })(App) + const router = { + push() {}, + replace() {}, + go() {}, + goBack() {}, + goForward() {}, + setRouteLeaveHook() {}, + isActive() {} + } + const component = render((), node, done) + expect(component.getWrappedInstance().testFunction()).toEqual('hello from the test function') + }) + }) diff --git a/modules/withRouter.js b/modules/withRouter.js index 396c5ad6c9..ff6f796338 100644 --- a/modules/withRouter.js +++ b/modules/withRouter.js @@ -1,23 +1,40 @@ import React from 'react' import hoistStatics from 'hoist-non-react-statics' import { routerShape } from './PropTypes' +import warning from './routerWarning' function getDisplayName(WrappedComponent) { return WrappedComponent.displayName || WrappedComponent.name || 'Component' } +export default function withRouter(options) { + const wrapWithRouter = function (WrappedComponent) { + const WithRouter = React.createClass({ + contextTypes: { router: routerShape }, + propTypes: { router: routerShape }, + getWrappedInstance() { + warning(options && options.withRef, 'To access the wrappedInstance you must provide {withRef : true} as the first argument of the withRouter call') + return this.refs.wrappedInstance + }, + render() { + const router = this.props.router || this.context.router + if(options && options.withRef) { + return + }else{ + return + } + } + }) + WithRouter.displayName = `withRouter(${getDisplayName(WrappedComponent)})` + WithRouter.WrappedComponent = WrappedComponent -export default function withRouter(WrappedComponent) { - const WithRouter = React.createClass({ - contextTypes: { router: routerShape }, - propTypes: { router: routerShape }, - render() { - const router = this.props.router || this.context.router - return - } - }) + return hoistStatics(WithRouter, WrappedComponent) + } - WithRouter.displayName = `withRouter(${getDisplayName(WrappedComponent)})` - WithRouter.WrappedComponent = WrappedComponent - - return hoistStatics(WithRouter, WrappedComponent) + if(typeof(options) === 'function') { + warning(false,'passing a component to the first invocation of withRouter has been depreciated, withRouter'+ + 'now needs to be invoked twice, once to pass the options through, and a second time with the component eg. withRouter()(MyComponent) or withRouter({withRef:true})(MyComponent)') + return wrapWithRouter(options) + }else{ + return wrapWithRouter + } } From 6d861e79ef0eebd5b5a06365d3c6ab00d020ea56 Mon Sep 17 00:00:00 2001 From: Thomas Florin Date: Wed, 17 Aug 2016 23:57:10 -0500 Subject: [PATCH 2/8] Fixing some spacing issues --- modules/__tests__/withRouter-test.js | 1 + 1 file changed, 1 insertion(+) diff --git a/modules/__tests__/withRouter-test.js b/modules/__tests__/withRouter-test.js index 42c3e2718d..112cdd0ddd 100644 --- a/modules/__tests__/withRouter-test.js +++ b/modules/__tests__/withRouter-test.js @@ -6,6 +6,7 @@ import Route from '../Route' import Router from '../Router' import routerShape from '../PropTypes' import withRouter from '../withRouter' + describe('withRouter', function () { class App extends Component { propTypes: { From 4bdccb07458e93e0b153bf2fde49f75cd486c95a Mon Sep 17 00:00:00 2001 From: Thomas Florin Date: Wed, 17 Aug 2016 23:58:57 -0500 Subject: [PATCH 3/8] Last minute spacing issues --- modules/__tests__/withRouter-test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/modules/__tests__/withRouter-test.js b/modules/__tests__/withRouter-test.js index 112cdd0ddd..bfc5142dbc 100644 --- a/modules/__tests__/withRouter-test.js +++ b/modules/__tests__/withRouter-test.js @@ -31,7 +31,6 @@ describe('withRouter', function () { }) it('puts router on context', function (done) { - const WrappedApp = withRouter()(App) render(( From 9ec7563f04ead9cf13fc924de5e7d6107296a39b Mon Sep 17 00:00:00 2001 From: Thomas Florin Date: Thu, 18 Aug 2016 11:15:25 -0500 Subject: [PATCH 4/8] Reverting change to withRouter --- modules/__tests__/withRouter-test.js | 23 ++------------- modules/withRouter.js | 43 +++++++++------------------- 2 files changed, 15 insertions(+), 51 deletions(-) diff --git a/modules/__tests__/withRouter-test.js b/modules/__tests__/withRouter-test.js index bfc5142dbc..c083bd74bf 100644 --- a/modules/__tests__/withRouter-test.js +++ b/modules/__tests__/withRouter-test.js @@ -12,9 +12,6 @@ describe('withRouter', function () { propTypes: { router: routerShape.isRequired } - testFunction() { - return 'hello from the test function' - } render() { expect(this.props.router).toExist() return

App

@@ -31,7 +28,7 @@ describe('withRouter', function () { }) it('puts router on context', function (done) { - const WrappedApp = withRouter()(App) + const WrappedApp = withRouter(App) render(( @@ -43,7 +40,7 @@ describe('withRouter', function () { }) it('still uses router prop if provided', function (done) { - const Test = withRouter()(function (props) { + const Test = withRouter(function (props) { props.test(props) return null }) @@ -62,20 +59,4 @@ describe('withRouter', function () { render(, node, done) }) - - it('should support withRefs as a parameter',function (done) { - const WrappedApp = withRouter({ withRef:true })(App) - const router = { - push() {}, - replace() {}, - go() {}, - goBack() {}, - goForward() {}, - setRouteLeaveHook() {}, - isActive() {} - } - const component = render((), node, done) - expect(component.getWrappedInstance().testFunction()).toEqual('hello from the test function') - }) - }) diff --git a/modules/withRouter.js b/modules/withRouter.js index ff6f796338..396c5ad6c9 100644 --- a/modules/withRouter.js +++ b/modules/withRouter.js @@ -1,40 +1,23 @@ import React from 'react' import hoistStatics from 'hoist-non-react-statics' import { routerShape } from './PropTypes' -import warning from './routerWarning' function getDisplayName(WrappedComponent) { return WrappedComponent.displayName || WrappedComponent.name || 'Component' } -export default function withRouter(options) { - const wrapWithRouter = function (WrappedComponent) { - const WithRouter = React.createClass({ - contextTypes: { router: routerShape }, - propTypes: { router: routerShape }, - getWrappedInstance() { - warning(options && options.withRef, 'To access the wrappedInstance you must provide {withRef : true} as the first argument of the withRouter call') - return this.refs.wrappedInstance - }, - render() { - const router = this.props.router || this.context.router - if(options && options.withRef) { - return - }else{ - return - } - } - }) - WithRouter.displayName = `withRouter(${getDisplayName(WrappedComponent)})` - WithRouter.WrappedComponent = WrappedComponent - return hoistStatics(WithRouter, WrappedComponent) - } +export default function withRouter(WrappedComponent) { + const WithRouter = React.createClass({ + contextTypes: { router: routerShape }, + propTypes: { router: routerShape }, + render() { + const router = this.props.router || this.context.router + return + } + }) - if(typeof(options) === 'function') { - warning(false,'passing a component to the first invocation of withRouter has been depreciated, withRouter'+ - 'now needs to be invoked twice, once to pass the options through, and a second time with the component eg. withRouter()(MyComponent) or withRouter({withRef:true})(MyComponent)') - return wrapWithRouter(options) - }else{ - return wrapWithRouter - } + WithRouter.displayName = `withRouter(${getDisplayName(WrappedComponent)})` + WithRouter.WrappedComponent = WrappedComponent + + return hoistStatics(WithRouter, WrappedComponent) } From ae24bcffce8ac54e4dd5e1e3c1240012c9eb720b Mon Sep 17 00:00:00 2001 From: Thomas Florin Date: Thu, 18 Aug 2016 11:22:09 -0500 Subject: [PATCH 5/8] Updating withRouter based on comments --- modules/__tests__/withRouter-test.js | 18 ++++++++++++++++++ modules/withRouter.js | 14 ++++++++++++-- 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/modules/__tests__/withRouter-test.js b/modules/__tests__/withRouter-test.js index c083bd74bf..cc146c4123 100644 --- a/modules/__tests__/withRouter-test.js +++ b/modules/__tests__/withRouter-test.js @@ -12,6 +12,9 @@ describe('withRouter', function () { propTypes: { router: routerShape.isRequired } + testFunction() { + return 'hello from the test function' + } render() { expect(this.props.router).toExist() return

App

@@ -59,4 +62,19 @@ describe('withRouter', function () { render(, node, done) }) + + it('should support withRefs as a parameter',function (done) { + const WrappedApp = withRouter(App,{ withRef:true }) + const router = { + push() {}, + replace() {}, + go() {}, + goBack() {}, + goForward() {}, + setRouteLeaveHook() {}, + isActive() {} + } + const component = render((), node, done) + expect(component.getWrappedInstance().testFunction()).toEqual('hello from the test function') + }) }) diff --git a/modules/withRouter.js b/modules/withRouter.js index 396c5ad6c9..474ecfd18d 100644 --- a/modules/withRouter.js +++ b/modules/withRouter.js @@ -1,18 +1,28 @@ import React from 'react' import hoistStatics from 'hoist-non-react-statics' import { routerShape } from './PropTypes' +import warning from './routerWarning' + function getDisplayName(WrappedComponent) { return WrappedComponent.displayName || WrappedComponent.name || 'Component' } -export default function withRouter(WrappedComponent) { +export default function withRouter(WrappedComponent, options) { const WithRouter = React.createClass({ contextTypes: { router: routerShape }, propTypes: { router: routerShape }, + getWrappedInstance() { + warning(options && options.withRef, 'To access the wrappedInstance you must provide {withRef : true} as the first argument of the withRouter call') + return this.refs.wrappedInstance + }, render() { const router = this.props.router || this.context.router - return + if(options && options.withRef) { + return + }else{ + return + } } }) From 017eb4b899b8bf8e3395cf4b1f14d52c4b8bd370 Mon Sep 17 00:00:00 2001 From: Thomas Florin Date: Thu, 18 Aug 2016 11:29:25 -0500 Subject: [PATCH 6/8] updating error message from not passing in proper options object --- modules/withRouter.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/withRouter.js b/modules/withRouter.js index 474ecfd18d..f357290128 100644 --- a/modules/withRouter.js +++ b/modules/withRouter.js @@ -13,7 +13,7 @@ export default function withRouter(WrappedComponent, options) { contextTypes: { router: routerShape }, propTypes: { router: routerShape }, getWrappedInstance() { - warning(options && options.withRef, 'To access the wrappedInstance you must provide {withRef : true} as the first argument of the withRouter call') + warning(options && options.withRef, 'To access the wrappedInstance you must provide {withRef : true} as the second argument of the withRouter call') return this.refs.wrappedInstance }, render() { From e9eb17484381854866a3b9eb21334569f6da30f4 Mon Sep 17 00:00:00 2001 From: Thomas Florin Date: Thu, 18 Aug 2016 19:31:04 -0500 Subject: [PATCH 7/8] Fixing code style, and updating ref to be a function ref --- modules/__tests__/withRouter-test.js | 6 ++++-- modules/withRouter.js | 6 +++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/modules/__tests__/withRouter-test.js b/modules/__tests__/withRouter-test.js index cc146c4123..5d2cf16502 100644 --- a/modules/__tests__/withRouter-test.js +++ b/modules/__tests__/withRouter-test.js @@ -63,8 +63,10 @@ describe('withRouter', function () { render(, node, done) }) - it('should support withRefs as a parameter',function (done) { - const WrappedApp = withRouter(App,{ withRef:true }) + it('should support withRefs as a parameter', function (done) { + const WrappedApp = withRouter(App,{ + withRef:true + }) const router = { push() {}, replace() {}, diff --git a/modules/withRouter.js b/modules/withRouter.js index f357290128..41ad625f89 100644 --- a/modules/withRouter.js +++ b/modules/withRouter.js @@ -14,13 +14,13 @@ export default function withRouter(WrappedComponent, options) { propTypes: { router: routerShape }, getWrappedInstance() { warning(options && options.withRef, 'To access the wrappedInstance you must provide {withRef : true} as the second argument of the withRouter call') - return this.refs.wrappedInstance + return this._wrappedComponent }, render() { const router = this.props.router || this.context.router if(options && options.withRef) { - return - }else{ + return this._wrappedComponent = component} router={router} /> + } else { return } } From f482d2b48bd049f9f5088c7189c8354218931964 Mon Sep 17 00:00:00 2001 From: Thomas Florin Date: Thu, 18 Aug 2016 19:35:06 -0500 Subject: [PATCH 8/8] More code style fixes --- modules/__tests__/withRouter-test.js | 4 +--- modules/withRouter.js | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/modules/__tests__/withRouter-test.js b/modules/__tests__/withRouter-test.js index 5d2cf16502..3828c5f508 100644 --- a/modules/__tests__/withRouter-test.js +++ b/modules/__tests__/withRouter-test.js @@ -64,9 +64,7 @@ describe('withRouter', function () { }) it('should support withRefs as a parameter', function (done) { - const WrappedApp = withRouter(App,{ - withRef:true - }) + const WrappedApp = withRouter(App, { withRef:true }) const router = { push() {}, replace() {}, diff --git a/modules/withRouter.js b/modules/withRouter.js index 41ad625f89..eccd94e856 100644 --- a/modules/withRouter.js +++ b/modules/withRouter.js @@ -18,7 +18,7 @@ export default function withRouter(WrappedComponent, options) { }, render() { const router = this.props.router || this.context.router - if(options && options.withRef) { + if (options && options.withRef) { return this._wrappedComponent = component} router={router} /> } else { return