lang.isObject isn't reliable in ES6 environments

[bryanforbes]

In ES6 environments, Symbol.toStringTag allows modifying the return from Object.prototype.toString.call(o):

var o = {};
o[Symbol.toStringTag] = 'Foo';
console.log(Object.prototype.toString.call(o)); // outputs [object Foo]

[kfranqueiro]

Darnit. So you're saying #45's idea has a hole? Is there something we can do instead that doesn't go back to the previous problem of having to think of every thing we want to exclude?

Also, does the fact that lang's isObject is only for internal use for determining whether to recurse when deep copying make this any less critical? (I'm guessing not, since any object that gets passed in could be susceptible to this.)

[kitsonk]

Do we maybe have an ES6 has branch? Something that detects Symbols and
traverses the Prototype chain? I suspect there is a logic that doesn't
waste too many cycles on edge cases.
On Thu, 13 Aug 2015 at 23:42, Kenneth G. Franqueiro <
notifications@github.com> wrote:

Darnit. So you're saying #45 #45
idea has a hole? Is there something we can do instead that doesn't go back
to the previous problem of having to think of every thing we want to
exclude?

Also, does the fact that lang's isObject is only for internal use for
determining whether to recurse when deep copying make this any less
critical? (I'm guessing not, since any object that gets passed in could be
susceptible to this.)

—
Reply to this email directly or view it on GitHub
#62 (comment).

[devpaul]

I ran into a similar curiosity when writing dojo/array. The spec suggested using a wrapper for length = Number(length) to support array-like objects. It ended up forcing incorrect array-like implementations (e.g. length = Infinity) into sensible things (e.g. 0). Values that were already numbers were just passed through.

In a similar manner, if we're targeting ES5+ we should be able to use:

function isObject(obj) {
    return obj === Object(obj);
}

If it's truly an object Object() will pass it through. Otherwise it will wrap it and fail the conditional.

[kfranqueiro]

I do not understand why #63 was merged. AFAICT my last comment there stands (where this completely regresses previous behavior where e.g. Dates and RegExps would not receive this treatment, which was pretty much the entire point of what was previously there).

See #45 for the original context.

[kitsonk]

Apologies... I read the first part of the comment and not the second part:

More importantly, the implementation on this branch currently doesn't filter what we need it to, either (e.g. Date objects would return true, but should not be deep copied).

It is getting a bit silly though that we can't sort patches like this in 5 months. We really need to try harder.

[kfranqueiro]

It is getting a bit silly though that we can't sort patches like this in 5 months. We really need to try harder.

If time were an unlimited resource...

My opinion on the matter at the time was "I'm not really worried about the symbols case, and what's there already works otherwise, so I'd just as soon not land this unless we can come up with something better", and my and others' attention were focused on other things, so I didn't see that particular PR as pressing.

[kitsonk]

If time were an unlimited resource...

Yup, five months is a really constrained resource. Gotcha.

[bryanforbes]

4869ea9 should be backed out. The following code is now producing strange results:

a = { a: /asdf/ };
b = { a: /qwer/ };
lang.deepMixin(a, b);

a.a === b.a // false
a.a // {}

Additionally, I don't think we should worry about the case of someone changing an object's Symbol.toStringTag, so this issue becomes moot and the original code is fine. We should, however, make our tests for deepAssign and deepMixin more robust.