Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding support to get the component instance that withRouter HOC wraps #3735

Merged
merged 8 commits into from
Aug 19, 2016
Merged

Adding support to get the component instance that withRouter HOC wraps #3735

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
048be89
withRouter now supports withRef as an option, to better support getti…
Bigguy34 Aug 18, 2016
6d861e7
Fixing some spacing issues
Bigguy34 Aug 18, 2016
4bdccb0
Last minute spacing issues
Bigguy34 Aug 18, 2016
9ec7563
Reverting change to withRouter
Bigguy34 Aug 18, 2016
ae24bcf
Updating withRouter based on comments
Bigguy34 Aug 18, 2016
017eb4b
updating error message from not passing in proper options object
Bigguy34 Aug 18, 2016
e9eb174
Fixing code style, and updating ref to be a function ref
Bigguy34 Aug 19, 2016
f482d2b
More code style fixes
Bigguy34 Aug 19, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions modules/__tests__/withRouter-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 <h1>App</h1>
Expand Down Expand Up @@ -59,4 +62,19 @@ describe('withRouter', function () {

render(<Test router={router} test={test} />, 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((<WrappedApp router={router}/>), node, done)
expect(component.getWrappedInstance().testFunction()).toEqual('hello from the test function')
})
})
14 changes: 12 additions & 2 deletions modules/withRouter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would instead use destructuring here. It essentially makes the code self-documenting. So, { withRef } instead of options.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That'd require doing e.g. { withRef: false } = {}, though. The transpiled code for default params is a bit ugly, plus there's that extra temporary object.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh? You can just do withRouter(WrappedComponent, { withRef = false }). But the default is undefined, so it's falsey and would work all the same.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That doesn't work unless the caller explicitly passes in an object as the 2nd arg: http://babeljs.io/repl/#?evaluate=true&lineWrap=false&presets=es2015-loose&experimental=true&loose=true&spec=false&code=function%20foo(%7B%20withRef%20%7D)%20%7B%0A%20%20console.log(withRef)%3B%0A%7D%0A%0Afoo()%3B&playground=false

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, shit, you're right. Forgot about that.

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 second argument of the withRouter call')
return this._wrappedComponent
},
render() {
const router = this.props.router || this.context.router
return <WrappedComponent {...this.props} router={router} />
if (options && options.withRef) {
return <WrappedComponent {...this.props} ref={(component)=>this._wrappedComponent = component} router={router} />
} else {
return <WrappedComponent {...this.props} router={router} />
}
}
})

Expand Down