Get/Set receivers don't work with inheritance in v8 about harmony-reflect HOT 14 CLOSED

Hmm I'm not seeing this with V8. What version are you using? There was a lot more issues in the V8's that node had during 0.6.x. This code for a similar but different library works correctly:

var proxied = meta.proxy({}, {
  get: function(fwd, target, key, rcvr){
    console.log(key, rcvr === proxied);
    return fwd();
  },
  set: function(fwd, target, key, value, rcvr){
    console.log(key, rcvr === proxied);
    return fwd();
  },
});

proxied.test = {};
// test - true

var child = Object.create(proxied);

child.test;
// test - false

from harmony-reflect.

That is the proxy/weak map implementation in pre 3.8 V8 had issues.

from harmony-reflect.

I've run this test in node 0.7.8 using v8 3.9.24.9, and also in Chrome Canary 21.0.1138.0 using v8 3.10.8.4. Same incorrect results. It looks like the get trap gets the right receiver, but the set trap does not.

from harmony-reflect.

Wow you're right. This explains some of the issues I've had. I never noticed because it only works on the setter.

• err DOESN'T work on the setter

from harmony-reflect.

Proxies & inheritance are a mess in the current Proxy implementations. In ES6, we plan to clean up this mess with an update to the way inheritance is specced. This is a separate proposal from proxies, which impacts the "get", "set" and "has" traps on a Proxy acting as the prototype of another object.

Here's the current state as I can piece it together, for both SpiderMonkey and V8.

SpiderMonkey

In Spidermonkey, when performing a get or set on a child object that inherits from a proxy prototype (as in the above example), if child does not have the corresponding property, the implementation will check whether the property is defined somewhere higher up in the proto chain as if by calling name in proxy. This will trigger the proxy's has() trap.

• If the has trap returns false, the property is assumed to be undefined on the proto chain. child.name would return undefined and child.name = 42 would add a new property to child
• If the has trap returns true, the property is retrieved from the proto chain using the internal [[GetProperty]] function, which in the old proxy API triggered the getPropertyDescriptor trap. The Proxy shim currently implements that trap to always return a fake descriptor that describes an accessor property whose get/set attributes trigger the get/set trap.

@polotek, the reason why you see the tests fail in FF is probably because you didn't add a has trap. Try adding a has trap that always returns true, as in has: function() { return true; }. The tests should then pass.

V8

In V8, the implementation does not call has() on the proxy to test whether the property exists, but instead immediately calls getPropertyDescriptor. This correctly triggers the inherited get/set traps (at least in v8 3.11.0)

However, in the case of set, the value of this wrongly points to the proxy rather than to child. This seems like a genuine bug in v8. I checked whether it also occurs for regular objects (i.e. have child inherit from a normal object that defines a getter/setter), but that seems to work fine.

in-operator

Related to all of this: in ES5, the in-operator is internally specced to use the getPropertyDescriptor trap (see ES5 8.12.6). Because this proxy shim "abuses" the deprecated getPropertyDescriptor trap to always return fake accessor properties, to make inherited get/set traps work, it does break the in-operator as follows:

When performing name in child, the result will always be true. If name exists in child, the result is obviously true. If it doesn't, the implementation will search for name in the prototype chain and call getPropertyDescriptor on the proxy prototype. This trap will return a fake descriptor, which makes the in-operator return true.

summary

So, the current state of things is:

• In spidermonkey, make sure that a proxy used as a prototype defines a has() trap that returns true. This enables the get and set traps for missing properties on children of the proxy.
• In v8, the get trap on a proxy-as-prototype should work fine, but in the set trap the receiver is wrong (this is a bug in v8, not in this shim, and I don't see an easy workaround either).
• In both SM and V8, be aware that the in-operator always returns true on objects that inherit from proxies.

@polotek the MethodSink abstraction only depends on get, not set, so that should work fine in both SM and V8 if you make the MethodSink's has() trap return true unconditionally.

from harmony-reflect.

Oh this will help me figure out some issues I was having trying to make a DOM wrapping membrane that works in V8 but fails in Spidermonkey. The issue there being that xpconnect wrappers provide DOM properties via their prototypes.

On the whole, your first sentence pretty much sums it up. Proxies work more or less but aside from the above mentioned bugs, they are just overall unstable and seem to unexpectedly cause the engines to stop functioning without much in the way of error messages. It's understandable though, since proxies expose things that previously never touched JS-land code or went through previously different paths.

from harmony-reflect.

There's another issue with V8 that I finally boiled down to this simple test case. Basically, the keys and getOwnPropertyNames traps cannot report any properties that exist in Object.prototype. Even if the proxy's prototype is null.

function broken(o){
  return Proxy.create({
    getOwnPropertyNames: function(){
      return Object.getOwnPropertyNames(o);
    },
    keys: function(){
      return Object.keys(o);
    },
  });
}


Object.keys(broken({ toString: function(){} }));
//TypeError: Trap 'undefined' returned repeated property name 'toString'

Object.getOwnPropertyNames(broken({ toString: function(){} }));
//TypeError: Trap 'getOwnPropertyNames' returned repeated property name 'toString'

from harmony-reflect.

@Benvie thanks for reporting. I filed this bug with v8. This seems like something that's easily fixable.

from harmony-reflect.

Looks like both the setter bug and keys error bugs will be fixed in the next V8 release:
https://chromiumcodereview.appspot.com/10451064/
https://chromiumcodereview.appspot.com/10453053/

from harmony-reflect.

The fixes for both issues landed in V8 3.11.8
http://code.google.com/p/v8/source/browse/trunk/ChangeLog?spec=svn11689&r=11689

from harmony-reflect.

Thanks for the heads-up. V8 3.11.9 now correctly passes the tests in test/testProxies.js, except for one:

fail: invoking inherited has on non-existent prop ( assertion on line 641 )

As noted before: on v8, applying the in operator to an object that inherits from a proxy doesn't trigger that proxy's has() trap but instead always returns true. This is due to the fact that the in operator still uses the now deprecated getPropertyDescriptor() trap. I don't see an obvious work-around for now.

from harmony-reflect.

With V8 3.11.10.25, in node.js v0.8.14, the following seems to demonstrate a Direct Proxies bug. Is this a known issue?

"use strict";
var Reflect = require('harmony-reflect');
var target = new Date(1969,10,3,8,0,0,0);
var handler = {};
var proxy = Proxy(target, handler);
console.log('target.getDate = '+target.getDate());
console.log('proxy.getDate = '+proxy.getDate()); // Bug - Fails with "TypeError: this is not a Date object."

from harmony-reflect.

I now see that it is a known issue. It is listed under "Date, RegExp, some Array prototype built-ins fail on proxies". Sorry for posting in the wrong place.

from harmony-reflect.

The in operator now works correctly in v8 when applying it to a child object inheriting from a proxy:

var p = new Proxy({}, {});
var c = Object.create(p);
print('foo' in c); // used to print true, is now false

AFAICT this is the last issue w.r.t. inheritance and proxies, so I'm closing this issue.

from harmony-reflect.