Only run proptypes checks on element creation like React. #370
Conversation
| ) { | ||
| props.children = vnode.children; | ||
| } | ||
| propsHook(props); |
There was a problem hiding this comment.
Thought: does this need to be called even if DEV=false?
There was a problem hiding this comment.
It is actually called all the time. see here.
There was a problem hiding this comment.
I think I see what you mean now. 🤔 I don't think so. I could have sworn we fixed this a while back but I can't find where.
There was a problem hiding this comment.
This is not an issue. We are rearranging the props so that prop-types detects the children correctly.
|
This is a reminder for myself to merge |
|
@developit do you still want to merge this? Is there something concerning you about this? |
|
Yup, I'm just insanely bad at keeping up with PRs right now |
| @@ -270,10 +270,35 @@ function statelessComponentHook(Ctor) { | |||
| return Wrapped; | |||
| } | |||
|
|
|||
| function validatePropTypes (vnode) { | |||
| if (DEV) { | |||
| let componentClass = typeof vnode.nodeName === "function" | |||
There was a problem hiding this comment.
I think this can just always look at vnode.nodeName, right?
|
|
||
| function createElement(...args) { | ||
| upgradeToVNodes(args, 2); | ||
| return normalizeVNode(h(...args)); | ||
| let node = h(...args); | ||
| validatePropTypes(node); |
There was a problem hiding this comment.
Just had a thought (I think I was noodling on this when I failed to review before):
What if we moved the invocation of this validation into handleComponentVNode()? That would make proptype validation work in non-compat components if you install compat, which is kinda compelling. Also it would nicely reuse the existing logic around detecting and forking for component VNodes vs element VNodes.
There was a problem hiding this comment.
I honestly got this working in a very specific way and I don't remember all the details. I do remember that I had good reasons for doing it the way I did though 😅
No description provided.