@Isiah:

That seems like it would be likely to 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,
would be the effect of the PD `{ get: undefined, set: undefined,
enumerable: false }` be 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.

On Tue, Sep 4, 2018 at 3:39 AM Isiah Meadows <isiahmeadows at gmail.com> wrote:

> 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
>
> On Tue, Sep 4, 2018 at 3:32 AM Darien Valentine <valentinium at gmail.com>
> wrote:
> >
> > 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 and 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:
> >
> > ```js
> > function breakEveryPropertyDescriptor() {
> >   Object.prototype.value = undefined;
> >   Object.prototype.get = undefined;
> > }
> > ```
> >
> > But this has been mentioned before. I found this prior discussion:
> >
> >
> https://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 the
> result of HasProperty(Obj, "get") changes from what it would have been if
> the PD had not passed through Reflect.defineProperty, and the descriptor
> has become `{ value: 1, get: undefined }`.
> >
> > 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.
> >
> > 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 in case this was not
> already a known quirk.
> > _______________________________________________
> > es-discuss mailing list
> > es-discuss at mozilla.org
> > https://mail.mozilla.org/listinfo/es-discuss
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.mozilla.org/pipermail/es-discuss/attachments/20180904/af5097a2/attachment-0001.html>

@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.

@Isiah:

That seems like it would be likely to 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.