Discuss
  • Reflect.defineProperty + FromPropertyDescriptor & ToPropertyDescriptor

Reflect.defineProperty + FromPropertyDescriptor & ToPropertyDescriptor

# Darien Valentine (6 years ago)

The following instrinsic functions invoke the ToPropertyDescriptor internal operation to convert JS-object property descriptors into "Property Descriptors" (spec-internal object type):

  • Reflect.defineProperty
  • Object.create
  • Object.defineProperty
  • Object.defineProperties
  • a Proxy instance if its handler includes getOwnPropertyDescriptor

The ToPropertyDescriptor operation uses HasProperty to check for the presence of the following properties of a JS object property descriptor in the order enumerable, configurable, value, writable, get, set.

If any two incompatible properties are found to be “had properties,” an error is thrown.

It seems unfortunate that this works:

function breakEveryPropertyDescriptor() {
  Object.prototype.value = undefined;
  Object.prototype.get = undefined;
}

But this has been mentioned before. I found this prior discussion:

esdiscuss.org/topic/topropertydescriptor-hasproperty-hasownproperty

As explained there, one can address this, if sufficiently paranoid, by only ever passing in null-prototype objects (or objects with a prototype you control) as JS-object property descriptors.

It gets a bit more complex. FromPropertyDescriptor produces JS-object PDs which also inherit from %ObjectPrototype%. Naturally you can perform the own-property check yourself — but it has a consequence I find particularly interesting:

const obj1 = {};
const obj2 = new Proxy({}, Reflect);

breakEveryPropertyDescriptor();

const pd = Object.assign(Object.create(null), { value: 1 });

try {
  Reflect.defineProperty(obj1, 'foo', pd);
  console.log('successful on obj1');
} catch {
  console.log('unsuccessful on obj1');
}

try {
  Reflect.defineProperty(obj2, 'foo', pd);
  console.log('successful on obj2');
} catch {
  console.log('unsuccessful on obj2');
}

In the Proxy case, the input JS PD object is converted to an internal Property Descriptor ({ value: 1 }) and then back to a fresh JS object (now with %ObjectPrototype%) — and then back to an internal Property Descriptor again. But it doesn’t successfully round trip! At this point the result of HasProperty(Obj, "get") changes from what it would have been if the PD had not passed through Reflect.defineProperty a second time, and the descriptor has become { value: 1, get: undefined }.

My understanding was that using Reflect as your handler object should mean the behaviors of all the trapped operations will be the same as they would have been for an ordinary object. This may be an incorrect assumption on my part (are there other examples like this?), but it does seem desirable for that to be true.

I’m curious about whether changes to these algorithms to alter this behavior in some way would be web-safe. The obvious solution would be switching HasProperty to HasOwnProperty in ToPropertyDescriptor and/or creating objects that inherit from null in FromPropertyDescriptor. But I’m guessing those changes would not be safe. In any case, I wanted to share the non-parity edge case with Reflect.defineProperty since maybe this is not already a known quirk.

# Darien Valentine (6 years ago)

On reflection (harrrr) I realize that there is an almost certainly safe change which can be made to address this in the Reflect.defineProperty case, which is to make Reflect.defineProperty alone return an object with a null prototype. I believe this would make it behave the same as ordinary [[defineOwnProperty]] in all cases.

# Isiah Meadows (6 years ago)

Personally, I'd prefer it just be handled this way:

  • If either desc.get or desc.set is not undefined, treat it as a data descriptor.
  • Otherwise, treat it as a value descriptor and ignore desc.get and desc.set.

It'd be much easier to reason about, and I've personally had a few issues around this area - it's annoying to have to build objects polymorphically for the only area where [[HasProperty]] actually has any major semantic meaning and the object in question isn't a key/value dictionary (like for Object.defineProperties). Most everything else is just based on [[Get]], so it's just odd and annoying.

Isiah Meadows contact at isiahmeadows.com, www.isiahmeadows.com

# Darien Valentine (6 years ago)

@Isiah:

That seems like it would likely be a safe change since it would only make currently invalid PDs become valid. However I’m unsure if it’s sufficiently unambiguous as stated on account of PDs which set only metadata without also overwriting an existing get/set/value. For example, what would be the effect of the PD { get: undefined, set: undefined, enumerable: false } on an existing accessor property { configurable: true, get: fn, enumerable: true }? Does it say to set the existing property to be unenumerable, like { enumerable: false } does presently? Or does it ask to redefine it as a data property whose value is undefined?.

Also, on its own, such a change would not address the Reflect.defineProperty non-parity quirk.

# Allen Wirfs-Brock (6 years ago)

On Sep 4, 2018, at 12:32 AM, Darien Valentine <valentinium at gmail.com <mailto:valentinium at gmail.com>> wrote:

My understanding was that, in theory, using Reflect as your handler object should mean the behavior of all the trapped operations should be the same as it would have been for an ordinary object. This may be an incorrect assumption on my part (are there other examples like this?), but it does seem desirable for that to be true.

Yes, this is surprising (at least to me) and slightly disturbing.

At one point, early in the development of ES6 the draft spec. had a [[OriginalDescriptor]] field (that may not be the actual name I used) in internal PropertyDescrptors that carried along a reference to the original descriptor object from which the PropertyDescriptor was derived. When the Proxy traps needed to reify a PropertyDescriptor as an object it would use the [[OriginalDescriptor]] if it had a value. The reason for having [[OriginalDescriptor]] wasn’t because of inheriting from %ObjectPrototype%. It was to enable Proxies to extend property descriptor objects with additional attributes (for example, imagine a public: <boolean> attribute) that the Proxy handler could then interpret.

[[OriginalDescriptor]] was removed from the spec. after some implementors pushed back with cncerns about the added complexity and skepticism about the value of the added utility.

I’m curious about whether changes to these algorithms to alter this behavior in some way would be web-safe.

Unlikely, part of the original intent of the descriptor object design was that prototypal inheritance could be used compose descriptor objects (BTW, there are es-discuss threads about these inheritance issues far older than the one you referenced). consider:

function friendlyDefaults (desc) {return Object.setPrototypeOf(desc, {enumerable: true, configurable: true, writable: true})};

var obj = Object.create(Object.prototype, { p1: friendlyDefaults({value: 1}), p2: friendlyDefaults({value: 2}), p3: friendlyDefaults({value: 3}) });

I won’t want to bet that nobody on the web has ever used prototypal inheritance to construct their property descriptors.

Similarly, I won’t want to bet that nobody on the web has written a Proxy trap handler where they have invoked the hasOwnProperty method on a reified descriptor object.

I think that bringing back the [[OriginalDescriptor]] concept might be web-safe, but good luck on convincing implementors that this problem is worth the effort. (I actually think, the overhead would not be so bad because implementations don’t actually create PropertyDescriptors in normal operation. It is only operations that start with a Object.* or Reflect.* operation that create the round tripping concern.)

# Michael Dyck (6 years ago)

On 2018-09-04 12:02 PM, Allen Wirfs-Brock wrote:

At one point, early in the development of ES6 the draft spec. had a [[OriginalDescriptor]] field  (that may not be the actual name I used) in internal PropertyDescrptors that carried along a reference to the original descriptor object from which the PropertyDescriptor was derived.

It looks like you're talking about [[Origin]].

Working drafts 12 though 25 had this wording or similar: A Property Descriptor may be derived from an object that has properties that directly correspond to the fields of a Property Descriptor. Such a derived Property Descriptor has an additional field named [[Origin]] whose value is the object from which the Property Descriptor was derived.

(In case that helps anyone researching this.)