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

Add ReactIs.isValidElementType() #12483

Merged
merged 3 commits into from
Mar 29, 2018

Conversation

jamesreggio
Copy link
Contributor

@jamesreggio jamesreggio commented Mar 29, 2018

Per the conversation on #12453, there are a number of third-party libraries (particularly those that generate higher-order components) that are performing suboptimal validation of element types.

This commit exposes a function that can perform the desired check without depending upon React internals.

In #12453, we discussed exposing this check within the new react-is package, but as I dug in, I lost confidence that that was indeed the right course of action.

Today, element type validation occurs within ReactElementValidator. ReactElementValidator has a dependency upon ReactElement, but lacks a dependency upon react-is. (In fact, the nothing within the core react package takes a dependency upon react-is.)

It felt wrong to duplicate the existing type validation code in a separate package, and it also felt wrong to factor it into react-is and introduce a new dependency from react to react-is. As such, I placed the type validation logic alongside React.isValidElement within ReactElement.

I realize that expanding the surface area of React's top-level API is a dicey prospect, but I wanted to explain my reasoning and propose it in code. If you think an alternative implementation would be more ideal, please just let me know and I'll revise.

Per the conversation on facebook#12453, there are a number of third-party
libraries (particularly those that generate higher-order components)
that are performing suboptimal validation of element types.

This commit exposes a function that can perform the desired check
without depending upon React internals.
@gaearon
Copy link
Collaborator

gaearon commented Mar 29, 2018

It felt wrong to duplicate the existing type validation code in a separate package, and it also felt wrong to factor it into react-is and introduce a new dependency from react to react-is.

You could put it in shared which is accessible by all packages (and would get copied in either).

Copy link
Contributor

@bvaughn bvaughn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

Needs an update to the README but I can add that separately.

@bvaughn bvaughn merged commit 96fe3b1 into facebook:master Mar 29, 2018
@gaearon gaearon changed the title Add React.isValidElementType() Add ReactIs.isValidElementType() Mar 29, 2018
@jamesreggio
Copy link
Contributor Author

Thanks for the quick review.

@jamesreggio jamesreggio deleted the add-react-isvalidelementtype branch March 29, 2018 19:36
This was referenced Mar 30, 2018
rhagigi pushed a commit to rhagigi/react that referenced this pull request Apr 19, 2018
* Add React.isValidElementType()

Per the conversation on facebook#12453, there are a number of third-party
libraries (particularly those that generate higher-order components)
that are performing suboptimal validation of element types.

This commit exposes a function that can perform the desired check
without depending upon React internals.

* Move isValidElementType to shared/
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants