Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix PropType of Formatted{,HTML}Message to accept any component #1228

Closed
wants to merge 1 commit into from
Closed

Fix PropType of Formatted{,HTML}Message to accept any component #1228

wants to merge 1 commit into from

Conversation

wereHamster
Copy link
Contributor

A React component is not the same as a React element! In our case, the tagName prop can be anything that can be passed to React.createElement(). Unfortunately the prop-types package doesn't provide a suitable validation function, so people are forced to build their own.

See facebook/prop-types#200 for a related discussion.

A React component is not the same as a React element! In our case,
the tagName prop can be anything that can be passed to
React.createElement(). Unfortunately the prop-types package doesn't
provide a suitable validation function, so people are forced to build
their own.

See facebook/prop-types#200 for a related
discussion.
@yahoocla
Copy link

yahoocla commented Jan 7, 2019

Thank you for submitting this pull request, however I do not see a valid CLA on file for you. Before we can merge this request please visit https://yahoocla.herokuapp.com/ and agree to the terms. Thanks! 😄

@wereHamster
Copy link
Contributor Author

@yahoocla please check again.

@redonkulus
Copy link
Member

ljharb mentions in that issue:

"it has a render function" is a pretty brittle check, when react-is has a robust isForwardRef method available.

Should we try to leverage react-is as well?

@wereHamster
Copy link
Contributor Author

I didn't want to add another dependency. Ideally prop-types would provide that validation itself, once it does you can switch to it. Until that time, this is a net improvement IMO.

@papasmile
Copy link
Contributor

There is a PR out for adding 'component' to prop-types and it relies on react-is. I think copying some of what was done there would be better than a workaround: facebook/prop-types#211

@wereHamster
Copy link
Contributor Author

Alright, I'll close this and wait until the prop-types package provides the validation. In the meantime, I'll be using this to silence the warning (I rely on TypeScript anyway):

if (process.env.NODE_ENV !== "production") {
  require("react-intl").FormattedMessage.propTypes.tagName = require("prop-types").any;
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants