Closed schester44 closed 2 years ago
I assume you mean https://github.com/schester44/deep-object-diff/commit/7caf8a1dd8990a343753e389d3b3a9accad16558. I like it. I think you should PR!
Comments so far:
The pattern of getting Object.keys(l)
and then iterating it comes up a lot. I think it could be extracted to a function to clarify. I imagine it should still perform well?
How did you benchmark? A benchmark script would be handy for other optimisation also, and for checking we're on the same page, and re-checking when Node updates, and etc.
I'd omit the version bump commit (https://github.com/schester44/deep-object-diff/commit/13f1964c867a7b328b2ac74a2b2906a0495e2a82) from the PR. It's usually clearest for everyone when the maintainer does those, just prior to publishing that version.
What are your thoughts on the Object.keys
function? It being a native one liner, i'm not seeing the benefit but definitely not opposed to going that route if thats something we want to do.
Benchmarking was done using this package.
version bump was a mistake, i'll fix that.
I'll go ahead and continue with this and create PR soon.
Closing as I've merged #64 and published in v1.1.7
There could be noticeable perf improvements if we were to stop using the reduce spread pattern in the diffing methods.
Some benchmarks from changing the
diff
method to use basic for loops:original x 377,694 ops/sec ±0.62% (86 runs sampled) new x 788,699 ops/sec ±0.55% (91 runs sampled)
Reference: https://www.richsnapp.com/blog/2019/06-09-reduce-spread-anti-pattern
I have most of the work done in a fork. Would you be open to a PR?