lang needs something that Just Works for simple value copying

[kfranqueiro]

I know we are already mulling over how to split up lang.copy to simplify its API and also the mental overhead regarding the combinations it supports. On top of that, one thing it doesn't seem to support is simply copying values, a la lang.mixin in Dojo 1.

Here's an interesting (though probably rather edgy) case I just ran into (trying to use it during development of dom/geometry, dojo/dom#8) that affects various browsers differently, presumably due to how they treat native ClientRect objects:

1. Get a native ClientRect object via someElement.getBoundingClientRect()
2. Attempt to coerce to a plain ol' object by running through copy

Results:

• On Chrome this happens to Just Work as expected - you get an object with the expected properties with numeric values.
• On Firefox (tested 38) and IE (tested 9 and 11), you end up with seemingly an empty object.

Tangentially, I would definitely vouch for not burying sources in a kwArgs object if we can help it, and accommodating one source without needing an array. Having to do that in this case seems rather obtuse. (I realize that other derivative APIs like create and duplicate wrap this complexity away.)

[kitsonk]

I just took a look at it, maybe should have before... That isn't really a usable API in my opinion.

To do the most basic with it, you have to do this:

let copy = lang.copy({ sources: [ orig ] });

Wow!

I mean of course I could have done this, in most cases:

let copy = Object.create(orig);

Some of the functionality in the kwArgs might be better split out as seperete API functions:

export function copy(...sources: any[]): any;
export function copyDeep(...sources: any[]): any;

I also don't understand the use case where we wouldn't want to copy descriptors. I could see there maybe variations in how we determine what keys to copy over, but why would we not copy the descriptor?

[kitsonk]

Looks like @kfranqueiro specific use case in this is interesting... The challenge is the a ClientRec instances has no own properties. All the properties are in the prototype and they are all "odd".

Looking at lang.copy though, if you did lang.copy({ sources: [ clientRec ] }); you should have ended up with an empty object in all three browsers. In lang.copy({ inherited: true, sources: [ clientRec ] }); you would have ended up with a generic object that would have had all the properties and if you did lang.copy({ descriptors: true, inherited: true, sources: [ clientRec ] }); you would have a generic object where when trying to access the properties, you would have gotten exceptions or error messages, because the descriptors of the host objects don't work when copied. Also, Object.create(clientRec) produces unusable objects in Chrome and Firefox.

[kitsonk]

Based on some further conversations offline between @bryanforbes and @kfranqueiro I am not seeing a ClientRec instance with own keys in Chrome (43) (nor Firefox). Here is a JSFiddle I have been playing with.

[mwistrand]

First off, lang.duplicate removes the additional overhead when copying a single object. But, as @kitsonk pointed out, accessing the properties currently doesn't work (I'm working on a fix).

That said, there are admittedly some things here that are confusing. For example, the current package also has an Object.assign shim (dojo/object#assign) that is essentially the simplified version of lang.copy @kfranqueiro refers to. This question might be somewhat tangential to this discussion, but where does that fit in with the various lang methods? As far as the complexity of lang.copy is concerned, I don't think breaking it up into more dedicated functions is going to be as easy as it seems at first glance. Even if we did remove the option for copying descriptors, we would still need a way of indicating whether inherited properties should be copied (assuming that we don't decide to drop that), and a signature like copy(options: { target?: {}; inherited?:boolean }, ...sources: any[]): any that requires users to pass null as the first argument when no options are needed doesn't seem much more elegant.

Dojo 1's lang.mixin had the benefit of not needing to worry about the additional functionality added in ES5; we may not have that same luxury now, hence the additional bulk.

[kitsonk]

We should align the lang features to the Object.assign in my opinion, and whatever behaviour is the behaviour of ES6 should be the starting point. Having additional functionality, we should build upon Object.assign.

I haven't looked at the shim, yet, but if it is fully compatible with ES6, it is supposed to be handling a lot of the edge cases we are running into.

[bryanforbes]

I believe we should have 4 functions: assign (based on Object.assign which only copyies own properties), deepAssign (like assign, except instead of shallow copies, it does deep copies), copy (or mixin or some other name, which copies all properties), and deepCopy (or deepMixin or some other name; like deepAssign, but works on all properties). To me, these four functions are needed almost immediately out of the box and the rest of the functionality that the current copy provides covers functionality that might be nice to have, but that we may not want to support down the road when we find out it's not as useful as it seemed. What do you think?

[kitsonk]

Sounds like a good idea... What about a clone() or create() like function? Do you see those copied by copy/deepCopy? Like what should the rules be around the .prototype and own properties in these cases?

Also, we should probably be explicit about how accessor descriptors, enumeration, own properties and the prototype are handled in copy. I suspect some of these are "implied" in the above, but I will admit, I don't have them fully delineated in my head at the moment. If I get a moment, I will write down what is knocking in my head.

[mwistrand]

The Dojo 2 Core proposal does include both create and duplicate methods. create is essentially identical to Object.assign(Object.create(somePrototoype), source1, source2, ...sourceN);. With copyDeep added, the duplicate method might be implemented as:

export function duplicate(source: {}): {} {
    const target = Object.create(Object.getPrototypeOf(source));

    return copyDeep(target, source);
}

As for the difference between copy and assign, it is a little more involved than that one copies only own properties and the other copies all properties. Object.assign doesn't actually copy property descriptors, but instead does a simple assignment like Dojo 1's lang.mixin. So if we were keeping true to that, then lang.assign would include only enumerable, own properties, and the resulting property descriptors would be { configurable: true, enumerable: true, value: source[key], writable: true }. lang.copy, on the other hand, would copy all properties (enumerable or otherwise, via Object.getOwnPropertyNames(source)), along with the associated descriptors. The exception here would be accessors, since I don't see how they can be safely copied, in which case the result of source[key] would be used for descriptor.value.