601379 - Harmony proxies: Object.defineProperty(proxy,name,desc) should pass non-standard attributes of desc to trap

Status: RESOLVED WONTFIX

(Core :: JavaScript Engine, defect, P2)

Description

[Tom Van Cutsem]

As discussed at last TC39 meeting, the Proxies spec. was changed such that Object.defineProperty(proxy,name,desc) would still convert 'desc' into an internal property descriptor first, but also copy any non-standard attributes from 'desc' into the sanitized property descriptor passed to the trap. This allows proxy handlers to access non-standard attributes.

Testcase:

    var result;
    
    var proxy = Proxy.create({
      defineProperty: function(name, descObj) {
        result = (descObj.non_standard === 1);
      }
    });
    
    var desc = { value: 42, writable: true, non_standard: 1 };
    Object.defineProperty(proxy, 'foo', desc);

result is false and should be true.

See the modifications to the semantics of Object.defineProperty on the harmony:proxies semantics page:
http://wiki.ecmascript.org/doku.php?id=harmony:proxies_semantics#modifications_to_other_object_built-ins

Comment 1

[Josh Matthews [:jdm]]

I wrote up a nice easy patch for this before I discovered that js::PropDesc != PropertyDescriptor, and suddenly I'm at a loss as to how this can work without changing public APIs, since I can't find any easy way to pass |v| from JSProxy::defineProperty down to the proxy handler that calls MakePropertyDescriptorObject.  Any input, gal/brendan?

Comment 2

[Andreas Gal :gal]

You can simply convert them. Though, if you want to fix the public API and reduce the number of PropertyDescriptor structs we have, that would be awesome.

Comment 3

[Josh Matthews [:jdm]]

When you say convert them, do you mean change the JSProxy API to take PropDesc instead?

Comment 4

[David Bruant]

Not solved by direct proxies. Updated test case:

var result;
var proxy = new Proxy({}, {
    defineProperty: function(target, name, descObj) {
        console.log('trap')
        result = (descObj.non_standard === 1);
    }
});
    
var desc = { value: 42, writable: true, non_standard: 1 };
Object.defineProperty(proxy, 'foo', desc);

console.log('result', result);
=====
Should log true, but logs false.
Changing dependency order since direct proxies need this bug to be fixed to be considered conformant

Comment 5

[Till Schneidereit [:till]]

Setting ni? to make sure efaust is aware of this.

Comment 6

[David Bruant]

Beware, it looks like the current consensus is to *not* let custom attributes pass
https://mail.mozilla.org/pipermail/es-discuss/2014-May/037124.html

Pending a TC39 meeting, I'd guess.

Comment 7

[Eric Faust [:efaust]]

This has indeed been removed from the spec. See bug 1031243 for more specific details.