React-redux: connect should hoist wrapped component's propTypes if they exist

[lencioni]

When using this HOC in development, propTypes warnings will normally appear in the console. This is great! However, if you are using shallow rendering in tests, and rely on propTypes warnings to be surfaced by your test suite, they will not be surfaced because they are not hoisted from the wrapped component.

This is further problematic if you are combining multiple HOCs on the same component, and one of them uses something like forbidExtraProps.

What do you think about adding some logic to connect that copies propTypes (and necessarily defaultProps) if they exist on the wrapped component?

cc @ljharb @barberdt

[ljharb]

+1 on propTypes - copying defaultProps isn't helpful for warnings, but it is helpful so that MyComponent.defaultProps = { ...InnerComponent.defaultProps }; isn't a breaking change when InnerComponent changes from unwrapped → wrapped.

In other words, starting to use an HOC, or ceasing to use one, should be utterly transparent to consumers of the wrapped/unwrapped component - and one way to help ensure that is to make sure that statics, including defaultProps and propTypes, are meaningfully similar on the wrapper as on the wrapped.

[jimbolla]

This fails immediately...

MyComponent.propTypes = {
  name: string.isRequired
} 

const mapStateToProps = state => state.name

Hoisting prop types would cause that to complain about name. The propTypes for the connected component are often not the same as the unconnected component.

[ljharb]

@jimbolla that would only fail if the wrapper did not declare its own propTypes. If it does so, and merges its own propTypes on top of the hoisted propTypes, there'd be no issue.

[ljharb]

Perhaps a swift close isn't warranted, simply because you provided one example where it would cause problems? We've provided examples where not hoisting causes problems, so I think it clearly warrants further discussion.

[markerikson]

I think Jim's point is that an HOC is not a "transparent wrapper". The wrapped component could have completely different props than the wrapper, and the specific case of connect is a good example of that. The wrapped component will be expecting props that come from mapState/mapDispatch, but the HOC wrapper does _not_ ever see or use those props itself.

[ljharb]

Yes, that's totally true - but mappers can include wrapper props in the returned props as well, and in our codebase, often do.

Surely there's some way we could perhaps provide an explicit list of prop names that we're going to be returning from the map functions, and then the wrapper could hoist the propTypes for those props?

[jimbolla]

This seems solvable in userland without needing to modify connect. One could wrap connect and add the desired behavior.