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.

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

(v3) Calling withRouter(Component, { withRef: true }) doesn't set the ref if there is no router prop available #5767

Closed
ben-chin opened this issue Dec 1, 2017 · 4 comments

Comments

@ben-chin
Copy link

ben-chin commented Dec 1, 2017

I'm unsure if you're still accepting issues for v3, so feel free to close if not.

Version

3.2.0

Test Case

Happy to add if issues are still being accepted for v3.

Steps to reproduce

class ComponentA extends React.Component {
    render() {
        if (props.router) {
            // return something
        } else {
            // return something else
        }
   }
}

export default withRouter(ComponentA, { withRef: true });

Rendering this component with no router means that the ref is never available in getWrappedInstance() (it always returns undefined).

After some debugging, this looks like it's due to this short circuit https:/ReactTraining/react-router/blob/v3/modules/withRouter.js#L36 which was added in #4295 .

It's only after this short circuit that the ref callback is properly set and rendered.
https:/ReactTraining/react-router/blob/v3/modules/withRouter.js#L43

I'm happy to make a PR to supply the ref prop callback to the first return if changes are still being accepted!

@timdorr
Copy link
Member

timdorr commented Dec 1, 2017

If you don't provide a router, what's the point of using withRouter?

@ben-chin
Copy link
Author

ben-chin commented Dec 1, 2017

thanks for the quick response, @timdorr, in ComponentA I need to conditionally render different things based on whether a router is available and on certain states of the location and params (in this case, a Link if it is available, and a button or a otherwise). so I need withRouter wrapping the component

@timdorr
Copy link
Member

timdorr commented Dec 1, 2017

export { ComponentA as OriginalComponentA }
export default withRouter(ComponentA, { withRef: true })

Then just import and use whatever you need:

import ComponentA from './ComponentA'
// or
import { OriginalComponentA } from './ComponentA'

@ben-chin
Copy link
Author

ben-chin commented Dec 4, 2017

hmm, we were actually using ComponentA all over the app which is not fully under React Router, so it was using withRouter to check the existence of the router. Instead of this, I found a workaround by testing for the router's existence in this.context, as the Link component does, and then rendering the relevant child components based on this.

feel free to close this, though I did find it a bit odd that the ref was obscured because of this.

thanks for the help!

@timdorr timdorr closed this as completed Dec 4, 2017
@lock lock bot locked as resolved and limited conversation to collaborators Jan 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants