Consolidate VNode compat options (-62 B)#2116
Conversation
| } | ||
|
|
||
| // TODO: Removing this saves 16 bytes! | ||
| vnode.preactCompatNormalized = true; |
There was a problem hiding this comment.
Anyone know if we need to support this property? Or can we remove it?
There was a problem hiding this comment.
I vote for removing it, it's unused and don't really see a use case for it
There was a problem hiding this comment.
Removed. Since this comment is now outdated (which I think means it'll be collapsed for some), I made a new comment for others to see and discuss.
| * @param {import('./internal').VNode} vnode | ||
| */ | ||
| function normalizeVNode(vnode) { | ||
| vnode.preactCompatNormalized = true; |
There was a problem hiding this comment.
Just calling that I removed this property cuz I don't think it's being used anymore. Holler if you think this is worth the 16 bytes.
There was a problem hiding this comment.
Iirc this is a leftover from earlier iterations. Previous versions would check for this property and only normalize a vnode if it was missing.
marvinhagemeister
left a comment
There was a problem hiding this comment.
So good 🙌💯 It makes the code a lot easier to read 👍
|
Great work @andrewiggins! 🎉 |
Currently the compat src has VNode compat functions spread across a couple different functions making it hard to modify (should I add it
normalizeVNode?createElement?handleElementVNode?).This PR simplifies VNode compat by inline all of those disparate functions into the
options.vnodehook. Every vnode that is created (includingcloneElement) goes through this options hook so it guarantees VNode compat is consistently applied.Also saves some bytes!