Discuss
  • Object.assign and __proto__ property

Object.assign and __proto__ property

# teramako (12 years ago)

I believe Object.assign never update the target's [[prototype]]. But current rev19 spec is using Put. So I'm afraid of following case will update the [[prototype]].

var soruce = {};
Object.defineProperty(source, "__proto__", {
    value: { a: "A", b: "B" },
    enumerable: true
});
var target = {};
Object.assign(target, source);

Is this expected result ? and is there a plan to change to DefinePropertyOrThrow from Put ?

# Rick Waldron (12 years ago)

Object.assign is essentially a "batch property assignment with Put"; I believe what you're describing above is Object.mixin, which is "batch property definition with DefinePropertyOrThrow".

# Allen Wirfs-Brock (12 years ago)

Actually, this is a good point. As currently specified Object.assign of with an own __proto__ property on the RHS object will trigger a [[SetPrototypeOf]] on the LHS object. Is that what we want. It is a direct fallout of specifying Object.assign as the equivalent of a sequence of property assignments from the RHS to the LHS. "__proto__" could be special cased. But should it?

Object.mixin doesn't have this issue because, as Rick points out, it uses GetOwnProperty/DefineProperty instead of Get/Set.

Anybody want to argue that Object.assign shouldn't trigger [[SetPrototypeOf]]?

# Andrea Giammarchi (12 years ago)

here again dunder keyword ^_^

As it is Object.assign does not bring any benefit over


for(var key in source) {
  target[key] = source[key];
}

except former pattern can easily filter undesired properties such


for(var key in source) {
  if (key != '__proto__') {
    target[key] = source[key];
  }
}

which is the new hasOwnProperty for ES5 era ...

I am not saying Object.assign should skip such property, 'cause when we deal with dictionaries everything should be considered valid, but I miss any advantage on using such method if not a false feeling that's gonna be safer 'cause implemented in core.

My 2 cents

# Nathan Wall (12 years ago)

Allen Wirfs-Brock wrote:

Anybody want to argue that Object.assign shouldn't trigger [[SetPrototypeOf]]?

Given that __proto__ is defined as an accessor on Object.prototype, I think this should be expected. Any other setter defined on the object or its prototype chain will be invoked in the same manner. It's directly triggering [[SetPrototypeOf]], it's just invoking the __proto__ setter which happens to trigger [[SetPrototypeOf]]. Any number of accessor properties could do weird things:

Object.defineProperty(Object.prototype, 'bloikle', {         set(value) { Object.freeze(this); }     });

// Somewhere else far off in the code...     Object.assign(foo, { bloikle: 'bar' });

// Now foo is strangely frozen...

For consistency, __proto__ shouldn't be hidden from Object.assign.

Plus, there's no reason this shouldn't work:

var info = JSON.stringify(         '{ "proto": "A description of proto",' +         '"hasOwnProperty": "A description of hasOwnProperty" }'     );

var otherInfo = JSON.stringify(         '{ "propertyIsEnumerable": "A description of propertyIsEnumerable" }'     );

Object.assign(otherInfo, info);     console.log(info.proto);

Exploring other options.. A possible alternative consistent option would be to expose some mechanism to hide properties from Object.assign, so that the bloikle property could be hidden in the above... Doing this well would probably require a way to make it work cross-realm, which at first thought sounds pretty difficult. And it probably wouldn't play nice with the above info example.

I think I agree with Andrea on this.  Should Object.assign be left out of ES6 for libraries to implement? ... especially given that this __proto__ dilemma feels "messy".

# Rick Waldron (12 years ago)

On Mon, Oct 21, 2013 at 10:28 PM, Nathan Wall <nathan.wall at live.com> wrote:

For consistency, __proto__ shouldn't be hidden from Object.assign.

Agreed.

I think I agree with Andrea on this. Should Object.assign be left out of ES6 for libraries to implement?

The point of adding Object.assign in ES6 was to pave the wide and heavily trodden cowpaths that exist in libraries. As a built-in, the implementation can be optimized in ways that would not be possible in library code.

... especially given that this __proto__ dilemma feels "messy".

The very last comment seems subjective, and my own subjective response is that it's not "messy".

# Nathan Wall (12 years ago)

In reading Rick's response, I noticed my comments were rather "messy" themselves.  Correcting for clarity.

Given that __proto__ is defined as an accessor on Object.prototype, I think this should be expected. Any other setter defined on the object or its prototype chain will be invoked in the same manner. It's directly triggering [[SetPrototypeOf]], it's just invoking the __proto__ setter which happens to trigger [[SetPrototypeOf]].

Should say "It's not directly triggering [[SetPrototypeOf]]..."

Plus, there's no reason this shouldn't work:

var info = JSON.stringify( 
'{ "__proto__": "A description of __proto__",' + 
'"hasOwnProperty": "A description of hasOwnProperty" }' 
); 

var otherInfo = JSON.stringify( 
'{ "propertyIsEnumerable": "A description of propertyIsEnumerable" }' 
); 

Object.assign(otherInfo, info); 
console.log(info.__proto__);

Should be JSON.parse, of course.

Rick Waldron wrote:

... especially given that this __proto__ dilemma feels "messy".

The very last comment seems subjective, and my own subjective response is that it's not "messy".

Although my message was objectively messy (referring to the mistakes above), you're right that the comment on the __proto__ dilemma is subjective, but it makes some sense.  As Andrea pointed out, it may become common practice (at least among those who care about integrity) to want to leave out __proto__ from assign-like operations, especially when the object to assign from comes from an unknown or (even moreso) an untrusted source -- the same as hasOwnProperty checks in for..in.

Maybe Object.assign should only write the property when the object being written to either doesn't contain it or it's a data property?  Should it really invoke any setters?

Too bad it's too late to bring that __proto__ ship back to the harbor because that's where the real problem lies.

# Andrea Giammarchi (12 years ago)

As sanity/integrity check, Object.assign should be something like this to me ...


for (var property in source) {
  if (Object.prototype.hasOwnProperty.call(source, key)) {
    if (key in target) {
      Object.defineProperty(
        target, key,
        Object.getOwnPropertyDescriptor(
          source, key
        )
      );
    } else {
      target[key] = source[key];
    }
  }
}

If this is what every dev should do in order to have a meaningful version of current Object.assign, I agree with Rick that in core this could/would be properly optimized and made faster.

Best

# Rick Waldron (12 years ago)

On Mon, Oct 21, 2013 at 11:18 PM, Nathan Wall <nathan.wall at live.com> wrote:

Maybe Object.assign should only write the property when the object being written to either doesn't contain it or it's a data property? Should it really invoke any setters?

If this were how it was defined, I would say get rid of it and practitioners will carry on using their own non-nanny implementations. If it can't invoke setters then it loses properties like innerHTML.

eg.

Object.assign(div, {
  innerHTML: `<p>${name}</p>`,
  id: "an-id",
  dataset: {
    foo: 1,
    bar: 2
  }
});

Too bad it's too late to bring that __proto__ ship back to the harbor because that's where the real problem lies.

It may be beneficial, in the long run, to make it painful to use __proto__.

# Rick Waldron (12 years ago)

On Mon, Oct 21, 2013 at 11:26 PM, Andrea Giammarchi < andrea.giammarchi at gmail.com> wrote:

As sanity/integrity check, Object.assign should be something like this to me ...

for (var property in source) {
  if (Object.prototype.hasOwnProperty.call(source, key)) {
    if (key in target) {
      Object.defineProperty(
        target, key,
        Object.getOwnPropertyDescriptor(
          source, key
        )
      );
    } else {
      target[key] = source[key];
    }

This is exactly why there is Object.assign and Object.mixin (originally called `Object.define), because you can't make this semantic decision and assume that it's what all code wants all the time.

  }
}

Traceur has:

function assign(target, source) {
  var props = $getOwnPropertyNames(source);
  var p, length = props.length;
  for (p = 0; p < length; p++) {
    target[props[p]] = source[props[p]];
  }
  return target;
}

Which is correct.

If this is what every dev should do in order to have a meaningful version of current Object.assign,

But it's wrong.

# Andrea Giammarchi (12 years ago)

just one thought on this ...

On Mon, Oct 21, 2013 at 8:34 PM, Rick Waldron <waldron.rick at gmail.com>wrote:

It may be beneficial, in the long run, to make it painful to use __proto__.

proto made IE life painful until now and TC39 decided to leave it in specs ... IE11 had to adopt it to not be left behind 'cause implementors went for it and some library stubbornly decided that IE mobile wasn't worth to be considered.

I wonder if we learn anything from this situation that for the whole "still ES5 compatible" era will have such problem.

But yeah, somebody told me the ship landed already ... you all know the story of the Titanic, right ?

# Nathan Wall (12 years ago)

Rick Waldron wrote:

If this were how it was defined, I would say get rid of it and practitioners will carry on using their own non-nanny implementations. If it can't invoke setters then it loses properties like innerHTML.

eg.

Object.assign(div, { 
innerHTML: `${name}<BR>`, 
id: "an-id", 
dataset: { 
foo: 1, 
bar: 2 
} 
});

I see.  I wasn't clear on the use-case for Object.assign.  You're right; this makes sense.

Given this use for Object.assign, integrity probably isn't a concern for these cases.  A lot of the times, you'll know where the data's coming from, and if you don't you can filter it.  Looks like a nice short-hand (though I'm still somewhat hoping for monocle-mustache in a later edition!)  I was thinking of it too much as a cousin to Object.mixin, which is great for meta-programming and building abstractions.  Object.assign seems more geared toward being a nice little helper function.

# Andreas Rossberg (12 years ago)

On 22 October 2013 04:55, Rick Waldron <waldron.rick at gmail.com> wrote:

The point of adding Object.assign in ES6 was to pave the wide and heavily trodden cowpaths that exist in libraries. As a built-in, the implementation can be optimized in ways that would not be possible in library code.

Frankly, I have a hard time imagining that this would ever be high enough on the priority list for implementors to invest in specifically optimising it, especially since the potential gain does not seem large either.

# André Bargull (12 years ago)

Traceur has:

function assign(target, source) {
   var props = $getOwnPropertyNames(source);
   var p, length = props.length;
   for (p = 0; p < length; p++) {
     target[props[p]] = source[props[p]];
   }
   return target;
}

Which is correct.

Almost correct ;-)

Exception handling and enumerable checks are missing. And calling $getOwnPropertyNames() is not fully compatible when compared to [[OwnKeys]] in some Proxy corner cases 1, but that's difficult to get right in ES5 code.

A more conformant implementation should be this one:

function assign(target, source) {
   function ToObject(o) {
     if (o == null) {
       throw new TypeError();
     }
     return Object(o);
   }
   var to = ToObject(target);
   var from = ToObject(source);
   var keys = $getOwnPropertyNames(from);
   var pendingException = null;
   for (var i = 0, length = keys.length; i < length; ++i) {
     var nextKey = keys[i];
     try {
       var desc = $getOwnPropertyDescriptor(from, nextKey);
       if (desc !== undefined && desc.enumerable) {
         to[nextKey] = from[nextKey];
       }
     } catch (e) {
       if (pendingException !== null) {
         pendingException = e;
       }
     }
   }
   if (pendingException !== null) {
     throw pendingException;
   }
   return to;
}
# Rick Waldron (12 years ago)

On Tue, Oct 22, 2013 at 6:41 AM, André Bargull <andre.bargull at udo.edu>wrote:

Almost correct ;-)

That was copied verbatim: google/traceur-compiler/blob/master/src/runtime/runtime.js#L308

# Erik Arvidsson (12 years ago)

On Tue, Oct 22, 2013 at 9:10 AM, Rick Waldron <waldron.rick at gmail.com> wrote:

That was copied verbatim: google/traceur-compiler/blob/master/src/runtime/runtime.js#L308-L316

I was a bad code reviewer...

google/traceur-compiler#367