Proxy deleteProperty has no receiver

[WebReflection]

There is a Chromium bug for this: https://bugs.chromium.org/p/chromium/issues/detail?id=844554

And a codepen live example that shows the issue: https://codepen.io/WebReflection/pen/qYLrNx?editors=0010

Summary

While get and set traps exposes the proxy/receiver, the deleteProperty doesn't and this makes it impossible to use proxies with instances.

const wm = new WeakMap;
class Test {
  constructor() {
    const proxy = new Proxy(this, handler);
    wm.set(proxy, []);
    return proxy;
  }
  method() {
    // the context here is the proxy/receiver, not its target
    wm.get(this).push(`invoked method`);
  }
}

const handler = {
  get(target, key, receiver) {
    wm.get(receiver).push(`get ${key}`);
    return target[key];
  },
  set(target, key, value, receiver) {
    wm.get(receiver).push(`set ${key} as ${value}`);
    target[key] = value;
    return true;
  },
  deleteProperty(target, key) {
    // how am I supposed to log/track the remote property
    // since there is no receiver in here ?
    wm.get({what: '???'}).push(`deleted ${key}`);
    delete target[key];
    return true;
  }
};

Reason

Having a proxy around a generic instance could be used to track remotely instance changes/updates/logs but without a receiver in the deleteProperty one need to add to the WeakMap both the proxy and the instance itself so that wm.get(target) inside the deleteProperty trap would bring in the same Array if the constructor also relate wm.set(this, wm.get(proxy)) (or just using same array).

The fact such workaround is needed suggests the deleteTrap could simply add a third argument and expose the proxy like every other method does.

Thanks for considering this fix.

[ljharb]

cc @erights @tvcutsem

[bathos]

I’ve also run into this, and wished that all trap handlers received a reference to the proxy as an argument.

Since terminology can be confusing with Proxies I would clarify though that receiver is specific to the [[Get]] and [[Set]] internal operations; unlike [[Delete]], whose "receiver" would always be the proxy by definition, the receiver in [[Get]] and [[Set]] is unique in that it may be a remote object that just inherits from proxy (receiver here is the context accessor functions would be called against). If the proxy were added as an argument for the [[Delete]] trap (as well as any other), this would be a novel thing and would break an existing pattern: right now, the arguments for the handlers mirror those of the internal methods being intercepted, which in turn are reflected by Reflect.

[WebReflection]

@bathos thanks for expanding, but the fact there's no way to know which proxy was asked to delete a property for its target is super annoying and inconsistent, IMO.

Also, from user-land perspective, since there is no third argument, this cannot possibly break anything if/once implemented and be also easily feature detected.

I wish we can make proxies better suitable for classes and wrapped instances in the near future, 'cause having to double relate through the weak map both proxy and instance feels plain wrong.

[bathos]

@WebReflection Agreed, I’m also in favor of somehow getting a ref to the proxy in there. I mentioned the stuff about the existing pattern because I suspect some might consider this asymmetry a downside. However if proxy were added as a final arg to all the handler functions, I’d think that remains sufficiently predictable.

[tvcutsem]

First, as @bathos correctly indicated: receiver is special to get/set only due to inheritance. For all other operations, the receiver is basically always the proxy itself. So it does not make sense to add a receiver parameter to just the deleteProperty trap.

However, it is certainly possible to add a reference to the proxy object itself as an argument to every trap (for the get/set traps, receiver may then be equal to the new proxy argument in certain cases).

While adding this extra proxy argument is certainly possible, there are a few drawbacks. First and foremost, one reason we avoided passing in a reference to the proxy in every trap is that it makes it very easy to get into infinite loops (if the handler touches the proxy, it will cause the handler to fire again, which touches the proxy again, etc. etc.)

Second, usually every proxy corresponds one-to-one with its target object, so if you want to keep state associated with the proxy's identity, you can often just as well associate that state with the target. In your original example, you then always do the lookup in the WeakMap based on target, never on receiver. This only becomes a problem when you want to use multiple proxies for the same target, and each proxy needs different private state, but in that case the recommended pattern is to create a separate handler instance per proxy, and to store the state inside the handler object.

Finally, in the example above, it seems this inside method() will be bound to the proxy object. You should think carefully if this is really what you want. Almost all the time you want this to refer to the target object inside the original object's methods. That does mean you need to return a bound method from the get trap.

[WebReflection]

you can often just as well associate that state with the target

you cannot, because as shown/explained methods are triggered through the proxy, as context, and not its target, so for classes this is a bummer.

Finally, in the example above, it seems this inside method() will be bound to the proxy object. You should think carefully if this is really what you want.

Yes, that's exactly what I want, relate the proxy I own and create, and handle every case that goes through it.

Almost all the time you want this to refer to the target object inside the original object's methods.

Well, no. I'd like to preserve original JavaScript behavior when you can simply borrow methods around.

I have a very specific use case that is hacky to implement with current specs and requires unnecessary work arounds, while an extra proxy reference would perfectly, and better, fix all my needs.

Returning a bound method will fail expectations from users of the class, theoretically unaware of the fact the instance is a Proxy, not its target, and the returned method is always bound (including the fact I'd need another reference to the bound method so that instance.method === instance.method).

That does mean you need to return a bound method from the get trap.

This is again a workaround, hence a limitation of the current Proxy specification when it comes to classes.

[WebReflection]

P.S. I'd be more than OK having an extra proxy argument per every single Proxy callback ... it looks like a no brainer, and whoever wants to play harakiri with infinite loops can do that, same way while(true) exists and it's not specifications fault :-)

[WebReflection]

P.S.2 relating the target or binding it to its own methods, also means exposing the target directly in case some method invokes some argument through this which is very undesired if you proxy an instance in the constructor and you want to be sure that outside the class any consumer deals with the proxy only, and not, directly, the instance.

Thanks for thinking this through and considering an improvement over current situation.

edit as proof of concept of current situation and possible side effects on binding methods to the target.

const actions = new WeakMap;
const proxies = new WeakMap;

const handler = {
  get(target, key, receiver) {
    actions.get(receiver)
            .push(`get ${key}`);
    return target[key];
  },
  set(target, key, value, receiver) {
    actions.get(receiver)
            .push(`set ${key} => ${value}`);
    return true;
  },
  deleteProperty(target, key) {
    actions.get(proxies.get(target))
            .push(`delete ${key}`);
    return true;
  }
};

class Trackable {
  constructor() {
    const proxy = new Proxy(this, handler);
    actions.set(proxy, []);
    proxies.set(this, proxy);
    return proxy;
  }
  method() {
    if (actions.has(this))
      actions.get(this).push(`method()`);
  }
}

class Test extends Trackable {
  method(options) {
    super.method();
    // if this was bound, it'd expose the target
    // if this is not bound, everything is fine
    options.invoke.call(this);
  }
}

[bathos]

FWIW @WebReflection I would point out that there are, presently, some alternatives to the double WeakMap solution:

Create the handlers in the constructor

The (potential) disadvantage is the fact that the handler functions are always created anew.

class Foo {
  constructor() {
    const proxy = new Proxy(this, {
      deleteProperty: (target, key) => {
        console.log({ proxy });
        return Reflect.deleteProperty(target, key);
      }
    });

    wm.set(proxy, []);
    return proxy;
  }
}

Make the proxy available to the handlers without creating new functions

This is possible because handlers are called against the handler object. So whatever you make available there is accessible as/via this.

const handlersProto = {
  deleteProperty(target, key) {
    console.log({ proxy: this.proxy });
    return Reflect.deleteProperty(target, key);
  }
};

class Foo {
  constructor() {
    const handlers = Object.create(handlersProto);
    const proxy = handlers.proxy = new Proxy(this, handlers);

    wm.set(proxy, []);
    return proxy;
  }
}

[WebReflection]

@bathos I've edited my previous post to show how articulated could be the current status, but I have to admit the last option you gave me is quite possibly the best workaround.

It still bothers me we are showcasing workarounds for something otherwise straight forward if we had the proxy passed along to proxy handlers we create ourselves, for proxies we own.

Yet I think I'd use that prototypal solution in the future, thanks!

[tvcutsem]

+1 to the second solution. That's how I would encode it as well.

As for passing the proxy along to the handlers: it's not a matter of whether it can be done, but a design issue of trading off better support for this use case (unclear how often the handler needs access to the proxy object, certainly not in most cases) versus increased API complexity and increased risk of bugs (runaway recursion). Of course runaway recursion is always the user's fault, but rest assured giving default access to the proxy object in the handler will significantly increase the number of recursion bugs. In my own experience with proxies, I've made these same mistakes in the get/set trap by doing something as harmless as trying to print out the receiver argument.

cheers

2018-05-22 0:49 GMT+02:00 Andrea Giammarchi notifications@github.com:

@bathos https://github.com/bathos I've edited my previous post to show how articulated could be the current status, but I have to admit the last option you gave me is quite possibly the best workaround.

It still bothers me we are showcasing workarounds for something otherwise straight forward if we had the proxy passed along to proxy handlers we create ourselves, for proxies we own.

Yet I think I'd use that prototypal solution in the future, thanks!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/tc39/ecma262/issues/1198#issuecomment-390805939, or mute the thread https://github.com/notifications/unsubscribe-auth/AAb4OyvnF8nhFi7SaTDUkT-rJwqi_34Kks5t00R9gaJpZM4UGvcK .

[WebReflection]

I think this should be labelled as wont fix then, so that others passing by might as well know what's best as workaround.

const handlersProto = {
  deleteProperty(target, key) {
    console.log({ proxy: this.proxy });
    return Reflect.deleteProperty(target, key);
  }
};

class Foo {
  constructor() {
    const handlers = Object.create(handlersProto);
    const proxy = (handlers.proxy = new Proxy(this, handlers));
    wm.set(proxy, []);
    return proxy;
  }
}

[trusktr]

Yeah, makes sense that receiver arg isn't passed to delete.

But I have this issue with the has trap, and has is related to inheritance. Although the JS engine itself doesn't use a receiver for has, I think that due to it being inheritance-related it should just have a receiver parameter for convenience (f.e. imagine making multiple inheritance, and you want the in operator to work on lookup of keys that branches on a "prototype tree" rather than a regular prototype chain).