drop clone module in favor of JSON.parse & JSON.stringify combo for better performance - #3
Conversation
… for better performance
|
Random thought: If headers is just a collection of strings, using Object.assign() could be even faster. |
|
@FredKSchott correct, that's ES6 though .. this library probably needs to have backward compatibility with early node versions, and doing polyfills goes back to performance issues. |
|
Ah good point, I didn't even think about how the polyfill would behave, performance wise. |
|
@ahmadnassri 👍 from me, I even added two more modules to your benchmarks here ahmadnassri/benchmark-node-clone#1 |
|
Couldn't we make it so that we still keep the var clone = function(obj) {
return JSON.parse(JSON.stringify(obj));
};Just in case we in the future would want to swap it for Object.assign or something. Minor thing, of course. |
|
@eiriksm I couldn't resist creating a wrapper: https://github.com/ahmadnassri/stringify-clone more for code cleanliness really ... I don't like seeing |
|
Looks good except you put the wrapper as devDependency when it really is dependency |
|
@eiriksm ah, thanks for catching that! |
|
travis build was failing due to node 0.8 missing dependency in |
|
The tests are still failing on node 0.8 even though the engine in stringify-clone is set to >=0.8, @nylen can we update the support to iojs, 0.12 and 0.10? Also I'm not sure why do we need to test against so many versions of request. |
|
@ahmadnassri we finally fixed the tests but it seems that there is a merge conflict, can you resolve that in your branch? |
|
Merged 👍 |
|
@simov sorry for missing your message! glad you got it fixed & merged. |
|
Yep, np, v0.2.0 is published on NPM. |
benchmarks show
JSON.stringifyto be faster and uses less memory