react-forms icon indicating copy to clipboard operation
react-forms copied to clipboard

Upgrading `@patternfly/react-core` causes propTypes error in Pf4FormTemplate

Open lucasgarfield opened this issue 3 years ago • 2 comments

Scope:: Pf4FormTemplate

Description: After upgrading @patternfly/react-core to the latest version, the following console error appears when running our app in development mode: Warning: Failed prop type: Invalid prop FormWrappersupplied toFormTemplate, expected one of type [function].

The error originates from the PF4FormTemplate component which imports Form from @patternfly/react-core: https://github.com/data-driven-forms/react-forms/blob/master/packages/pf4-component-mapper/src/form-template/form-template.js#L6

Form is then supplied as the FormWrapper prop to FormTemplate: https://github.com/data-driven-forms/react-forms/blob/master/packages/pf4-component-mapper/src/form-template/form-template.js#L79

FormTemplate in term expects FormWrapper to be a func propType: https://github.com/data-driven-forms/react-forms/blob/master/packages/common/src/form-template/form-template.js#L174

I have discovered the cause of the problem. https://github.com/patternfly/patternfly-react/pull/7995/commits/be2c23fa93d4442f6c79642d8ff9959d319ccb18 refactored the Form component to support ref forwarding. Previously the typeof of Form was a function, however typeof of forwardRef is apparently actually object. This results in incorrect propTypes and a console error. This exact issue is discussed at length here: https://github.com/facebook/react/issues/12453.

We are tracking the issue as it relates to our app here: https://github.com/RedHatInsights/image-builder-frontend/issues/907

I am not sure what a good solution looks like. Per the React docs, introducing forwardRef should be considered a breaking change. Unfortunately, that was not done. https://reactjs.org/docs/forwarding-refs.html#note-for-component-library-maintainers

Updating the propType would solve the problem in our app, but would be a breaking change (which is arguable, I suppose, given it is just a type warning) for anyone using a version of @patternfly/react-core released prior to the introduction of forwardRef in the Form component.

lucasgarfield avatar Jan 09 '23 17:01 lucasgarfield