-
Notifications
You must be signed in to change notification settings - Fork 47k
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
More helpful message when passing an element to createElement() #13131
Conversation
@@ -327,7 +327,10 @@ export function createElementWithValidation(type, props, children) { | |||
info += getStackAddendum() || ''; | |||
|
|||
let typeString; | |||
if (type === null) { | |||
if (type && type.$$typeof) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's move this into the last else
clause instead?
Also let's make the check more concrete, e.g. type.hasOwnPropery('$$typeof')
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Definitely will use hasOwnProperty
over dot notation.
Just to be clear, are you saying to nest another if
statement inside that last else
clause or make this the last else if
before the else
below?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Going to go with the latter to avoid any nested if
statements (ugly). If you want it the other way just lmk.
This will need tests too ;-) |
@@ -333,6 +333,10 @@ export function createElementWithValidation(type, props, children) { | |||
typeString = 'array'; | |||
} else { | |||
typeString = typeof type; | |||
if (type && type.hasOwnProperty('$$typeof')) { | |||
typeString = 'element'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"React element" would be better here.
Also now that I think of it let's compare type.$$typeof === REACT_ELEMENT_TYPE
explicitly. We don't actually "own" this convention. You can get REACT_ELEMENT_TYPE
from existing ReactSymbols
import in this file.
@gaearon totally understand you're a busy guy and this is probably a low priority PR, just wondering if there is anything I need to do on my end to get this ready for merge |
Been on a vacation :-) Checking it out. |
Made a few tweaks. Thanks! |
…book#13131) * [facebook#13130] Add a more helpful message when passing an element to createElement() * better conditional flow * update after review * move last condition inside last else clause * Added test case * compare 25132typeof to REACT_ELEMENT_TYPE * runs prettier * remove unrelated changes * Tweak the message
Fixes #13130
createElement
in development.