react-redux: connect() does not work with React.forwardRef

We chatted with @sebmarkbage and he pointed out that if you want to “redirect” ref to some inner component, forwardRef needs to be outside of connect, not inside it.

class MyComponent extends React.Component {
  render() {
    return <SomeInnerComponent ref={this.props.myForwardedRef} />;
  }
}

const ConnectedMyComponent = connect(
  mapStateToProps
)(MyComponent);

export default forwardRef((props, ref) =>
  <ConnectedMyComponent {...props} myForwardedRef={ref} />
);

There is also another scenario in which you might want to point connect()ed component’s ref to the wrapped component’s own instance (MyComponent in this example). Arguably this is what most React Redux users would want. You can’t do this in userland. It would need to be done by connect() itself because it would need to pass ref={forwardedRef} to the wrapped instance (which isn’t possible from the user code).

I was confused on what that exactly means, so I can spare you the research: use { forwardRef: true } as option to connect

This breaks withTheme of styled-components v4 as they are using forwardRef:

https://github.com/styled-components/styled-components/blob/2b87a5f7e307b16e1b18dc31467d1f04cfe4738a/src/hoc/withTheme.js#L9

I slightly changed the solution proposed by @evannorton so that it could be drop-in replacement of connect.

import { forwardRef } from 'react';
import { connect } from 'react-redux';

const connectAndForwardRef = (
  mapStateToProps = null,
  mapDispatchToProps = null,
  mergeProps = null,
  options = {},
) => component => connect(
  mapStateToProps,
  mapDispatchToProps,
  mergeProps,
  {
    ...options,
    forwardRef: true,
  },
)(forwardRef(component));

const ConnectedMyComponent = connectAndForwardRef(mapStateToProps, mapDispatchToProps)(MyComponent)

5.1.0 is out! https://github.com/reduxjs/react-redux/releases/tag/v5.1.0

Do we not pass the ref prop through? In which case, wouldn’t this work?

class MyComponent extends React.Component {
  render() {
    return <SomeInnerComponent ref={this.props.myForwardedRef} />;
  }
}

const ConnectedMyComponent = connect(
  mapStateToProps
)(MyComponent);

export default forwardRef((props, ref) =>
  <ConnectedMyComponent {...props} ref={ref} />
);

@JLarky : As JLarky suggested passing extra parameter did the trick

export default connect(mapStateToProps, mapDispatchToProps, null, { forwardRef: true })(MyContainer);

More specifically, do that only in 6.0+.

#1062 just landed and closes this out. I’ll look at packaging it up for release (namely, if react-is still bloats things a lot). Hopefully today, but I an just a mere mortal.

The fix would be to use require('react-is').isValidElementType() instead of a strict function check. See https://github.com/facebook/react/issues/12453.

react-is is a new package we just published.

We were originally planning to address this as part of the larger React-Redux v6 work, as seen in #995 and #1000 . However, given that this breaks with React 16.6’s React.memo(), I’m inclined to say we should put out a 5.0.8 / 5.1.0 release that only switches to using react-is instead of the current check.

I don’t have time to tackle this one myself right now, but if someone can file a PR, @timdorr and I can figure out the release strategy.

Also, found a polyfill, but it looks pretty young: https://github.com/soupaJ/create-react-ref

I would recommend against using this polyfill for this reason:

Caveats

The polyfilled forwardRef is only compatible with refs created from createRef and not compatible with ref callbacks/functions. If you attach a ref callback to a component returned from the polyfilled forwardRef, you will get a RefForwarder component instance. This is one instance of how this library differs from React’s implementation.

The forwardRef method would be nice to polyfill, except that it can’t be done without introducing a non-standard API. Anyone that used the polyfill would have to deviate from the official API, which would probably cause confusion in the larger community and be more trouble than it’s worth.

Does passing a forwarded ref type also break displayName and hoistStatics as well?

Yes.

I’ll look into this today at some point.

For those of you just joining —

A fix for this problem (and many other concerns related to React 16.4) is in progress in #1000.

Here’s to hoping it lands soon. Many thanks to @cellog for taking this on.

The React-Redux fix would be just remove the invariant?

Yes. This would be a breaking change, because we’ll be removing the withRef option at that point, so it’s targeted for 6.0. Please see the roadmap in #950 for our plans.

Is this feature inside some roadmap? We are removing connect components because they are not able to pass the ref, which is really needed.

I had no awareness of this forwardRef stuff until my app started catastrophically failing today

Your app broke because you updated a component library (not React!) to a version that includes a breaking change (using forwardRef). Note that React adding forwardRef is not a breaking change; a component library using forwardRef is.

I don’t know whether that component library used a major or a minor release for this, but if you don’t want your app to break on random dependency updates and people’s mistakes, I encourage you to use a lockfile. Both npm and Yarn provide lockfiles by default. If you like “living on the edge” and having unreproducible builds, it’s not surprising your app breaks every now and then.

Again, I want to reiterate there was no breaking change on React’s part. I’m sorry that this new feature caused confusion and churn for you, but I don’t see why it would be a problem on the React side.

Yes, the design you mentioned was considered. It was rejected because it would incur a performance penalty as we’d have to check every single component for a special field (instead of immediately recognizing “special” components through a different type).

This just burned me when react-jss started using React.forwardRef: https://github.com/cssinjs/react-jss/issues/255 😢 The fact that forwarding a ref to the wrapped component in an HoC is difficult and this attempted fix is just causing more problems tells me that we really shouldn’t be using HoCs any more. Render props components are far less prone to all this annoying awkwardness.

Sorry, I’m understanding the issue now: The ref prop doesn’t show up on this.props without forwardRef. I wasn’t making that connection.

We can definitely look at this with regards to going 16+ only. Is there any sort of polyfill available?

Seems to me like we’re starting to pile up a pretty big list of potential breaking changes here, between this and all the other 16.x-related aspects. And React.forwardRef() is 16.3-only.

Connect is passing all props down. So yes, if the consumer uses forwardRef it will work.

The proposal is that connect itself should use forwardRef internally. Then this would work:

class Something extends Component {
  focus() { ... }
}

const Connected = connect(Something)(mapStateToProps)

// Later
<Connected ref={inst => inst && inst.focus()} />

forwardRef() was added primarily for HOCs (which connect is) to be used inside of them.

If this change was made then the connect() wrapper itself would be unobservable. And getWrappedInstance() would need to be removed.

Otherwise, this test would fail

I’m not sure why it’s related. This test doesn’t assert anything about whether this.c is the connect wrapper or the inner component. Moreover it is impossible to “redirect” the ref prop without forwardRef so I don’t see what you mean by it working already.