-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Add a new option to control element comparison #1120
Conversation
New version of React-Hot-Loader with partial Preact support was released. |
src/vdom/index.js
Outdated
* @private | ||
*/ | ||
export function areComponentsEqual(component1, component2) { | ||
return options.areComponentsEqual |
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.
One could remove the if
check here by making it the default implementation for options.areComponentsEqual
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.
Pro - speed matters
Cons - you will be unable to revert configuration. Not a big deal, but could be nasty.
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.
Probably one if
here is way faster than one more function call you are asking for.
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.
@marvinhagemeister - you were right.
My variant is 4x times slower than yours, and yours are equal to the base line.
Obsolete |
Just adding an option (to pair
vnode
) to hook element comparison. Required by #1007Another option is to use @arqex's code - master...arqex:master with setComponentComparator function. It is just a matter of taste.
Looking forward to make Preact quite 🔥[loadable]
(😕I did not found tests for vnode, and did not add my ones next to them 😭)