davidmarkclements / rfdc

Really Fast Deep Clone
MIT License
635 stars 24 forks source link

Add BSON ObjectId support #7

Closed niftylettuce closed 3 months ago

niftylettuce commented 5 years ago

Right now, even with { proto: true }, ObjectId's are cloned inaccurately, resulting in errors like this when one attempts to even console.log them after they have been cloned:

✖  error     TypeError: argument should be a Buffer
    at stringSlice (buffer.js:586:20)
    at Object.Buffer.toString (buffer.js:633:10)
    at Object.inspect (buffer.js:652:14)
    at formatValue (util.js:430:38)
    at formatProperty (util.js:831:11)
    at formatObject (util.js:647:17)
    at formatValue (util.js:609:18)
    at formatProperty (util.js:831:11)
    at formatObject (util.js:647:17)
    at formatValue (util.js:609:18)
    at formatProperty (util.js:831:11)
    at formatArray (util.js:729:17)
    at formatValue (util.js:609:18)
    at inspect (util.js:324:10)
    at format (util.js:253:18)
    at Console.log (console.js:130:21)
niftylettuce commented 5 years ago

https://github.com/williamkapke/bson-objectid

BridgeAR commented 5 years ago

This would require a new option. The proto option allows to copy prototype properties, not the prototype itself. It was meant as a performance improvement (that is mostly not required with the latest implementation).

Adding an option like that might be a good idea but it would likely be significantly slower. I can also imagine that I currently miss some edge cases that could not fully be handled even when cloning the prototype (we should probably also copy properties with the correct property descriptor, if we add such a mode).

alecgibson commented 3 months ago

You can now do this as of v1.4.0:

const clone = rfdc({
  constructorHandlers: [
    [ObjectId, (o) => new ObjectId(o)],
  ],
});