amd-utils

amd-utils copied to clipboard

var Foo = function () {}; Foo.prototype.bar = function () {}; var someFoo = new Foo();

{
  xpto: someFoo
}

If I clone this object, the xpto value will be an empty object. The bar method won't exist and foo instanceof someFoo would be false.

I think that the clone function should account for this. One possible solution is to analyse the object like this:

obj.constructor !== Object // means it's an instance

// If so, clone it like this
newObj = createObject(obj)

@millermedeiros what you think about this?

Yes, clone implementation is naive, no support for custom types and it also doesnt handle circular references. Open for suggestions/improvements but I feel lang/clone should only handle natives/primitives. Maybe it should pass non-natives by reference and create a new method inside the Object package that handles custom types...

@millermedeiros agree. The solution I presented above work on instances but it's not optimal. Could the clone function accept a second argument, that if present, would be called for custom types?

If not present, the clone would behave just like it is now.

Underscore.JS _.clone function does copy properties on the prototype, but does not actually use the original prototype. I don't think that this is necessarily the best behavior, but some people might expect it to have the same behavior as Underscore. I think that the current behavior (copying only the object's own properties) is preferable, since with Underscore's behavior someone could think that it does have the original prototype when it actually does not.

Underscore behavior:

function F() {}
F.prototype.foo = true;
_.clone(new F()) instanceof F // = false
_.clone(new F()).foo // = true
_.clone(new F()).__proto__ === F.prototype // = false (uses non-standard __proto__)

Two possible solutions:

clone(some, true|false);  // if true, would clone instances as well (defaults to false)
clone(some, function (customType) {
  if (isObject(customType) && customType.constructor !== Object) {
    return createObject(customType):
  }
}):

The first option is more restrictive but it's easier for users to understand. The second option is more flexible. If the user passed a function as the second argument, it would be called for custom types. The function would just be responsible for cloning custom types. If nothing is returned from the function then the clone function would handle the custom type just like it's handling now. But then, the question arrises, what is considered a "custom" type?

@conradz Indeed. I don't like underscore behavior. I easily spotted that amd-utils was not cloning instances like I expected because of the missing properties. With underscore, it would cause deep bugs in places where instanceof is used.

was thinking about it and I think we should follow underscore.js behavior by default (use object/forIn instead of object/forOwn during iteration) and add the boolean to toggle the behavior - if true it would use createObject to clone instances.

I guess most people don't use instanceof (I can't remember when was the last time I used it on any application code), what matters in most cases is if the objects have all the properties we expect.

I will implement it as a separate branch right now and wait for feedback before merging.