Private fields incompatible with JS Proxy

[amilligan]

For example:

class Proxied {
  #data;

  constructor() {
    this.#data = {
      foo: 'foo',
      bar: 'bar',
    };

    return new Proxy(this, {
      get(target, prop, receiver) {
        if (prop in target) {
          return Reflect.get(target, prop, receiver);
        } else {
          return target.data[prop];
        }
      },
    });
  }

  get data() {
    return this.#data;
  }

  get wibble() {
    return 'wibble';
  }
}

const proxied = new Proxied();
console.log('========> ', proxied.wibble); // no private member access
console.log('========> ', proxied.foo); // private member access via proxy
console.log('========> ', proxied.data.foo); // private member access via direct call

This should print out three lines, thusly:

========> wibble
========> foo
========> foo

The first and second calls work fine, but the third fails with the error

Uncaught TypeError: Cannot read from private field

I fiddled with this a fair bit, and I found that in the call to __privateGet the obj parameter is the proxy, but the member parameter (the WeakMap) contains the proxy's target. The two are not the same object, so the __accessCheck method fails.

On further investigation, I see the WeakMap is instantiated with this in the class constructor, when this is the target object and not the proxy returned from the constructor. Later, when accessing the private attribute, __privateGet is called with this, which is now the proxy.

So, I suppose it's not so much the proxy, as any JavaScript constructor that returns an object other than this, which is a common enough mechanism in JavaScript I'm surprised this hasn't been reported before. I searched the issues for a similar report, but didn't find anything equivalent.

[jridgewell]

esbuild's behavior here is correct, and if you run the original code in Chrome today you'll see the same error. See tc39/proposal-class-fields#106 (and there are a lot of similar issue threads) for context.

[amilligan]

@jridgewell I see how that conversation appears similar, but I think this comment correctly points out that the issue is different from what I have brought up here. In the linked issue the problem is that the proxy, when called, cannot access a private member. That's fine, and I agree it's correct. The proxy in the example I gave, when called, never attempts to reference a private member; it either references the property from the target (via the correct use of Reflect, as mentioned in the linked comment, and the example shows that this works fine), or it references a public member of the target.

The fundamental problem is not that the proxy is attempting to reference a private member; the data property is public. The problem occurs when the data getter, internally, attempts to reference the #data private member on this. I think we can agree an object should be able to reference its own private members, no matter how the method is invoked.

[jridgewell]

attempts to reference the #data private member on this

That's the issue. The this receiver isn't a Proxied instance, it's a Proxy instance wrapping a Proxied instance. The committee decided that Proxy is not meant to be transparent wrapper around all functionality, and that invoking getter/setter/methods need to receive the proxy's target to be able to operate on private fields.

-          return Reflect.get(target, prop, receiver);
+          return Reflect.get(target, prop);

Either way, this is not a bug with esbuild, this is just the way private fields works.

[amilligan]

The line that you referenced is not the line that is failing.

The line that is failing is

          return target.data[prop];

On that line, target is a Proxied instance. In the failing method call, this is a Proxied instance. The Proxy has done its job and is no longer involved.

[amilligan]

Okay, I see the mistake I made; passing the receiver through to the call to Reflect.get set this to the original receiver (the proxy) rather than the target. I assume (although I didn't realize this) that leaving the receiver off defaults this to the first parameter of Reflect.get.

So, with that change the original example works. But, if you add a method call that references a private member, it fails again. E.g. (in Proxied):

  do() {
    return this.#data.foo;
  }
...

console.log('========> ', proxied.data.do());

In this case, without the explicit receiver in Reflect.get, the value of this in the method call should be the instance of Proxied and all should work. I admit I'm confused why the getter works and the method call does not.

I suspect it's because the function object is returned via the Reflect.get call and then this reverts to the proxy before the function is invoked. I admit this seems to severely limit the utility of Proxy. I also find it odd that this works fine in node and when built with webpack/babel. I'm not suggesting esbuild should prefer existing behavior over the actual standard; it's just a frustrating return to the compiler vendor wars I had hoped never to revist.

[jridgewell]

You can use the super-return override trick so that private fields are installed on an instance of a Proxy, instead of the target the proxy wraps:

class Data {
  constructor(data) {
    // This is a super-return override.
    return new Proxy(this, {
      get(target, prop, receiver) {
        if (prop in target) {
          return Reflect.get(target, prop, receiver);
        } else {
          return data[prop];
        }
      },
    });
  }
}

class Proxied extends Data {
  #data;

  constructor() {
    // This will hold our data if we use an assignment.
    const data = {
      foo: 'foo',
      bar: 'bar',
    };

    super(data);

    // #data was installed on the Proxy instance, not the unwrapped target!
    this.#data = data;
  }

  get data() {
    return this.#data;
  }

  get wibble() {
    return 'wibble';
  }
}

const proxied = new Proxied();
console.log('========> ', proxied.wibble); // no private member access
console.log('========> ', proxied.foo); // private member access via proxy
console.log('========> ', proxied.data.foo); // private member access via direct call

---

I suspect it's because the function object is returned via the Reflect.get call and then this reverts to the proxy before the function is invoked.

Correct! You need another proxy instance, and another proxy hook (apply) that unwraps the Proxied instance out of the wrapped thisArg to properly handle methods. Proxies are unfortunately very difficult to use as wrappers now.

I also find it odd that this works fine in node and when built with webpack/babel

You're likely using loose mode for private fields transformation, instead of the spec compliant transform.

[evanw]

The private class transform used by esbuild is essentially the same thing that TypeScript and Babel do, which both came before esbuild and which the community already relies on for this. So even if there is a subtle specification issue with esbuild and we can fix it to do what you want, writing code like this is still likely going to be brittle and non-portable until those other projects are also fixed and the fixes have been out for a while.

First of all, the code you posted in the original comment appears to be broken. I tried it in various JS environments and everyone throws some kind of TypeError, so esbuild's behavior seems correct:

• Firefox: Uncaught TypeError: can't access private field or method: object is not the right class
• Safari: TypeError: Cannot access invalid private field (evaluating 'this.#data')
• Chrome: Uncaught TypeError: Cannot read private member #data from an object whose class did not declare it
• TypeScript: Uncaught TypeError: Cannot read private member from an object whose class did not declare it
• Babel: Uncaught TypeError: attempted to get private field on non-instance
• esbuild: Uncaught TypeError: Cannot read from private field

What's the proposed change to esbuild? Sorry the above thread is long and detailed and I scanned it but I couldn't find a description of what esbuild is supposed to do differently. Or is this thread just saying that esbuild's behavior is correct and then discussing generally how to use private fields with proxies in JavaScript? I'd like to close this issue if it's not a bug with esbuild.

[jridgewell]

Or is this thread just saying that esbuild's behavior is correct and then discussing generally how to use private fields with proxies in JavaScript?

This is the correct takeaway. esbuild is correct according to the spec (in every way that I've tested), and the rest was explanation of why and how to avoid the issue with proxies.

[evanw]

Ok, thanks! I appreciate your comments by the way. Closing this issue as "working as intended" in that case. Feel free to continue discussing with additional comments here if you like.

[bathos]

You can use the super-return override trick so that private fields are installed on an instance of a Proxy, instead of the target the proxy wraps

At the moment, there is a bug in V8 which leads to incorrect evaluation of #foo in bar where bar is a proxy exotic object that actually does have #foo allocated using super return override. Just mentioning this as a small heads up because of the potential for this to cause further confusion; I'm guessing it will be fixed soon, though.

[amilligan]

I appreciate that this issue is closed, as it should be, and apologies to @evanw for the tempest in a teapot, but I feel that while all of the conclusions reached here are correct they are not fundamentally helpful. That's the fault of the standards committee, not the participants in this conversation. There may be good reasons for implementing Proxy as it is, but that doesn't change the fact that it has limited utility. As stated,

Proxies are unfortunately very difficult to use as wrappers now.

Perhaps Proxy isn't the appropriate solution for wrappers, but it was a solution, and JS doesn't seem to provide any others. The ruby language provides #method_missing, and it has been used to fantastic effect; it would be nice to be able to do similarly in JS.

Also, I get the mechanism of the super-return override given above, and I appreciate the suggestion. I'll definitely play around with that. But, I feel it must be said that forcing a solution that uses inheritance for a fundamentally un-inheritance problem seems like a major step backwards.

That's enough rant, before this spirals into an argument about multiple inheritance or similar. Thank you @jridgewell for your suggestions.

[bathos]

@amilligan It is awkward. I think there’s reason to be hopeful that this can change, though — the current situation doesn’t need to be the end of the story for private names.

The sense I got from following past discussions was that the decision to introduce private fields (and methods/accessors) with their syntactic availablity being inextricable from class bodies (the only possible lexical scope) and their allocation being inextricable from [[Construct]]/SuperCall was a matter of initially delivering the essential functionality in some form that could be agreed upon which was believed likely to serve a large number of developers effectively. And it probably has done a pretty good job of that so far. It was a big set of additions all at once, so it makes sense that a line needed to be drawn somewhere, but there can still be new proposals that improve it.

@jridgewell’s Private declarations could address the first constraint (lexical scope being limited to class bodies).
@littledan’s new.initialize could address the second (allocation being limited to cases where return overrides are either not involved or only occur ancestrally through — and only through — SuperCall).

In the mean time, there are a variety of patterns that can make the super-return requirement for proxies a lot less awkward in at least some cases. A unique heritage for each class isn’t usually needed because the responsibilities of a SuperCall delegatee can be generalized. The most basic case is just function IdentityConstructor(o) { return o; }, a totally generic object bouncer so that the actual class is left fully responsible for allocation. But usually, you can still let the heritage be responsible for allocation and reduce boilerplate in the process. Below, “Proximator” lets the target instance get allocated as usual and accepts the handler as its argument. Can even still have %Object.prototype% be the instance’s prototype’s prototype as normal for a base class.

// GENERIC HELPER

function Proximator(handler) {
  return new Proxy(this, handler);
}

Proximator.prototype = Object.prototype;

// SPECIFIC USAGE

class Proxied extends Proximator {
  #data = { foo: "foo", bar: "bar" };

  constructor() {
    super({ get: (t, k, r) => k in t ? Reflect.get(t, k, r) : this.#data[k] });
  }

  get data() {
    return this.#data;
  }

  get wibble() {
    return "wibble";
  }
}

let proxied = new Proxied;

console.log('========> ', proxied.wibble);
console.log('========> ', proxied.foo);
console.log('========> ', proxied.data.foo);
//========>  wibble
//========>  foo
//========>  foo

(Also: handlers are stateful objects; you might even wanna implement them as classes. You can still do handler.proxy = this; after super(), it can have knowledge of both the target and the proxy even in methods without a receiver arg. There’s no guarantee that receiver is the proxy — it just usually is — so that is prob what you really wanted in the initial example).

This next one is not actually a recommendation, but Proximator could be realized instead as just Proxy if you add a prototype property to it and pass the target:

// NOT GOOD PATTERN, JUST ILLUSTRATING
Proxy.prototype = Object.prototype;

class Foo extends Proxy {
  #bar = 7;
  constructor() { super(Object.create(new.target.prototype), { has: () => true }); }
  get bar() { return this.#bar; }
}

console.log('========> ', "anything" in new Foo);
console.log('========> ', new Foo().bar);
//========>  true
//========>  7

This example might also provide some ideas about what is already possible:

class Mixin extends function (o) { return o; } {
  static allocate = o => new this(o);
  static getFoo = o => o.#foo;
  #foo = 10;
}

let date = Mixin.allocate(new Date);

console.log('========> ', date);
console.log('========> ', Mixin.getFoo(date));
//========>  Fri Jan 28 2022 21:21:17 GMT-0500 (Eastern Standard Time)
//========>  10

Again, I agree with your sense that something’s still off here — just want to share some ideas that might help you get where you want to anyway.

Oh, one last recipe that may be useful: because Get and Set realize the behavior of accessors (by supplying receiver), if that’s the only behavior needed, it’s obtainable with only a single proxy across all instances (just like how you’d only need a single ancestral accessor across all instances): setPrototypeOf(myclass.prototype, justOneProxy).

Hope something here was maybe helpful — and that those two proposals gain traction.