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

check if type.prototype is object in instantiateReactComponent #3638

Merged
merged 2 commits into from
Apr 9, 2015

Conversation

teosz
Copy link
Contributor

@teosz teosz commented Apr 9, 2015

When I try instantiate a component inside react router nested view I get :
Uncaught TypeError: Cannot read property 'mountComponent' of undefined

@teosz teosz changed the title check if type.prototype is object instantiateReactComponent check if type.prototype is object in instantiateReactComponent Apr 9, 2015
@@ -40,6 +40,7 @@ assign(
function isInternalComponentType(type) {
return (
typeof type === 'function' &&
typeof type.prototype === 'object' &&
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm a little curious how this would interact with ES6 classes if we upgrade to ES6. Is the prototype of an ES6 class a object or a function? I feel like what you are actually wanting to check is if the prototype is undefined in order to avoid throwing on the next line.

@jimfb
Copy link
Contributor

jimfb commented Apr 9, 2015

Looks good. I suggest changing it to check for undefined instead of checking for object, since that's what you're really trying to protect against. Also, let's add a unit test that simulates the failure case you were experiencing and asserts that we now return false.

@jimfb
Copy link
Contributor

jimfb commented Apr 9, 2015

Looks good to me. Clearly Sebastian agrees.

@teosz I'd still like to see a unit test that verifies this functionality (ie. simulates the case you were running into, so we don't ever accidentally break that for you). But I suppose that can be a separate PR.

jimfb added a commit that referenced this pull request Apr 9, 2015
check if type.prototype is object in instantiateReactComponent
@jimfb jimfb merged commit 0f0f5aa into facebook:master Apr 9, 2015
@@ -40,6 +40,7 @@ assign(
function isInternalComponentType(type) {
return (
typeof type === 'function' &&
typeof type.prototype !== 'undefined' &&

Choose a reason for hiding this comment

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

you could just do

type.prototype !== undefined

Copy link
Collaborator

Choose a reason for hiding this comment

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

True. I don't have a particular preference.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

for consistency i verified type like others lines

@zpao zpao added this to the 0.13.2 milestone Apr 14, 2015
zpao pushed a commit to zpao/react that referenced this pull request Apr 14, 2015
check if type.prototype is object in instantiateReactComponent
zpao pushed a commit to zpao/react that referenced this pull request Apr 17, 2015
check if type.prototype is object in instantiateReactComponent
zpao pushed a commit to zpao/react that referenced this pull request Apr 18, 2015
check if type.prototype is object in instantiateReactComponent
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.

7 participants