Skip to content

Add a new option to control element comparison#1120

Closed
theKashey wants to merge 5 commits intopreactjs:masterfrom
theKashey:master
Closed

Add a new option to control element comparison#1120
theKashey wants to merge 5 commits intopreactjs:masterfrom
theKashey:master

Conversation

@theKashey
Copy link
Copy Markdown

@theKashey theKashey commented May 25, 2018

Just adding an option (to pair vnode) to hook element comparison. Required by #1007
Another 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 😭)

@coveralls
Copy link
Copy Markdown

coveralls commented May 25, 2018

Coverage Status

Coverage remained the same at 100.0% when pulling 06d2463 on theKashey:master into 7c3e6fe on developit:master.

@theKashey
Copy link
Copy Markdown
Author

New version of React-Hot-Loader with partial Preact support was released.
Waiting for this stuff for full compliance.

* @private
*/
export function areComponentsEqual(component1, component2) {
return options.areComponentsEqual
Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
Author

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.

Copy link
Copy Markdown
Author

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.

Copy link
Copy Markdown
Author

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.

https://jsperf.com/call-or-if/1

@theKashey
Copy link
Copy Markdown
Author

Obsolete

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.

3 participants