9.5.6 [[DefineOwnProperty]] (P, Desc), step 21.b:
> b. If settingConfigFalse is true, then throw a TypeError exception.
This condition is not correct, instead it needs to be:
> b. If Desc does not have a [[Configurable]] field or settingConfigFalse is true, then throw a TypeError exception.
Test case:
---
Object.defineProperty(Proxy({}, {defineProperty(){return true}}), "p", {})
---
That can't be right either.
Assume that proxy P is maintaining a side table of virtual configurable propertie and that property "p" is already in that table. "p" need not exist as a property of P's target object. In that case, it would be perfect reasonable to say:
Object.defineProperty(P,"p", {value:42}); //or even just { } as the PD.
but I believe that your proposed change would throw in that case.
It seems like the root issue is that simply by looking at the argument Desc and the boolean trap result we don't know anything about what the handler actually did including how it interpreted the absence of various fields. It isn't clear that we can actually check the desired invariants without call [[GetOwnProperty]] on the Prozy to see what it claims to have done.
From http://wiki.ecmascript.org/doku.php?id=harmony:direct_proxies#invariant_enforcement
> on success, if argument descriptor is non-configurable, check if
> the property exists on the target and is also non-configurable
If the argument descriptor does not have a configurable field, is it considered to be a non-configurable or configurable property descriptor?
(In reply to comment #2)
> From
> http://wiki.ecmascript.org/doku.php?id=harmony:direct_proxies#invariant_enforcement
> > on success, if argument descriptor is non-configurable, check if
> > the property exists on the target and is also non-configurable
>
> If the argument descriptor does not have a configurable field, is it considered
> to be a non-configurable or configurable property descriptor?
I think my intent was that an argument descriptor is only considered non-configurable if it explicitly has a configurable:false attribute.
If the configurable: attribute is missing, then for ordinary (non-exotic) objects it depends on whether the property being set already exists or not. If it exists, and configurable is not specified, the attribute remains untouched. If the property being defined is new, it will be configurable:false.
So imagine performing Object.defineProperty(o, "p", {}) on an object you don't know anything about. Then you can't tell at this time whether "p" will be configurable or not. So I think it's fine if this case doesn't throw a TypeError for proxies. IOW I think your test case outlined above should just work.
(In reply to comment #3)
Going back to the original bug bug report, the spec (at least currently) only sets 'settingConfigFalse' to true if the Desc actually has a [[Configurable]] field, so the original test case wouldn't throw.
So, I guess I now don't see a problem in the current spec. language. Does anybody else see one?
A handler might decide to treat a missing Configurable property as meaning non-configurable for a virtual property as long as it didn't also create a corresponding configurable property on the target object. A configurable target property would case the invariant checks for [[GetOwnProperty]] to fail if it tried to claim the virtual property was non-configurable.
>
> So imagine performing Object.defineProperty(o, "p", {}) on an object you don't
> know anything about. Then you can't tell at this time whether "p" will be
> configurable or not. So I think it's fine if this case doesn't throw a
> TypeError for proxies. IOW I think your test case outlined above should just
> work.
Presumably, somebody making a call like this know their intent and would make an explicit Object.hasOwnProperty check if they wanted the result to be dependent upon the actual existence of the property.
A different approach that would still be ES5 compatible would be to move the missing attribute defaulting out of ordinary [[DefineOwnProperty]] and instead do it in Object.definePropery. That way no Object.defineProperty originated [[DefineOwnProperty]] would ever get an partial descriptor for a non-existent property. However, that would probably be undesirable for Proxy handlers that wanted do support virtual properties with new attributes or different defaulting rules.
(In reply to comment #4)
> However, that would probably be undesirable for Proxy handlers that
> wanted do support virtual properties with new attributes or different
> defaulting rules.
Exactly. We didn't discuss different defaulting rules before, but I remember that we aimed for the defineProperty trap to be able to process non-standard attributes on the argument descriptor.
We could still attempt to "complete" the argument descriptor by filling in missing standard attributes, but I think that would be a can of worms (e.g. if it's an empty descriptor, would we turn it into a data or accessor descriptor?)
In any case, I'm not sure anything is wrong with the current spec text either.
André, can you perhaps elaborate on the problem you identified?
(In reply to comment #5)
> André, can you perhaps elaborate on the problem you identified?
I don't recall if there was a specific issue which required to throw in that case or if throwing a TypeError was just the expected result from some Mozilla Proxy test. I suspect it was the latter, but I'd need to inspect the individual test cases to be sure. (Or maybe just I wrongly assumed that Object.defineProperty() with an empty property descriptor works the same for ordinary objects and proxies.)
(In reply to comment #6)
> I suspect it was the latter, but I'd need to inspect the individual
> test cases to be sure.
I tried to reproduce any failures with the rev21 implementation from last November, but to no avail. So from point of you view, this bug report can be closed as invalid...