React.forwardRef results in propTypes etc. being hoisted

[eps1lon]

propTypes and defaultProps are still processed statics for react in forwardRef components (other statics might be too but those two were the most straight forward to verify). They should therefore not be hoisted.

Edit:
displayName might be another issue. It's probably safe to assume that FORWARD_RED_STATICS should be merged if the sourceComponent is a forwardRef component instead of replacing all react statics.

full sandbox

Edit: broken example; see #66 (comment) for fixed code

import React from "react";
import ReactDOM from "react-dom";
import hoistStatics from "hoist-non-react-statics";
import PropTypes from 'prop-types';

const App = React.forwardRef((props, ref) => <div {...props} ref={ref} />)
App.propTypes = {
  id: PropTypes.number.isRequired
}

App.defaultProps = {
  children: 'Hello, World',
}

const withDefaultId = id => Component => {
  function EnhancedComponent({innerRef, ...props}) {
    return <Component id={id} {...props} ref={innerRef} />
  }

  return hoistStatics(EnhancedComponent, Component);
}

// we should now be able to savely use AppWithId without propTypes warning
// that `id` is missing since with injected it. But propTypes is being hoisted 
// so react warns anyway
const AppWithId = withDefaultId('default-id')(App);

const rootElement = document.getElementById("root");
ReactDOM.render(<React.StrictMode><AppWithId innerRef={r => console.log(r)} /></React.StrictMode>, rootElement);

[mridgway]

I think this is only currently a problem for hoisting from ForwardRef to ForwardRef since we also check against the target component's list of statics as well. In your example, the only warning I'm seeing is that id is an invalid type (string instead of number).

I think this is solvable for ForwardRef to ForwardRef hoisting (and future types if there are more).

[eps1lon]

Right. I constructed this from the code but thought that my mistake is yours. I though this was also an issue for forwardRef -> class component but was unable to reproduce.

I update the sandbox to illustrate the actual issue you described:

import React from "react";
import ReactDOM from "react-dom";
import hoistStatics from "hoist-non-react-statics";
import PropTypes from 'prop-types';

const App = React.forwardRef((props, ref) => <div {...props} ref={ref} />)
App.propTypes = {
  id: PropTypes.number.isRequired
}

App.defaultProps = {
  children: 'Hello, World',
}

const withDefaultId = id => Component => {
  const EnhancedComponent = React.forwardRef(function Enhanced(props, ref) {
    return <Component id={id} {...props} ref={ref} />
  })

  return hoistStatics(EnhancedComponent, Component);
}

// we should now be able to savely use AppWithId without propTypes warning
// that `id` is missing since with injected it. But propTypes is being hoisted 
// so react warns anyway
const AppWithId = withDefaultId(1)(App);

const rootElement = document.getElementById("root");
ReactDOM.render(<React.StrictMode><AppWithId ref={r => console.log(r)} /></React.StrictMode>, rootElement);

[oliviertassinari]

I have just been hit by the same issue with displayName. Here is a pseudo reproduction:

// Component is using forwardRef

function WithStyles(props) {
  return <Component {...props} />
}

WithStyles.displayName = 'Foo';

hoistNonReactStatics(WithStyles, Component);

// WithStyles.displayName === Component.displayName, it should be Foo

[eps1lon]

@oliviertassinari Looks like you're using an outdated version. Latest only has an issue if both components are forwardRef:

const Component = React.forwardRef((props, ref) => null)
  Component.displayName = 'Bar';

  function WithStyles(props) {
    return <Component {...props} />
  }
  WithStyles.displayName = 'Foo';
  
  hoistStatics(WithStyles, Component);
  // passes
  console.assert(WithStyles.displayName === 'Foo');

  const BadStyles = React.forwardRef((props, ref) => null);
  BadStyles.displayName = 'Foo';

  hoistStatics(BadStyles, Component);
  // only this fails
  console.assert(BadStyles.displayName === 'Foo');

[oliviertassinari]

@eps1lon Yes, both components are using forwardRef, like in your example. I should have taken more time and provide an actual reproduction.

[eps1lon]

Working on a fix.

[oliviertassinari]

It's worse than what I thought. On a Node.js environment, it overrides the render method 😱.

[eps1lon]

Sounds like a separate issue. It should already prevent hoisting of render. There's a test for that already. Better open a new issue for that.