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

Added failing test check for Route and forwardRef #6384

Closed
wants to merge 1 commit into from
Closed

Added failing test check for Route and forwardRef #6384

wants to merge 1 commit into from

Conversation

frehner
Copy link
Contributor

@frehner frehner commented Oct 9, 2018

Currently, Route uses the prop-type of func to check the Component prop.

However, that doesn't cover all valid React components, since React.forwardRef returns an object as noted here.

So this failing test is being added to help correct this at some point in the future.

I thought it may be nice to talk about possible solutions here as well:

Hopefully this is helpful!

@frehner
Copy link
Contributor Author

frehner commented Oct 15, 2018

Just checking in to see if this is helpful or if I should just close it out. :)

@mjackson
Copy link
Member

Hi @frehner, thanks for the PR. Yes, I'm interested in fixing this. Maybe the best option for now is to just add a PropTypes.object to the array, and then upgrade to use a PropTypes.Component when that's released?

@mjackson
Copy link
Member

Ah, just noticed this is a duplicate of #6327. Let's follow up there.

@mjackson mjackson closed this Oct 15, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Dec 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants