data-driven-forms / react-forms

React library for rendering forms.
https://data-driven-forms.org/
Apache License 2.0
304 stars 87 forks source link

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

Open lucasgarfield opened 1 year ago

lucasgarfield commented 1 year ago

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.

rvsia commented 1 year ago

Hello, thank you for reporting it.

What about to use PropTypes.elementType, does it contain support for the forwardRef?

Fewwy commented 1 year ago

Encountered the same issue while I am preparing a PR for the insights inventory with new modals. https://github.com/RedHatInsights/insights-inventory-frontend/pull/1755