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

Use new forwardRef API in withRouter HOC #6056

Closed
sanniassin opened this issue Apr 2, 2018 · 21 comments
Closed

Use new forwardRef API in withRouter HOC #6056

sanniassin opened this issue Apr 2, 2018 · 21 comments

Comments

@sanniassin
Copy link
Contributor

React 16.3 is out and now we have an "official" way to pass ref through HOCs with new forwardRef API. Would you like to use it as a replacement for wrappedComponentRef prop to make wrapping transparent for parent components?

Can submit a PR.

@sanniassin sanniassin changed the title Use new forwardRef API in withRouter wrapper Use new forwardRef API in withRouter HOC Apr 2, 2018
@pshrmn
Copy link
Contributor

pshrmn commented Apr 2, 2018

@sanniassin The issue with using forwardRef right now is that React Router still supports React < 16.3. Unless there is a polyfill for forwarding refs like there is with new/legacy context via create-react-context, RR won't be able to support this until its React min dep is increased.

@sanniassin
Copy link
Contributor Author

@pshrmn It should be easy to check if forwardRef is available and pass ref argument only if it is. The only problem is potential breaking change when ref callbacks on wrapped components suddenly become callable, but I don't think it's a real problem because React alerts you if you try to define ref on stateless component without ref forwarding.

@timdorr
Copy link
Member

timdorr commented Apr 2, 2018

We're dealing with this on react-redux as well: reduxjs/react-redux#914

I think it's mainly about forwarding in the ref properly, not anything to do with BC support.

@jathanasiou
Copy link

jathanasiou commented May 9, 2018

I just decided to move to the forwardRef API for the HOCs in our application because there is a large amount of logic relying on ref for various components that I recently rewrote with composition. Of course right now the Route calls are complaining with:

Warning: Failed prop type: Invalid prop `component` of type `object` supplied to `Route`, expected `function`. in Route

as the object being passed as component prop to <Route> right now is

console.log('MyWrappedComponent=', MyWrappedComponent);
// MyWrappedComponent= {$$typeof: Symbol(react.forward_ref), render: ƒ}
<Route path="/myPath" component={MyWrappedComponent} />

Is there any workaround that could be used to forward refs to Components wrapped by a HOC while simultaneously having functional routing?

@kevinL5
Copy link

kevinL5 commented May 25, 2018

@jathanasiou I have the same problem. Did you find a solution?

@jathanasiou
Copy link

jathanasiou commented May 25, 2018

@kevinL5 It's been a while since I looked into this matter but I think I had partial success with passing the refs through my outer HOCs like so:

export default function outerHOC() {
	return function(WrappedComponent) {
		return class Better extends React.Component {
			render() {
				// ...
				return <WrappedComponent passedRef={this.props.passedRef} { ...this.props}/>
			}
		}
	}
}

// unlike the Better HOC, this always wraps MyActualBaseComponent
export default function innerHOC() {
	return function(WrappedComponent) {
		return class Stronger extends React.Component {
			render() {
				// ...
				return <WrappedComponent ref={this.props.passedRef} { ...this.props}/>
			}
		}
	}
}

class MyActualBaseComponent extends React.Component {/*...*/}
export outerHOC()(innerHOC()(MyActualBaseComponent))


import BetterStronger from './MyActualBaseComponent'
class IndexControlComponent extends React.Component {
	constructor(props) {
		super(props)
		this.neededRef = React.createRef()
	}
	render() {
	const betterStronger = <BetterStronger passedRef={this.neededRef} {/*some props*/} />
	}
	needyFunction () => { // needs to control the wrapped MyActualBaseComponent instance
		this.neededRef.current.doSomethingBetter()
		this.neededRef.current.doSomethingStrong()
	}
	// ...
}

It is a very hacky workaround however. Ideally we should try to avoid using refs with HOCs until this issue is resolved on react-router.

@mathieuprog
Copy link

mathieuprog commented Jul 12, 2018

Same problem with react-router using forwardRef

Warning: Failed prop type: Invalid prop component of type object supplied to Route, expected function. In Route [...]

Workaround for now is to use the render prop of the Route component instead of the component prop:
<Route exact path="/users" component={Users}/>
becomes
<Route exact path="/users" render={() => <Users />}/>

@mjackson
Copy link
Member

mjackson commented Nov 2, 2018

Yes, we should update withRouter to use the React.forwardRef API. As @pshrmn noted above, we can't do it in 4.x because it is compatible with React 15 and the forwardRef API was introduced in 16.3.

We will fix this in version 5 when we upgrade our dep to React 16.7+.

@mjackson mjackson added this to the 5.0 milestone Nov 2, 2018
@oreporan
Copy link

oreporan commented Nov 22, 2018

After much searching, I ended up creating a withForwardingRef

const withForwardingRef = <Props extends {[_: string]: any}>(BaseComponent: React.ReactType<Props>) =>
    React.forwardRef((props, ref) => <BaseComponent {...props} forwardedRef={ref} />);

export default withForwardingRef;

usage:

const Comp = ({forwardedRef}) => (
 <button ref={forwardedRef} />
)
const MyBeautifulComponent = withForwardingRef<Props>(Comp);  // Now Comp has a prop called forwardedRef

usage of usage:

<MyBeautifulComponent ref={someRef} />

@esellin
Copy link

esellin commented Apr 1, 2019

I see that v5 is out now (https://reacttraining.com/blog/react-router-v5/), but I'm struggling to find any documentation on how to use forwardRef and withRouter() together. Can anyone help?

@timdorr
Copy link
Member

timdorr commented Apr 1, 2019

@esellin We haven't bumped the React version in 5.0, so we can't yet support forwardRef.

@eliassotodo
Copy link

eliassotodo commented May 9, 2019

I just renamed the ref prop in the outter componente to myRef and passed it down to the component I want a ref from.

@charlax
Copy link

charlax commented Jun 6, 2019

This also applies to NavLink btw, and since material-ui's v4, it's raising a lot of warnings for things such as <ListItem component={NavLink} ...>...

Warning: Failed prop type: Invalid prop component supplied to ForwardRef(ButtonBase). Expected an element type that can hold a ref. Did you accidentally provide a plain function component instead? For more information see https://material-ui.com/guides/composition/#caveat-with-refs

@eps1lon
Copy link
Contributor

eps1lon commented Jun 6, 2019

@charlax And this is why the section describes a workaround. We even explicitly document it for react-router: https://material-ui.com/components/links/#third-party-routing-library

@charlax
Copy link

charlax commented Jun 11, 2019

Indeed! I knew about this page but did not think to look it up when using NavLink. Thanks!

@ranjith-s
Copy link

React 16.3 is out and now we have an "official" way to pass ref through HOCs with new forwardRef API. Would you like to use it as a replacement for wrappedComponentRef prop to make wrapping transparent for parent components?

Can submit a PR.

React 16.3 is out and now we have an "official" way to pass ref through HOCs with new forwardRef API. Would you like to use it as a replacement for wrappedComponentRef prop to make wrapping transparent for parent components?

Can submit a PR.

Finally, I have done this way! this will work for sure

import { withRouter } from 'react-router';

//Just copy and add this withRouterAndRef HOC

const withRouterAndRef = (WrappedComponent) => {
class InnerComponentWithRef extends React.Component {    
      render() {
          const { forwardRef, ...rest } = this.props;
          return <WrappedComponent {...rest} ref={forwardRef} />;
      }
  }
  const ComponentWithRouter = withRouter(InnerComponentWithRef, { withRef: true });
  return React.forwardRef((props, ref) => {
      return <ComponentWithRouter {...props} forwardRef={ref} />;
    });
}

class MyComponent extends Component {
   constructor(props) {
      super(props);
   }
}

//export using withRouterAndRef

export default withRouterAndRef (MyComponent)

@pandaiolo
Copy link

As suggested in original SO question, here is a version built on @ranjith-s work, a bit simpler and that supports displayName which can be useful in tests or dev tools.

const withRouterAndRef = Wrapped => {
  const WithRouter = withRouter(({ forwardRef, ...otherProps }) => (
    <Wrapped ref={forwardRef} {...otherProps} />
  ))
  const WithRouterAndRef = React.forwardRef((props, ref) => (
    <WithRouter {...props} forwardRef={ref} />
  ))
  const name = Wrapped.displayName || Wrapped.name
  WithRouterAndRef.displayName = `withRouterAndRef(${name})`
  return WithRouterAndRef
}

@stale
Copy link

stale bot commented Nov 5, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the stale label Nov 5, 2019
@timdorr timdorr removed the stale label Nov 5, 2019
@KaDaniil
Copy link

KaDaniil commented Dec 6, 2019

I think it is easier to create your own component like this

import React from 'react';
import { Link as MaterialLink } from '@material-ui/core';
import { Link as RouterLink } from 'react-router-dom';

export default function (props) {
  const MiddlewareLink = React.forwardRef((props, ref) => <RouterLink innerRef={ref} {...props} />);
  return <MaterialLink component={MiddlewareLink} {...props} />;
}

@stale
Copy link

stale bot commented Feb 4, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
You can add the fresh label to prevent me from taking any action.

@stale stale bot added the stale label Feb 4, 2020
@eps1lon
Copy link
Contributor

eps1lon commented Feb 4, 2020

I guess this can be closed anyway since withRouter is no longer part of the API in v6.

@stale stale bot removed the stale label Feb 4, 2020
@timdorr timdorr closed this as completed Feb 4, 2020
@lock lock bot locked as resolved and limited conversation to collaborators Apr 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.