Skip to content
This repository has been archived by the owner on Jan 25, 2022. It is now read-only.

Allow Private Fields to Transparently Tunnel through Proxies #102

Closed
Jamesernator opened this issue Aug 3, 2017 · 6 comments
Closed

Allow Private Fields to Transparently Tunnel through Proxies #102

Jamesernator opened this issue Aug 3, 2017 · 6 comments

Comments

@Jamesernator
Copy link

This was mentioned in #1 but I don't really see why it's the case e.g. I'd find it quite surprising if this did not work:

class Point {
    #x
    #y
    constructor(x, y) {
        this.#x = x
        this.#y = y
    }

    add(other) {
        return new Point(this.#x + other.#x, this.#y + other.#y
    }
}

const p = new Point(3, 4)
const p2 = new Proxy(new Point(4, 5), {})

const p3 = p.add(p2) // TypeError
```

If there is a good reason for not forwarding them it'd be nice to see it added to the FAQ.
@bakkot
Copy link
Contributor

bakkot commented Aug 3, 2017

I don't remember if we've talked about this recently.

This is kind of weird either way: internal fields do not normally pass through proxies (so that, e.g. +(new Proxy(new Number(0), {})) throws a type error), and arguably private fields should follow the same model.

Also, Proxies break strict equality, and it would be surprising if, say, two object with different identity could share a private field. But then, proxies are surprising all the time.

This is further complicated by the fact that a superclass can return a Proxy, which the subclass's private fields are then duly installed on: should those be installed on the Proxy object itself, as is currently spec'd, or on the object it is proxying? See snippet:

let o = {};

class Base {
  constructor() { return new Proxy(o, {}); }
}

class Derived extends Base {
  #prop = 0;
  static m(obj) {
    return obj.#prop;
  }
}

Derived.m(o); // does this work?

@littledan
Copy link
Member

Thanks for the summary, @bakkot. To give another example of internal slots not tunneling through Proxies: If you put a Proxy around any object from the Web Platform and call methods on it, the methods will throw a TypeError as the receiver doesn't have the appropriate internal slots.

I don't think tunneling through Proxies is a good design here. Let's stick with the current semantics.

@jods4
Copy link

jods4 commented Jun 13, 2018

This is the most recent comment about proxies I could find, does it still hold? Private properties don't tunnel through proxies?

That can be very disruptive. I know objects that rely on internal slots are broken when proxified, but they are few.

Some frameworks want to use proxies for several usage such dependency (read) detection, detect when fields are added or removed, and more. The kind of stuff that proxies were made for and that works reasonably well on user-defined classes (no internal slot).

Private fields as they are defined here will break. Any class that uses private fields for encapsulation will be unusable/broken with a framework that uses proxies.

Example:

// Returns a proxified object that logs every read to every field, maybe for dependency detection or debugging purposes.
function logReads(target) { 
  return new Proxy(target, { get() { /*..*/ });
}

// Some random business class, using private fields because they're nice
class TodoList {
  #items

  get activeCount() {
    return #items.reduce((n, item) => item.done ? n : n + 1, 0);
  }
}

// Try to use the class while logging reads
let list = logReads(new TodoList);
let count = list.activeCount;  // <-- BOOM! TodoList can't be proxified.

I am fine with private fields not being intercepted by proxies, that's encapsulation after all, but they in my opinion they should tunnel through the proxy.
If they don't proxies and private fields won't mix and that will precludes many uses of proxies.

@ljharb
Copy link
Member

ljharb commented Jun 13, 2018

@jods4 unfortunately they're not "few", they're "almost every JS builtin value" (except for arrays, and plain objects).

@bakkot
Copy link
Contributor

bakkot commented Jun 13, 2018

@jods4:

Yes, this still holds. We discussed it a few times and decided that the WeakMap mental model was what we wanted to settle on, including not forwarding private fields.

But this particular case works: you just have to forward accesses on the proxy to the target, which presumably you were doing anyway. (@erights talks about this somewhere.) This won't forward the private fields, but it will cause the getter which accesses them to be invoked on the right object. For example:

function logReads(target) { 
  return new Proxy(target, {
    get (obj, prop, receiver) {
      console.log(prop);
      return target[prop];
    },
  });
}

// Some random business class, using private fields because they're nice
class TodoList {
  #items = [];

  get activeCount() {
    return this.#items.reduce((n, item) => item.done ? n : n + 1, 0);
  }
}

// Try to use the class while logging reads
let list = logReads(new TodoList);
list.activeCount; // Works fine.

It's a little more complicated proxying methods rather than getters, but it can be done.

@jods4
Copy link

jods4 commented Jun 14, 2018

@ljharb They are "few" compared to the number of classes I write inside my LOB applications.
It is unfortunate, but I can live with Date, Number, Promise and co. not proxify-able.
If none of the classes I wrote work with proxies, that will annoy me like hell.

@bakkot OK, the getter example actually works. It was a bad example, I intended to show that methods don't work:

function logReads(target) { 
  return new Proxy(target, {
    get (obj, prop, receiver) {
      console.log(prop);
      return target[prop];
    },
  });
}

// Some random business class, using private fields because they're nice
class TodoList {
  #items = [];
  threshold = 50;

  countCheap() {
    return this.#items.reduce((n, item) => item.price < this.threshold ? n : n + 1, 0);
  }
}

// Try to use the class while logging reads
let list = logReads(new TodoList);
list.countCheap(); // BOOOM

When you say "it can be done": I don't think it can. Are you thinking of binding functions in the proxy before returning them?

  1. That's a perf hit and tons of unneeded allocations.
  2. It means that this inside the function will actually be this and not the proxy. It is opposite of what we want! In my example above, access to threshold won't be logged by proxy, which is precisely what the proxy is supposed to do.
  3. It breaks several assumptions that caller code might do. For example, it breaks function identity, unless you cache them (more complexity and cost...). Breaking function identity is not OK if they're used with add/removeEventListener, for example.
  4. It unexpectedly binds functions! How do you know the caller is not going to do list.countCheap.call(somethingElse)? This would very unexpectedly break.
  5. It puts a lot of additionnal burden and complexity on the proxy developer. No need to make them worse than they already are + it's bad for their perf.

Proxies already have defects. They don't work with internal slots, they break this identity and hence don't work with WeakMaps. But this is no excuse to make more mistakes.
As is, proxies and private fields won't work together. It's an either-or situation: JS has two features, pick one. It's bad.

Don't create syntactic sugar for WeakMap. Fix it.

All you need to do is spec private names to tunnel through proxies.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants