unclechu / node-deep-extend

Recursive extend module
MIT License
202 stars 53 forks source link

Error object is cloned as empty #26

Closed vogdb closed 8 years ago

vogdb commented 8 years ago
var deepExtend = require('deep-extend');

var a = {exceptionNormal: new Error('normal')};
deepExtend(a, {exceptionBroken: new Error('broken')});

console.log(a);

gives

{ exceptionNormal: 
   Error: normal
       at Object.<anonymous> (....)
  exceptionBroken: {} }
vogdb commented 8 years ago

@unclechu please review

unclechu commented 8 years ago

@vogdb I'm not sure about just copying pointer to error, I guess we should clone it somehow, like we do it with other objects (Data, Buffer, RegExp), am I right?

unclechu commented 8 years ago

I guess it's time to new version with use-cases customization, where it's possible to choose some strategies to extend specific objects.

unclechu commented 8 years ago

@vogdb Anyway, if we want it right now (about exceptions) then we need to think how to clone exception.

vogdb commented 8 years ago

Ah, ok. I didn't think it carefully. Just made it in a workflow rush. Let me think about cloning exception.

vogdb commented 8 years ago

@unclechu do you have thoughts about cloning stacks, e.g. error.stack? I'm out of ideas, need to sleep.

vogdb commented 8 years ago

please look btw

unclechu commented 8 years ago

@vogdb Also I think that cloning exceptions making less sense, because it will be new object, that can't be checked easily by instanceof operator for example. This way of cloning doesn't consider if exception constructor was inherited from Error and then another constructor inherited from that one, and now we trying to clone exception just ignoring this levels of inheritance. Cloning objects with exceptions by my opinion is very custom and specific case, and I guess deep-extend should have customizable API for that cases, but it would be only in next major release, I see no way to implement this without brake backward compatibility.

vogdb commented 8 years ago

Ok. Good to know. Closing this for now.