Add package: @wordpress/is-shallow-equal #110
Conversation
Codecov Report
@@ Coverage Diff @@
## master #110 +/- ##
==========================================
+ Coverage 64.07% 67.53% +3.45%
==========================================
Files 42 56 +14
Lines 604 693 +89
Branches 118 142 +24
==========================================
+ Hits 387 468 +81
- Misses 175 183 +8
Partials 42 42
Continue to review full report at Codecov.
|
| 4. …be intended for use in non-Facebook projects. | ||
| - Sorry, [`fbjs/lib/shallowEqual`](https://www.npmjs.com/package/fbjs). | ||
| 5. …be the fastest implementation. | ||
| - Sorry, _every other solution_. |
| @@ -0,0 +1,90 @@ | |||
| var isArray = Array.isArray, | |||
| keys = Object.keys; | |||
There was a problem hiding this comment.
Are you using var for performance reasons? Thinking these may create ESlint errors if we add strict rules (const, let).
There was a problem hiding this comment.
Are you using
varfor performance reasons? Thinking these may create ESlint errors if we add strict rules (const, let).
I'm using var because I'm effectively opting out of any transpilation (wanting full control over code being executed). I think if we were to introduce such rules, we should apply them to each package's src folder, not necessarily a raw implementation as is done here.
packages/is-shallow-equal/index.js
Outdated
|
|
||
| var aKeys = keys( a ), | ||
| bKeys = keys( b ), | ||
| length = aKeys.length; |
There was a problem hiding this comment.
Can't we use separate var for each one?
There was a problem hiding this comment.
Can't we use separate
varfor each one?
We can, but my obsessive-yet-likely-often-misled interpretations of benchmark results had me do very specific code style 😄
There was a problem hiding this comment.
But in reality, these are not that important because in general these get mangled, minified in the app bundles.
There was a problem hiding this comment.
Fair point, at least for web bundles. There's still application in a Node script, which would use this code as written.
I should note that I'm not even certain this was the reason for the particular style in this case, just a general note that common style best practices which are encouraged more for maintainability's sake were deemphasized here. The file would probably need a fair few number of eslint-disable.
packages/is-shallow-equal/index.js
Outdated
| } | ||
|
|
||
| var aKeys = keys( a ), | ||
| bKeys = keys( b ), |
There was a problem hiding this comment.
I wonder if a for in loop could be better than relying on keys performance wise?
There was a problem hiding this comment.
I guess this one is better if the keys length is different?
There was a problem hiding this comment.
Object.keys is actually quite fast. Faster than for in: https://jsperf.com/object-keys-iteration/20
And yes, there's a shortcutting opportunity here that we can assume the two values are different if they have keys of unequal length. Also not just for shortcutting, but verifying in case the second object has more keys than the first that we don't miss out on considering those (a = { foo: 1 }, b = { foo: 1, bar: 2 })
|
#111 seeks to solve the codecov lowering by not checking the benchmark script. |
No need to affect the codecoverage stats with a benchmarking script
aduth
force-pushed
the
add/is-shallow-equal
branch
from
6c2feeb
to
703d20e
Compare
Otherwise considered a proper required dependency from perspective of consuming project
Related: WordPress/gutenberg#6313
Related: WordPress/gutenberg#5616
This pull request seeks to add a new package:
@wordpress/is-shallow-equal. In my pursuit of the perfect shallow equality utility, I've come to the conclusion that for various reasons, none of the existing solutions are quite perfect. Since a function likeisShallowEqualis often used for performance-critical code, anything less than perfection was not adequate. I've enumerated all of these reasons in the included README.md. To the point of performance specifically, contrasted withshallowequalas used in Gutenberg, there is shown to be a ~40% improvement.View README
Testing instructions:
Verify tests pass: