tc39 / proposal-class-fields

Orthogonally-informed combination of public and private fields proposals
https://arai-a.github.io/ecma262-compare/?pr=1668
1.72k stars 113 forks source link

How to dynamically access private properties/classes #257

Open d42ohpaz opened 5 years ago

d42ohpaz commented 5 years ago

In traditional Javascript objects, and public properties of classes, I can dynamically access properties using array notation, but not private properties using similar notation:

class Foo {
  bam = 'boom';
  #bar = 'baz';

  constructor(p1, p2) {
    console.log(p1, this[p1]); // #bar undefined
    console.log(p2, this[p2]); // bam boom
  }
}

new Foo('#bar', 'bam');

Is this supported? If not, what would it take to add support for it?

I have a class that defines certain events that are available to other classes, and I'm using myClass.constructor.name to get the name at runtime to check if a given event name is supported by said class. I'm sure there are other valid use cases for such functionality. Why else would it be supported for public properties?

Thank you.

Edit: thanks to @bakkot for pointing out I had miswrote the examples, so I rewrote them correctly and simplified them for clarity.

ljharb commented 5 years ago

It is not supported.

You can use a static private field name that holds, for example, a Set of supported event names - that’s the approach id recommend for public properties too anyways.

bakkot commented 5 years ago

console.log(foo[myObj[p]]); // undefined

Also, it looks like you're trying to access private properties from outside of the class. The fact that this doesn't work is the whole point of private fields.

d42ohpaz commented 5 years ago

@bakkot thank you; you're right, so I updated my original example to reflect the issue correctly.

littledan commented 5 years ago

I'm still a bit confused by the example. It still looks like you are expecting the private field to be accessible from outside the class. Square bracket property access deliberately excludes private fields to prevent this leak. Could you clarify?

d42ohpaz commented 5 years ago

Maybe that’s part of the issue. I wasn’t aware that square-bracket access was not allowed. In that case, how would I dynamically access private variables?

And if there isn’t currently a way, I would posit that it should be part of the overall discussion as it’s a valid use case.

Thank you.

On Sat, Sep 14, 2019 at 6:58 AM Daniel Ehrenberg notifications@github.com wrote:

I'm still a bit confused by the example. It still looks like you are expecting the private field to be accessible from outside the class. Square bracket property access deliberately excludes private fields to prevent this leak. Could you clarify?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/tc39/proposal-class-fields/issues/257?email_source=notifications&email_token=AATJJT7OO2YBYYNXEK6Y7GLQJS7U7A5CNFSM4IEWJHM2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6WZRHI#issuecomment-531470493, or mute the thread https://github.com/notifications/unsubscribe-auth/AATJJTZOF23EEJFWDFDAEEDQJS7U7ANCNFSM4IEWJHMQ .

-- "Do not go gentle into that good night. Rage, rage against the dying of the light." — Dylan Thomas

rdking commented 5 years ago

It's not currently possible, nor do they have any intention at present to allow it. It's been discussed before, but every time, the advocates reply by suggesting that you should do something like this:

class Foo {
  bam = 'boom';
  #pvt = {
    bar: 'baz'
  };

  constructor(p1, p2) {
    let list = [p1, p2];
    for (let p of list) {
      if ((typeof(p) === "string") && (p[0] === '#')) {
        console.log(p, this.#pvt[p.substring(1)]);
      }
      else {    
        console.log(p, this[p]);
      }
    }
  }
}

new Foo('#bar', 'bam');
d42ohpaz commented 5 years ago

Sigh. Not ideal, but if the powers that be refuse to do anything else, I guess it’ll have to do.

For what it’s worth: that is an unnecessary amount of boilerplate code for a simple concept. It almost feels like the current implementation is either incomplete, or incorrect, since it doesn’t provide all the needed features.

On Sat, Sep 14, 2019 at 2:24 PM Ranando D Washington-King < notifications@github.com> wrote:

It's not currently possible, nor do they have any intention at present to allow it. It's been discussed before, but every time, the advocates reply by suggesting that you should do something like this:

class Foo { bam = 'boom';

pvt = {

bar: 'baz'

};

constructor(p1, p2) { let list = [p1, p2]; for (let p of list) { if ((typeof(p) === "string") && (p[0] === '#')) { console.log(p, this.#pvt[p.substring(1)]); } else { console.log(p, this[p]); } } } } new Foo('#bar', 'bam');

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/tc39/proposal-class-fields/issues/257?email_source=notifications&email_token=AATJJT665AF5ZIX4SS4YIBLQJUT5BA5CNFSM4IEWJHM2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6XBM6I#issuecomment-531502713, or mute the thread https://github.com/notifications/unsubscribe-auth/AATJJT3KJ5IP6NJNJ6RHKH3QJUT5BANCNFSM4IEWJHMQ .

-- "Do not go gentle into that good night. Rage, rage against the dying of the light." — Dylan Thomas

rdking commented 5 years ago

Square bracket property access deliberately excludes private fields to prevent this leak.

Huh? I don't understand that. If this.x === this["x"] then dot access is literally just a subset of square-bracket access. Given the "class owns the name" version of privacy this proposal uses, the only way to leak data would be to leak the private name, but private names are not valid RHS values. What at all does square bracket access have to do with impossible private name leaks?

rdking commented 5 years ago

For what it’s worth: that is an unnecessary amount of boilerplate code for a simple concept. It almost feels like the current implementation is either incomplete, or incorrect, since it doesn’t provide all the needed features.

I and others have been saying that since this proposal was still in stage 2. Apparently TC39 had other heretofore undocumented concerns that outweighed this fact.

bakkot commented 5 years ago

heretofore undocumented concerns

The concerns are documented in the FAQ, and have been for years.

rdking commented 5 years ago

If you were talking about this particularly set of arguments:

Why doesn't this['#x'] access the private field named #x, given that this.#x does?

This would complicate property access semantics.

Dynamic access to private fields is contrary to the notion of 'private'. E.g. this is concerning:

class Dict extends null {
  #data = something_secret;
  add(key, value) {
   this[key] = value;
 }

  get(key) {
    return this[key];
  }
}

(new Dict).get('#data'); // returns something_secret

But doesn't giving this.#x and this['#x'] different semantics break an invariant of current syntax? Not exactly, but it is a concern. this.#x has never previously been legal syntax, so from one point of view there can be no invariant regarding it.

On the other hand, it might be surprising that they differ, and this is a downside of the current proposal.

then, you're forgetting that these arguments had been thoroughly refuted even in stage 2. The fact that you even stated that "it is a concern" means that from the time of that writing, you already knew this was going to be an issue. Further, given that formats like this[#'x'] avoid all the exclusion criteria listed in the FAQ over this issue and were brought up several times between the past 2 proposal stages, certainly there has to be another reason why this was not accepted. Yet nothing in the FAQ even comes close to a reason.

Given that I understand TC39 to be an at least reasonable group, I can only conclude that the reason for not allowing for such a thing is not yet documented. Care to try again?

d42ohpaz commented 5 years ago

If somebody wants to write return this[key]; then that’s on them not the language.

What business is it of the languages to stop them? The language simply enforced specific rules like direct access from outside the instance or class.

Frameworks, in the other hand, are free to be opinionated. Leave it to them to make those decisions.

On Sat, Sep 14, 2019 at 6:53 PM Ranando D Washington-King < notifications@github.com> wrote:

If you were talking about this particularly set of arguments:

Why doesn't this['#x'] access the private field named #x, given that this.#x does?

This would complicate property access semantics.

Dynamic access to private fields is contrary to the notion of 'private'. E.g. this is concerning:

class Dict extends null {

data = something_secret;

add(key, value) { this[key] = value; }

get(key) { return this[key]; } }

(new Dict).get('#data'); // returns something_secret

But doesn't giving this.#x and this['#x'] different semantics break an invariant of current syntax? Not exactly, but it is a concern. this.#x has never previously been legal syntax, so from one point of view there can be no invariant regarding it.

On the other hand, it might be surprising that they differ, and this is a downside of the current proposal.

then, you're forgetting that these arguments had been thoroughly refuted even in stage 2. The fact that you even stated that "it is a concern" means that from the time of that writing, you already knew this was going to be an issue. Further, given that formats like this[#'x'] avoid all the exclusion criteria listed in the FAQ over this issue and were brought up several times between the past 2 proposal stages, certainly there has to be another reason why this was not accepted. Yet nothing in the FAQ even comes close to a reason.

Given that I understand TC39 to be an at least reasonable group, I can only conclude that the reason for not allowing for such a thing is not yet documented. Care to try again?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/tc39/proposal-class-fields/issues/257?email_source=notifications&email_token=AATJJT2DM5V2VSQK43QNVR3QJVTN3A5CNFSM4IEWJHM2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6XFSTY#issuecomment-531519823, or mute the thread https://github.com/notifications/unsubscribe-auth/AATJJT5YGKQMMJMY65UFN4TQJVTN3ANCNFSM4IEWJHMQ .

-- "Do not go gentle into that good night. Rage, rage against the dying of the light." — Dylan Thomas

d42ohpaz commented 5 years ago

To further my point, it is perfectly reasonable to have private properties that are readable by outside the class. It’s not always about hiding the data itself, but keeping the data in a specific state (immutability).

This is similar to the concept of getters without a corresponding setter; except this is being done using private properties.

On Sat, Sep 14, 2019 at 6:53 PM Ranando D Washington-King < notifications@github.com> wrote:

If you were talking about this particularly set of arguments:

Why doesn't this['#x'] access the private field named #x, given that this.#x does?

This would complicate property access semantics.

Dynamic access to private fields is contrary to the notion of 'private'. E.g. this is concerning:

class Dict extends null {

data = something_secret;

add(key, value) { this[key] = value; }

get(key) { return this[key]; } }

(new Dict).get('#data'); // returns something_secret

But doesn't giving this.#x and this['#x'] different semantics break an invariant of current syntax? Not exactly, but it is a concern. this.#x has never previously been legal syntax, so from one point of view there can be no invariant regarding it.

On the other hand, it might be surprising that they differ, and this is a downside of the current proposal.

then, you're forgetting that these arguments had been thoroughly refuted even in stage 2. The fact that you even stated that "it is a concern" means that from the time of that writing, you already knew this was going to be an issue. Further, given that formats like this[#'x'] avoid all the exclusion criteria listed in the FAQ over this issue and were brought up several times between the past 2 proposal stages, certainly there has to be another reason why this was not accepted. Yet nothing in the FAQ even comes close to a reason.

Given that I understand TC39 to be an at least reasonable group, I can only conclude that the reason for not allowing for such a thing is not yet documented. Care to try again?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/tc39/proposal-class-fields/issues/257?email_source=notifications&email_token=AATJJT2DM5V2VSQK43QNVR3QJVTN3A5CNFSM4IEWJHM2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6XFSTY#issuecomment-531519823, or mute the thread https://github.com/notifications/unsubscribe-auth/AATJJT5YGKQMMJMY65UFN4TQJVTN3ANCNFSM4IEWJHMQ .

-- "Do not go gentle into that good night. Rage, rage against the dying of the light." — Dylan Thomas

rdking commented 5 years ago

To further my point, it is perfectly reasonable to have private properties that are readable by outside the class. It’s not always about hiding the data itself, but keeping the data in a specific state (immutability).

That's where I (and probably TC39) will disagree with you. Its not about the state of the data but rather its accessibility. Regardless of the state of the data, controlling the means of accessing the data is what's important.

Where I disagree with TC39 is this: The problem is that the design of this proposal is akin to "helicopter parenting". They're going so far as to ensure as absolutely as possible that a developer cannot accidentally leak private data. It's like saying, since fire can burn you, let's have you put on this asbestos suit while you carry this unlit book of matches. It's not the language's fault if a developer is sloppy and leaks data.

d42ohpaz commented 5 years ago

The problem with “helicoptering parents” is that they go to great lengths trying to control something that really can’t (nor shouldn’t) be controlled. And in the end, they tend to turn their kids into neurotic type-a personalities or on the other end of the extreme, kids who always get into trouble because I’m rebelling against mommy and daddy.

Analogies aside, ultimately I cannot do anything to convince someone to abandon their efforts on something like this. That’s on the people who wrote this proposal and those who support it. Hopefully though, as naive as it may be on my part, hopefully someone can see my side enough to at least not immediately dismiss the argument. I can dream. :)

I stand by my argument that it is a bad path to take to have the language dictate coding practices. After all, if a developer genuinely wants to “leak” data (btw, poor choice of wording - if I do it on purpose, it’s not leaking), they will find a way. And usually that way will be even worse than just letting them do it the same way.

On Sat, Sep 14, 2019 at 7:24 PM Ranando D Washington-King < notifications@github.com> wrote:

To further my point, it is perfectly reasonable to have private properties that are readable by outside the class. It’s not always about hiding the data itself, but keeping the data in a specific state (immutability).

That's where I (and probably TC39) will disagree with you. Its not about the state of the data but rather its accessibility. Regardless of the state of the data, controlling the means of accessing the data is what's important. The problem is that the design of this proposal is akin to "helicopter parenting". They're going so far as to ensure as absolutely as possible that a developer cannot accidentally leak private data. It's like saying, since fire can burn you, let's have you put on this asbestos suit while you carry this unlit book of matches. It's not the language's fault if a developer is sloppy and leaks data.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/tc39/proposal-class-fields/issues/257?email_source=notifications&email_token=AATJJT2N6PZLLKAVAJAJNXLQJVXDTA5CNFSM4IEWJHM2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6XF6BY#issuecomment-531521287, or mute the thread https://github.com/notifications/unsubscribe-auth/AATJJT7DIGNC3JGI3LZUT4DQJVXDTANCNFSM4IEWJHMQ .

-- "Do not go gentle into that good night. Rage, rage against the dying of the light." — Dylan Thomas

MichaelTheriot commented 4 years ago

This is another example of how intuition leads one to expect private fields to be dynamic.

It is important to note that dynamic access is in fact possible via eval. This grants the developer freedom to create dangerous code which bypasses the restriction.

'use strict';

class Antinomy {
  #x = 1;
  #y = 2;
  #z = 3;

  hasPrivate(identifier) {
    try {
      eval('this.#' + identifier);
      return true;
    } catch {
      return false;
    }
  }

  accessPrivate(identifier) {
    return eval('this.#' + identifier);
  }
}

let a = new Antinomy();

// dynamically test values
a.hasPrivate('nope');     // false
a.hasPrivate('x');        // true

// dynamically set values
a.accessPrivate('x');     // 1
a.accessPrivate('x = 5'); // 5
a.accessPrivate('x');     // 5

The rules can be hacked around.

let leak = Antinomy.prototype.hasPrivate; // save the leak
delete Antinomy.prototype.hasPrivate;     // remove the leak

// use the leak somewhere else
leak.call(a, 'x'); // true

The ability of eval to access private fields is not only contradictory but also abnormal. Somehow the function "knows" whether or not it was declared in the class body and eval will throw if not.

e.g.

function leak2(identifier) {
  try {
    eval('this.#' + identifier);
    return true;
  } catch {
    return false;
  }
}

leak.call(a, 'x');  // true
leak2.call(a, 'x'); // false, despite being functionally identical

This means the output of the same function is dependent on where it is declared.

nicolo-ribaudo commented 4 years ago

This means the output of the same function is dependent on where it is declared.

This is already true:

"use strict";

let leak, leak2;

{
  let a;

  leak = function () {
    try {
      a;
      return true;
    } catch {
      return false;
    }
  }
}

{
  leak2 = function () {
    try {
      a; // ReferenceError
      return true;
    } catch {
      return false;
    }
  }
}

leak(); // true
leak(); // false
MichaelTheriot commented 4 years ago

True, and the nature of eval lets input result in different output solely due to where it is called. So it's not exactly doing anything new, although the issue of leaking private in a hazardous way still stands and I would rather just allow dynamic access if it is possible to opt-in already. I am not sure what blocking dynamic access really achieves.

bakkot commented 4 years ago

Private fields offer very much the same kind of privacy as closures. eval offers exactly the same escape hatch for accessing private fields as it does for closed-over variables. There is no other way to dynamically access private fields, just as there is no other way to dynamically access closed-over variables. That doesn't really seem surprising.

rdking commented 4 years ago

@bakkot

Private fields offer very much the same kind of privacy as closures.

Patently false. If you had said "a similar kind" instead of "the same kind", then I could've let that one go. If it were the same kind, then using private fields in conjunction with Proxy wouldn't break.

ljharb commented 4 years ago

It's the same kind:

const Foo = (function () {
  const secret = new WeakMap();
  function Foo() {
    if (!new.target) {
      throw 'must use new';
    }
    secret.set(this, 42);
  }
  Foo.prototype.get = function () {
    if (!secret.has(this)) {
      throw 'brand check failed';
    }
    return secret.get(this);
  };
  return Foo;
}());
new Foo().get(); // 42
new Proxy(new Foo(), {}).get(); // throws

vs

class Foo {
  #secret = 42;
  get() {
    return this.#secret;
  }
}
new Foo().get(); // 42
new Proxy(new Foo(), {}).get(); // throws
rdking commented 4 years ago

It's different. What you showed is a case of shared closure. Every instance is using the same closure. Very different scenario than each instance using its own closure. It's the WeakMap in your scenario that fails, not the shared closure. Try this instead:

function Foo() {
  let secret;
  if (!new.target)
    throw "Must use 'new'";
  secret = 42;
  this.get = function get() {
    return secret;
  };
}
new Foo().get(); // 42
new Proxy(new Foo(), {}).get(); // 42

Of course, this has the weakness of recreating get() every time, but it still proves my point.

ljharb commented 4 years ago

Each instance is conceptually sharing the same closure in private fields; the one that holds one WeakMap per field, with the instance as the key.

The analogy without a WeakMap would be, perhaps, an array of entries that used === to locate the key.

rdking commented 4 years ago

That's my point. It's the wrong concept to use. The concept to use should be more similar to one closure per instance as if the constructor bound a closure to each instance and allowed access to the methods on the prototype. Doing things this way wouldn't prevent you from pushing the instance properties, but it would fix the unnecessary Proxy embargo.

This is what I meant when I said private fields do not offer the same privacy as closures. A closure owned by the class doesn't compare to a closure owned by each instance. What you've described is what I call WeakMap-based privacy, because it doesn't give you instance support without a WeakMap. Closure-based privacy only requires the closure.

ljharb commented 4 years ago

It sounds like you’re talking about instance privacy versus class privacy - a “closure per instance” model wouldn’t allow a class method (instance or static) to access the fields of another instance, and that’s a big pile of critical use cases.

rdking commented 4 years ago

It is instance privacy, but given the current design of ES, there's a way of giving the class the ability to access the closure of all instance. It would be available through an operator and only viable within the lexical context of the declaring class. In this way, instance closures would provide the privacy, and a specific operator would be responsible for accessing the closures of other instances of the same class.

Conceptually, it's very different from what this proposal does, but the implementation is likely to be as straight forward. The catch is that since closures would be used instead of internal slots, there would be nothing breaking the behavior of Proxy. Since the Prototype would be the place properties would be [[Define]]'d, there would be no inheritance issues. And, of course, initialization would still cause all of the properties to be [[Set]] on the instance, resolving the last set of major issues.

As I've been arguing for a long time, there are no technical issues preventing us from both getting what we want. It's not any kind of incompatibility, just a set of unfortunate decisions. There are 3 notions in this idea that don't directly align to the language, but given the spec, it shouldn't be hard to make it work.

  1. Allow an object to carry a closure
    • Currently, only function execution scope can do that. Allowing an object to carry them as well is a necessary piece
  2. Apply the carried execution scope of the function's context object to an executed function that lexically came from the same class that created the execution scope.
    • This is what would allow prototype functions to have access to the context and the private data in it.
  3. Use an operator (->, #.,.#,]>, or whatever) to temporarily access a binding contained in the closure on a different instance of the same class.
    • This solves the problem(s) you just mentioned.

As I said, and keep saying. It can be done.