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

Nested classes create inconsistent access paradigms #111

Closed bdistin closed 5 years ago

bdistin commented 6 years ago

img

The above is tested in V8's implementation of this proposal. While both are true according to spec, only one should be true according to the requirements laid out in the FAQ

Namely:

  1. It is a goal of this proposal to allow accessing private fields of other instances of the same class.
  2. It (Hard-Private/Encapsulation) means that private fields are purely internal: no JS code outside of a class can detect or affect the existence, name, or value of any private field of instances of said class

So let's break that down:

On the left, we have private fields being accessed in a class that is not the same as the class the code is being run from. It is my personal opinion that is a breach of Hard-Private/Encapsulation, and the left example code should be considered incorrect. However, as that is an opinion, I am open to the idea "What if the left code results are correct?"

On the right, we have the result I would have expected given a strict same class definition of Hard-Private/Encapsulation. But in the context that the left is correct, we get an equally unexpected result.

  1. Given 2 public fields, you can access both inner and outer public fields, given the this (context): img
  2. Given 2 private fields and providing instances of the inner class and outer class as this, as shown in the top image right, you can only access the inner private field.
  3. Given 1 outer private field and an instance of the outer class, as shown in the top image left, the outer private field is accessible.

Assuming access to the outer private field is given to an inner class, it would be expected to be able to access both the inner and outer private field, just as we can access as public fields. The this syntax implies that #a (a private a field) should be just as relative to the this of the function call, as a (a public a field). As it stands, declaring a private field shadowing an outer private field affects the accessibility of the outer private field.

This issue arose in the conversation here: https://github.com/tc39/proposal-private-methods/issues/28

Edit: I concede that the left-hand side should be considered correct. I also understand why the result of the right-hand side is how it is. But I think the right-hand side still is not ideal/greatly breaks consistency and expectations setup by public fields.

ljharb commented 6 years ago

I think the results on the right are correct (since A and B each have a distinct #a). Which one does v8 implement?

Either way, i don’t expect that you should be able to access the outer #a if you have shadowed it with an inner one. A public field using a Symbol that’s stored in a variable of the same name seems like a more apt comparison to me - when the outer one is shadowed, the inner one is the only key you can get to.

bakkot commented 6 years ago

@ljharb, the left and the right differ in that the left lacks a shadowing #a in the nested class, so they're both correct.

bdistin commented 6 years ago

All screenshots are from the latest chrome canary console/repl.

While I understand your comparison using symbols, can you see how this is confusing to a developer? How the expected access paradigms can only work when certain planets are aligned, but never would they all work at the same time (like they do with public fields)?

Then if we look at the situation through a "strict same class" lens, all access paradigms are very clear and defined. You would never be able to get the outer private field because that's not the class that's running the code. But you would be free to get it's own private field, just as you would be able to get private fields from any same class situation.

bakkot commented 6 years ago

Then if we look at the situation through a "strict same class" lens, all access paradigms are very clear and defined. You would never be able to get the outer private field because that's not the class that's running the code.

I don't share this intuition at all! In your screenshot above, B is as much a part of A as would be function B(x) { console.log(x.#a); }.

jridgewell commented 6 years ago

I think this is very similar to lexical scoping:

function A() {
  let a = 1;

  function B() {
    let a = 2;
  }
}

Here, you've obviously overshadowed a in the inner B function, so you no longer have access to it. You can use helper functions, etc, to allow manipulation.

bdistin commented 6 years ago

So I have spent some time looking at other languages, and I think I was wrong about being strict about being the same class, as in other language's nested classes are a precursor for friend classes.

In fact, I can see the following example being a useful code pattern (more so than a method defining a class sometime after the containing class is JIT-compiled): img

I think that's a strong argument, as it does set up the possibility of a future of actual friend classes to Javascript.

So back to the right side, take that pretty new example and make both private fields #a instead, we are back to only being able to access the inner #a when as it is written it couldn't be clearer we want the outer #a: img

But it's not to do with the base object (either this or an instance of the containing class) it's because the meaning of #a itself changes. Akin to the Symbol example ljharb pointed out. So it seems to me that the right hand side is due to technical limitations, rather than an ideal design. I can understand this, and I don't have a suggestion on how you could maintain the meaning of both '#a's, so that this.#a and this.#container.#a would both work as you would expect to work if they were public.

bdistin commented 6 years ago

Wait, why does #a act like a symbol? I thought this proposal worked on weak maps like in the example:

const Person = (function(){
  const privates = new WeakMap;
  let ids = 0;
  return class Person {
    constructor(name) {
      this.name = name;
      privates.set(this, {id: ids++});
    }
    equals(otherPerson) {
      return privates.get(this).id === privates.get(otherPerson).id;
    }
  }
})();

This falls back into other arguments I have made to use private as a keyword, as a function, and as an object to provide a more consistent private to classes than what the # Sigil can.

Assuming private x = 'foo'; does privates.get(this).x = 'foo'; effectively; private.x is privates.get(this).x effectively; and private(obj).x does privates.get(obj).x effectively: allowing for dynamic access and destructuring. Wouldn't that also fix this issue?

You have no weird access issues because .a is ['a'] is always a, and not some symbol reference.

In fact, let's solve all of those issues with a WeakMap and a couple proxies:

const A = (function(){
  const pri = (function(){
      const __private__ = new WeakMap();

      function proxyFactory(instance, privateValues = __private__.get(instance)) {
          if (typeof privateValues === 'undefined') return undefined;
          return new Proxy(privateValues, {
              apply(target, thisArg, args) {
                  return proxyFactory(args[0])
              },
              get(target, prop, receiver) {
                  if (typeof target[prop] === 'function') return target[prop].bind(instance);
                  return target[prop];
              }
          });
      }

      return new Proxy(Function, {
          construct(target, args, newTarget) {
              let privateValues = __private__.get(args[0]);

              if (!privateValues) {
                  privateValues = new target();
                  __private__.set(args[0], privateValues);
              }

              return proxyFactory(args[0], privateValues)
          }
      });
  })();

  return class {

      constructor() {
          const privates = new pri(this);

          privates.B = class {

              constructor(container) {
                  const privates = new pri(this);

                  this.bar = 'bar';

                  privates.container = container;
                  privates.a = '10';
                  privates.d = function() {
                      return this.bar;
                  }
              }

              get a() {
                  const privates = new pri(this);

                  return privates(privates.container).a;
              }

              get d() {
                  const privates = new pri(this);

                  return privates.d;
              }

              get b() {
                  const privates = new pri(this);

                  return privates.a
              }

          };
          privates.a = '42';
          privates.b = '24';
          privates.d = function() {
              return this.foo;
          }
          this.foo = 'foo';
      }

      init() {
          const privates = new pri(this);

          const b = new privates.B(this);
          console.log(`we can get the private a field in both containing and nested classes: ${b.a}, ${b.b}`);
          console.log(`let's do a dynamic access, privates['a'] is: ${privates['a']}`)
          console.log(privates.a, '===', privates(this).a);
          const { a, b: c } = privates;
          console.log(`we did a destructure! ${a}, ${c}`);
          console.log(privates.d(), b.d());
          try {
            privates({}).test;
          } catch (err) {
            console.error(err.message);
          }
      }

  }
})();

new A().init();

Feel free to run that code if you wish, as it absolutely does what # Sigil fails to do. If we could hide some of that ugly setup code around the class, and in each of the methods, and drop the 's' off privates we would really have something tbh...

Edit: I fully understand that the example provided is not completely perfect.

For instance, while yes, in other languages getting the private state of the containing class is possible. I can't be sure it's intended to go the other way around... And for that reason, the nested class probably would need a similar function wrap. And if we wrap the nested class, then how does it access the parent's private. So that aspect would require further thought.

However, the example does display dynamic access and destructuring which the # Sigil fails to provide with a fair degree of accuracy I believe.

mmis1000 commented 6 years ago

@bdistin The proposal used to work like the way WeakMap works, but it soon changed to the way just like symbol works.
And this already happened for a long time.

bdistin commented 6 years ago

The FAQ makes it clear that symbols can only make hidden, but not encapsulated, fields: https://github.com/tc39/proposal-class-fields/blob/master/PRIVATE_SYNTAX_FAQ.md#how-can-you-provide-hidden-but-not-encapsulated-properties-using-symbols

That, and it's been stated repeatedly that private has a "fairly straightforward and precisely equivalent desugaring using WeakMaps"

mmis1000 commented 6 years ago

@bdistin Doesn't "fairly straightforward and precisely equivalent desugaring using WeakMaps" means It is implement with WeakMap, but it works in the way symbol works?

bdistin commented 6 years ago

the statements do not say, using WeakMaps and symbols... They always exclusively refer to WeakMaps...

And you are going off on the wrong tangent... The point is, they are breaking tons of consistency and functionality by blindly sticking to their # Sigil idea, no matter what it breaks or what the community thinks about it. And that's the real problem here.

mmis1000 commented 6 years ago

No, they aren't. It just works as if it is use symbol to implement it but actually implement with WeakMap, That's all.

mmis1000 commented 6 years ago

And the way you describe actually has some shortcomings that may even fail on a very simple case, consider this.

const store = new WeakMap();
const privates = (ins)=>{
  let privateData = store.get(ins) ?
    store.get{ins}:
    {};
  store.set(ins, privateData);
  return privateData;
}

class test {
  constructer () {
    privates(this).message = 'hello'
    privates(this).print= function () {
      this.format(privates(this).message)
    }

    privates(this).print()
  }

  format (str) {
    console.log(`I say: ${str}`)
  }
}

It will fail, because the this in privates(this).print is now the private data holder associated to this instance instead of the instance itself.

bdistin commented 6 years ago

Please read and comprehend:

I fully understand that the example provided is not completely perfect.

While yes it's a working loose example, it is not a fully fleshed out proposal. For instance, it would have to be an error to directly return the private namespace. It merely points to a syntax that doesn't break the existing language paradigms and is far more future proof.

Does tc39 intend on coming up with another half dozen ASI compatible sigils to add protected and other access modifiers? Is there even that many left? We certainly can add private to Javascript without breaking the language (in all sorts of unexpected ways) and screwing up the future because of short-sightedness.

Edit: it's also trivial to overcome that issue... my example is now updated to handle that and a few other issues, along with working examples/tests. Still I make no claims on it being a fully fleshed out proposal, or that it is perfect.

rdking commented 6 years ago

@bdistin @mmis1000 The current proposal when desugarred, looks a lot like this:

class Sugar {
   #field = 2;
   getField() {
      return this.#field;
   }
}

const SugarFree = (function() {
   let pvt = new WeakMap();
   const field = Symbol("field");
   return class SugarFree {
      constructor() {
         pvt.set(this, { [field]: 2 });
      }
      getField() {
         pvt.has(this) || throw new TypeError();
         return pvt(this)[field];
      }
   }
})();

The Symbol usage is the [[PrivateName]] for each private field lexically scoped to the definition of the class. That's why new functions attached to the prototype won't have access to the private members of the class. The WeakMap usage substitutes for the internal slots used to store the private data.

I came up with a solution that fixes this and several other breaks in the current proposal earlier. Treat # as a binary operator, making access syntax either obj#.field or obj#['field']. Reusing @bdistin's example:

class A {
   #B = class {
      #container;
      #a='10';

      constructor(container) {
         this#.container = container;
      }
      get a() { return this#.a; }
      get b() { return this#.container#.a; }
   }

   #a = 42;

   init() {
      const b = new this#.B(this);
      console.log(b.a, b.b);
   }
}

new A(). init(); //output: '10' 42

It would work because the # as an operator translates from this#.a to A.[[WeakMap]].get(this).a if this was constructed by A, and to B.[[WeakMap]].get(this).a if this was constructed by B. (with appropriate checks done before access). Changing the proposal this way would also fix the other access issues with the current proposal. I hope @littledan will reconsider.

bdistin commented 6 years ago

const field = Symbol("field");

That sort of behavior is problematic for shadowing reasons in nested classes, and for a future friend class feature.

If you were to define another class as a friend, it wouldn't have access to those Symbols, so that would just fail from the start... Not to mention, if you try to access a field that the class doesn't have, there is an error thrown when the class is defined. That obviously has to go before friend classes can be a thing because you are going to be accessing private fields that the class doesn't have.

Also if you are treating #. as it's own unique operator, something like what's proposed in classes 1.1 -> would probably be better suited IMO. Even if that means you need one more char to do dynamic lookup this->['a'].

Edit: @rdking a lot of the underlying ideas between your idea and my ideas about using the private keyword for access, are the same. The bigest issues I take with private as it's defined in the proposal is all the functionality/paradigms it breaks without considering future features because most of us know this is just the beginning of access modifiers in javascript. There are a lot of well-defined features that can and should follow this feature (private). If these issues can be resolved, I think it's less of an issue of what the syntax becomes.

mmis1000 commented 6 years ago

From my perspective, I don't think mix up of two property lookup way in a single language is a good idea. There are enough people from java(or some other class based language) to get confused by the way js this lookup (property of this in method is decided by who call the object).
And now, you also add the way java lookup this (method and property is attached to object) to js. The people from js also get confused by the way java lookup now.
Even looking for property in another way sounds more reasonable, it is still not a good idea to mix several similar but different way of lookup together. It will just confuse everyone that use this language.

ljharb commented 6 years ago

@bdistin I'm not so confident that this is "just the beginning", nor that other access modifiers are even appropriate. JavaScript doesn't have "public" - everything's either reachable, or not. Thus, reachable things are "public" and unreachable things are "private", and this feature is syntax for unreachable per-class-instance things. I don't think the case has been convincingly made that protected/friend/etc is warranted, and since it's entirely achievable in the language when built on top of either properties or private fields, I'm not sure we need to worry about preserving a specific space for it.

bdistin commented 6 years ago

JavaScript doesn't have "public" - everything's either reachable, or not.

How is it you can say private is appropriate then? While the vast majority of javascript is defacto public by implication, there does exist closure based variables that cannot be leaked. A fact the shim for private takes advantage of.

Many arguments have been made that there aren't enough mechanisms for encapsulated state and behavior currently in javascript. Can you state that there are (even after including this proposal) enough mechanisms for horizontal (friend) or vertical (protected) shared encapsulated state and behavior in javascript?

Do you not see a value, or think that javascript provides enough mechanisms for this sort of code pattern? Taking a spin on an FAQ example... (I will use keywords for clarity of all readers)

class Line {
    private lazyLength = null;

    protected points = [];
    protected pointsLimit = 2;

    add(point) {
        if (!(point instanceof Point)) throw new TypeError('You can only add Point instances to a Spline');
        if (protected.points.length + 1 > protected.pointsLimit) throw new RangeError('A Line is composed of exactly 2 points');
        private.lazyLength = null;
        protected.points.push(point);
        return this;
    }

    get length() {
        if (private.lazyLength === null) private.lazyLength = protected.length();
        return private.lazyLength;
    }

    protected length(point1 = protected.points[0], point2 = protected.points[1]) {
        if (protected.points.length < 2) throw new RangeError('There must be atleast 2 points to determine a length');
        if (point1.equals(point2)) return 0;

        const { x: x1, y: y1 } = private(point1);
        const { x: x2, y: y2 } = private(point2);
        return Math.sqrt(((x2-x1) ** 2) + ((y2-y1) ** 2));
    }
}

class Spline extends Line {
    protected pointsLimit = Infinity;

    protected length() {
        let length = 0;
        let lastPoint = null;

        for (point of protected.points) {
            if (lastPoint) length += protected(super).length(point, lastPoint);
            lastPoint = point;
        }

        return length;
    }
}

class Point {
    friend Line;

    private x;
    private y;

    constructor(x = 0, y = 0) {
        private.x = x;
        private.y = y;
    }

    equals(point) {
        return private.x === private(point).x && private.y == private(point).y;
    }
}

Let's look at what we have:

  1. Point#x and Point#y are horizontally encapsulated with the Line class, through use of defining Line as a friend to Point. But as they are private, they are limited vertically. Trying to access the private x or y state of a point in the Spline class would be impossible unless Spline was also declared a friend to Point.
  2. Line#lazyLength is encapsulated to the class Line. Neither the Point or Spline classes can see the private state held in lazyLength.
  3. Line#points, Line#pointsLimit, and Line#length() are vertically encapsulated between the Line and Spline classes. While a Point cannot see these private states or behavior, the Spline class can see, modify, and extend that shared private state and behavior to the advantage of Spline's unique identity.

From the research I have done this evening, I cannot find a single major OOP language with access modifiers that do not at least provide public, protected, and private. And the majority provide mechanisms like friend classes to provide horizontal encapsulation as shown in the above example. C# goes even further and adds a few more access modifiers yet. If these major OOP languages have unanimously decided the importance of shared encapsulation, why do you think javascript should have the hubris to not warrant mechanisms for shared encapsulation?

I certainly don't think there are enough mechanisms available to provide shared encapsulation, and it would be a mistake to not account for the need of such functionality and an even greater mistake to make them semantically impossible by forcing a poorly planned specification into the language.

mmis1000 commented 6 years ago

@bdistin JavaScript is NOT a OOP language. It is a prototype-based language. The class in JavaScript merely means a template of object, You can even live without them if you you like.

bdistin commented 6 years ago

Look up the definition of JavaScript anywhere, and you will be squarely proven wrong:

What is JavaScript? JavaScript® (often shortened to JS) is a lightweight, interpreted, object-oriented language with first-class functions, and is best known as the scripting language for Web pages, but it's used in many non-browser environments as well. It is a prototype-based, multi-paradigm scripting language that is dynamic, and supports object-oriented, imperative, and functional programming styles.

https://developer.mozilla.org/en-US/docs/Web/JavaScript/About_JavaScript https://en.wikipedia.org/wiki/JavaScript ...

maple3142 commented 6 years ago

JavaScript is a object-oriented (prototype-based) language. Both class-based and prototype-based is object-oriented. https://en.wikipedia.org/wiki/JavaScript https://en.wikipedia.org/wiki/Prototype-based_programming

jridgewell commented 6 years ago

@mmis1000, @bdistin: I'd like to remind you that TC39 is governed by a code of conduct, the first of which is be respectful and the second is be friendly and patient.

While none of your comments has yet crossed the line, they're certainly heated. Further rude comments may be moderated.

mmis1000 commented 6 years ago

There is actually a major OOP language lives without hard private, it is python.
The private in python merely prefix the property with class name. (it looks similar enough to the current proposal. You just name it with a scope exclusive name.) And it trust programmer won't touch it incorrectly.
Currently, JavaScript just do it in the same way.

I think, the problem you though of override parent private property name, may be fixed with just a prefix

class A {
  #a = 0;
  check () {
     class B {;
       #a = 0;
       compare (A_inst) {
         A_inst.#A#a === this.#a;
       }
     }

     return new B().compare(this);
  }
}

Instead introduce such complex behaviour, a simple prefix just fix the problem you described.
this.#a just become a shorthand of this.#MyClassName#a. And the whole confusion is gone.
Because you won't be able to get #B#a from class A, so A_inst.#a in class b is going to be an error.

I guess that is why I thinks this proposal looks good to me somehow, maybe just because i wrote python before?

maple3142 commented 6 years ago

@mmis1000 How about anonymous class? const A=class{} They don't have a name, so how to handle them?

mmis1000 commented 6 years ago

The name should be convert to the object's internal id at the method construct phase(so different class object get their own private field(aka internal slot)), since you never assigned a name to it, no class below them will be able to use that. only itself can use it, because it know what itself is.

Because you internally not name it, it will be not referable by other class under it just as you wish.

Although the way you wrote is actually going to make a named class.

// in firefox
const A = class {} // this is a class named A
const B = (0, class {}) // this is an anonymous class
bdistin commented 6 years ago

@jridgewell I apologize, I had lost my patience in that last comment. I will try to maintain patience going forward.

@mmis1000 According to what I can find on python: python intentionally does not provide access modifiers. (I did qualify in my above comment only taking into account major programming languages that provide access modifiers) Python is like Javascript up until before this proposal taking a "We're all consenting adults here" philosophy. What you can do with a double underscore in python, you can do with symbols in javascript. It's not truly private, just somewhat hidden.

If javascript chose to continue with a "We're all consenting adults here" philosophy, shared encapsulation would be unnecessary as it is currently. And that would be completely fine. But tc39 has decided to take the route of many other OOP languages, and provide encapsulation mechanisms. That philosophy is equally fine. I think both sides of the philosophy have merits.

After looking at python examples, I can see what you are trying to emulate with A_inst.#A#a but I have to say, that is compounding the complexity of the already confusing # Sigil proposal. If I came across that while I was reading some javascript source, I would certainly have trouble following the logic being expressed.

Where I see the "evolution of access modifiers in javascript" is very well expressed in the FAQs Point class. The developer doesn't want to expose x or y as a part of the public API. If that's what the developer really wants, I think that's a fine philosophy. But simple class-based encapsulation only goes so far. Sure you can make an equals method to compare to instances, but by making that state encapsulated to the class you have just made it impossible to use that class with other classes without being forced to expose x and y publically with the current proposal.

That's why I believe private class state and behavior, if done, must be done in a holistic way. Encapsulation is so much more powerful a tool when given mechanisms for developers to explicitly share that encapsulation with other classes. Developers don't just want Points, they want to use those Points to make Lines and Splines and so much more while providing the public interface they want to provide.

mmis1000 commented 6 years ago

As I mentioned above, introduce a complex behavior that different to current way won't make anyone happy. Newbies will find he has to look for a property in two different ways at same time, make learning the language more difficult. People already use language for a long time also needs longer time to tell himself that there is another way to look the property if it is a private field. Or he is going to be accidentally trapped by the new mechanism.
As a user who already use this language for a long time. I think such thing is very disgusted.
If you think the way JavaScript works currently is really bad and not worth using, I think you can just use another true class based language that compile to js(maybe dart or kotlin or some other?), instead of mix up everything together and confuse everyone.

Partial refactoring can sometimes be worse than not do it at all.
Consider there is a project A works with lib a. Then some years later, the first maintainer leaves, the second maintainer think lib b is better and rewrite part of the project with b. Again some years later, the second maintainer leave, the third comes. Now the new maintainer found he has to learn lib a and b and same time, or he won't able to maintain this project.
Add everything you think good in a single language without remove the old one just make the same things happen.
You will find that, unless you learn a, b, c, d, e, f..... at same time, you don't know what other programmer wrote about.

Beside this, even everyone will happily learn two ways it work at same time. They may still can't use it very productively. For some people, context switch between different model (like write Java <->write JavaScript) is a very expensive operation. He will became somehow less productive for a short time after the context switch. Add different model to js just make the same thing happen in a single language.

bdistin commented 6 years ago

From my point of view, the new syntax serves one purpose. There must be a way for a private, and a public field to have the same identifier, without collision. Continuing on with that, protected and other access modifiers would also need to have the same sort of mechanisms to prevent collision with public and private fields of the same name.

While there are many ways to solve that problem (# Sigil, different operator, etc), I find using the private keyword to define and access the fields on instances of the class, and other instances where explicitly defined, to be the clearest, most expressive, and congruent with existing paradigms; syntax. That's just my opinion. Where I stated above about the -> operator, that was an "If it's insisted to use an operator to solve the collision problem, then -> would be the lesser of two evils compared to #."

Take my above example, pretend we are sticking with the "We're all consenting adults here" philosophy, and you will see that when using the private keyword the code maps very consistently: img

Edit: So yes, when considering different syntaxes, it should be considered how consistent the new syntax's behavior is with existing syntax behavior. I agree with that point, but that's also exactly the complaint I am lodging in this issue. The proposed syntax isn't congruent with existing syntax behavior creating unexpected and confusing "gotchas" like not being able to destructor, or dynamic access, or access containing class fields when the nested class implements a field of the same name (even though that behavior can be expressed like this.#container.#a === this.#a, the implementation fails to provide expected results).

mmis1000 commented 6 years ago

Although the readme claims it you can just change _ to #, it actually works more like the way python prefix it or like the symbol in js. Collide with same name means nothing. As it is automatically prefixed with different instance.

According to the proposal, current behavior looks more like this if you are not using weakmap or symbol to make it capsuled. default

Collide with the name does not matter as it create a unique name every time when a classConstructor is initialized.

If you use loop to create a lot of classConstrutors, you will just get a lot of classConstrutors that have different private field.

Looks very like the way python works, but still has some different as js can initialize classConstructor everywhere at any time. Prefix with the class name does not make sense, because js can create a class very much times.

kyranet commented 6 years ago

I have been reading this discussion (between @bdistin and @mmis1000) in multiple repositories about the private fields approach and I have the feeling that the former is trying to demonstrate points and the latter is trying to prove him wrong.

Despite of your differences, bdistin has proven many valid points, his approach allows consistency, unlike mmis1000 mentioned, I find it easier to learn this, private, and protected, between other accessment operators that may come in the future, than using different sigils.

Everything is set to the simple fact the average developer usually learns ES5 code first (oldest and more robust tutorials/guides for JS), then Promises (alongside ES6) and climb until the latest standard version as their experience in the language increases. By this fact, private is something they'll learn very lately, and if they do earlier, they'll probably do very well.

The # has many problems, many coming from the this "weird" behaviour, and going through many problems such as it being inconsistent with the language (no dynamic object accessment), hard to read (the # sigil looks very heavy), not self-documented code (for a new developer, what does # mean? They won't know until they learn this stage of the standard. What does private mean? It's self-documented, it's a private variable, and they can look up in MDN or somewhere else for that).

What Python does is not a concern for us, JavaScript and Python are different languages, with different philosophies and design. If you dislike hard private fields/methods, I would suggest you to continue using _ like we have always done, it's not syntax we will remove.

Since in your point of view the # sigil is better, @mmis1000, may I ask the advantages of it over @bdistin's approach, please?

mmis1000 commented 6 years ago

The most important advantage I think is that you can also limit the behaviour of this to not happen at the new private field. You can disallow the use of private.myField to be used if the this is incorrect with some lint rules. (maybe we can name it disallow-creepy-private-field-shorthand) You fault will now be instantly highlighted with a big red X if you cause an accident.

original proposal

// consider this
class {
  #value = null;
  listen() {
    someBrocaster.register(function () {
      // oops, you it brokes, but you won't know it until you run
      console.log(this.#a); 
    })
  }
}

You don't know what is this from and it will not prevent you to use the wrong this until your program crashed. And you never have a way to check it at design time because their syntax is exactly the same.

But with private and a simple extra limit to disallow the shortcut when use it at wrong place.

Failed:

// consider this
class {
  private value = null;
  listen() {
    someBrocaster.register(function () {
      // you are not allowed to use the shortcut because you have the wrong `this`
      // why you trying to get your own field from the weird thing coming from some other?
      console.log(private.value);
    })
  }
}

Passed

// consider this
class {
  private value = 'Hi';
  listen() {
    someBrocaster.register(()=>{
      // ok, `this` is exactly the same, so shortcut is allowed.
      console.log(private.value); 
    })
    someBrocaster.register(function () {
      // also ok, you claimed you will check it by yourself
      console.log(private(this).value); 
    }.bind(this))
  }
}

Whether you just think private shortcut is a keyword point to this(like js) or just a variable name that will shared within every methods(like java) does not matter now. Because it won't let you to use it if it does not know what the this is, private shortcut will always means the correct instance like in some other class based language (just like what bdistin desired).

But you can make it without mess up the way js works.
It is instead now works in the both way js and java works.
Everyone is happy.

And private(this).something means private field from another instance. You will need to carefully use it because it might throw when you pass the wrong thing into it. But i think this is totally unrelated to this issue.

kyranet commented 6 years ago

Thanks you for your reply! That is a good point, but I see that leads to the same issue as the this. That means private is being consistent with this, with the only exception it can't be bound (but private(this) can be used as a workaround).

I have also seen developers use aliases (const that = this;) inside proxies to get the instance of the class, specially when they needed the this value inside a Proxy (or nested Proxy).

That way the following code will work like a charm, and that practise is not rare, I believe developers who have been working before ES6's arrow functions went live are familiar with this:


class {
  private value = 'Hi';
  listen() {
    const privateThat = private;
    someBrocaster.register(()=>{
      // ok, `this` is exactly the same, so shortcut is allowed.
      console.log(privateThat.value); 
    })
    someBrocaster.register(function () {
      // also ok, you claimed you will check it by yourself
      console.log(privateThat.value); 
    }.bind(this))
  }
}

Note: The alias for private may or may not be valid, it depends on the semantics the final proposal will have, but in case it throws, consider const privateThat = private(this); instead.

If you need to "guard", you can name the class and perform an instanceof check:

function () {
  if (!(this instanceof MyClass)) throw new TypeError('Incompatible receiver sent.');
  console.log(private(this).value); 
}.bind(this);

But realistically, in real world, this is not a problem, unless the API you use is exposing the functions, this should not be an issue. The API may use those privates in the collections for the callbacks, achieving a full security guard. Just in case, you can always perform that check, or use the alias workaround.

I agree that this.#field is secure, since it throws an error if you try to access a field that does not exist, but in my opinion, that feels unwise and it's inconsistent with the current design of JavaScript. IMO this is another flaw of the current proposal.

Edit: Fixed a typo in the example, replacing private to privateThat for consistency between both kinds of non-generic functions.

Edit 2: Fixed last codeblock where it shows an example of guarded dynamic private accessment with the bound this value.

mmis1000 commented 6 years ago

I think that's why the proposal @zocky proposed does not allow alias or deference the private shortcut or private(this). It will be always a keyword but not an actual object, just like super. Since you can't alias/deference it, such confusion will not get the chance to happen. And private(wrongThis) should still throw, it is just allow you to write it, doesn't mean it allow you run it.

ljharb commented 6 years ago

@mmis1000 you have to be able to get private fields on other instances of the same class tho. ie, static compare(a, b) { return a#id === b#id; } should work if a and b are instances of the same class.

mmis1000 commented 6 years ago

classConstrutor instance !== class instance

class A{} A !== new A()

class instance constructed from the same classConstructer should share the same private field of course.

ljharb commented 6 years ago

yes, I'm aware of that, but i'm talking about a static method accessing instance fields. The same would apply with compare(other) { return this#id === other#id; }

mmis1000 commented 6 years ago

The syntax access property from other instance proposed by @zocky is private(that).id so it should be

static compare(a, b) { return private(a).id === private(b).id; } 

https://gist.github.com/mmis1000/3025eaad2d39a035a016d49ab42f26f5
I made a compare with each proposal and non capsuled symbol polyfill.
The proposal is actually almost strict 1:1 map to current syntax.
It does not even change the behavior current proposal works.
That why i say it is unrelated to current issue, it is not a new way of lookup or capsule, it is merely a syntax. and may be even possible to be achieved with just a macro, discuss in this thread does not make too much sense. The title of this issue is Nested classes create inconsistent access paradigms #111 , so I think we are not discussing about the syntax here

mmis1000 commented 6 years ago
replace /private\((\w+)\)\./ with "$1.#"    private(that).id      become that.#id
replace /private\./          with "this.#"  private.id            become this.#id
replace /private\s+get\s+/   with "get #"   private get some() {} become get #some() {}
replace /private\s+set\s+/   with "set #"   private set some() {} become set #some() {}
replace /private\s+/         with "#"       private some() {}     become #some() {}

you will find you convert from that proposal to current

bdistin commented 6 years ago

As far as syntax, all of the examples you have shown are correct, and you are right that most of the syntax has a 1:1 mapping. But there are other's that have no working analog with the current proposal.

const { x, y } = private; // const { #x, #y } = this; // doesn't work
const { x, y } = private(that); // const { #x, #y } = that; // doesn't work

const calculated = '#secret'; // to be used below

private[calculated]; // this['#secret']; // is a very problematic syntax (this can refer to a public property when you do `this['#secret'] = 'something';` somewhere, and you can't be leaking private by accident)
private(that)[calculated]; // that['#secret']; // is also very problematic for the above reasons
private(private.containingClassInstance).a === private.a; // this.#containingClassInstance.#a === this.#a; // seems like it should work (there is no question of what it means as expressed), but due to implementation details using symbols, it just doesn't work
mmis1000 commented 6 years ago
const { x, y } = private; // const { #x, #y } = this; // doesn't work
const { x, y } = private(that); // const { #x, #y } = that; // doesn't work
// These two are syntax error. because the proposal proposed by @zocky does not make private a identifier
// it is like doing this on super
const { x, y } = super;

const calculated = '#secret'; // to be used below

private[calculated]; // this['#secret']; // is a very problematic syntax (this can refer to a public property when you do `this['#secret'] = 'something';` somewhere, and you can't be leaking private by accident)
private(that)[calculated]; // that['#secret']; // is also very problematic for the above reasons
// yes, these two are not incompatible, I think they should be removed
// I think that should be discused in @zocky 's thread.

private(private.containingClassInstance).a === private.a; // this.#containingClassInstance.#a === this.#a; // seems like it should work (there is no question of what it means as expressed), but due to implementation details using symbols, it just doesn't work
// just like as doing
this.#containingClassInstance.#a === this.#a
// I think the sigil proposal actually make chaining easier, this is a disadvantage of private keyword proposal 
kyranet commented 6 years ago

Regarding https://github.com/tc39/proposal-class-fields/issues/111#issuecomment-404908637, new A() instanceof A returns true, new ChildClassOfA() instanceof A returns true (as long as ChildClassOfA is a class that extends A). Perhaps we want to do if (!(this && this.constructor === MyClass)) throw new TypeError('...'); so you'll only do operations if this is an instance of the class.

I did some testing in Google Chrome Canary 69.0.3489.1 and the following code throws:

class A {
    #a = 'test';
    init() {
        (function() {
            console.log(this.#a);
        })();
    }
}
new A().init();

Whereas binding the function to the class instance does not throw.

Similarly, trying to "leak" the private fields via Function#call or Function#bind to change the this context with an instance of another class (with a different implementation of the equals method) always throw. Consider the following code:

class A {
    #a = 'CLASS A';
    equals(inst) {
        return this.#a === inst.#a;
    }
}
class B {
    #a = 'CLASS B';
    equals(inst) {
        try { console.log('this', this.#a); } catch { console.log('Error: this'); };
        try { console.log('inst', inst.#a); } catch { console.log('Error: inst'); };
        return this.#a === inst.#a;
    }
}
const [a, b] = [new A, new B];

Running a.equals(new A); works fine, so does a.equals.call(new A, a);, doing b.equals.call(new A, a); causes an error in both accesses (both catch branches are called), b.equals.call(new A, b); errors in the first (this is an instance of A) but successes in the second (inst is an instance of B).

So the semantics are working fine here, you can call or bind with another context to A#equals with an instance of A and it will always work fine, using an instance of another class or children of A will always fail. That's intended and works correctly.

To avoid private data leaking, we can apply the same private security from this.#prop to private.prop, that way, private(this) or private(other) will only work if this and other are instances of the class where it's being called. That way, you cannot bind another this value to the function to cause unexpected errors, but throw like the current API does.

I think that's why the proposal @zocky proposed does not allow alias or deference the private shortcut or private(this). It will be always a keyword but not an actual object, just like super. Since you can't alias/deference it, such confusion will not get the chance to happen.

Yeah, my bad. We can apply similar semantics for the private keyword/expression (it's an hybrid, likewise the import API, it works as a keyword (import x from 'a'), as a function import() and as an object import.meta). const priv = private; may not work, but const priv = private(this); should. If not, may you link/reference the comment that bans this kind of behaviour, please?

you will find you convert from one proposal to another

Not really, the private keyword/expression allows more, such as dynamic accessment. Check https://github.com/tc39/proposal-class-fields/issues/100#issuecomment-400724177.

And one question, how would you transform this to use the # sigil?

In case the First-Class Protocols lands in JavaScript, we may want to be able to use both features together. But considering the current syntax (as the proposal is current stage 0: https://github.com/michaelficarra/proposal-first-class-protocols)

const SOME = 'some';

class A {
    hasKey(key) {
        return private[SOME](key);
    }
    private [SOME](key) {
        // Body of the function
    }
}
mmis1000 commented 6 years ago

@kyranet I had already said this is the only part that are incompatible with the sigil proposal. Since they by design disallow dynamic name. remove this will make them completely compatible. And you missed this link

In methods, private and private(otherObject) are not valid constructs without being followed by .field or ["field"] and throw syntax errors.

So aliasing it is actually an invalid syntax, the chart @bdistin created just get a little wrong about this.

bdistin commented 6 years ago

Oh right, I forgot another thing that's not possible with the # Sigil syntax:

    private *[Symbol.iterator]() {
        for (const member of this.values()) yield private.metaData.get(member);
    }

Define an encapsulated for of loop behavior on a map. And other encapsulated symbol state and behavior.

mmis1000 commented 6 years ago

@bdistin the proposal proposed by @zocky actully disallow Destructure, I though the chart you create needs a little rewrote? The point 5 said that https://github.com/tc39/proposal-private-methods/issues/28#issue-306250831

bdistin commented 6 years ago

That's a detail I don't necessarily agree with, as there are many times when you should be able to use private as a construct. for(const metaData of private) or for(const metaData of private(other)) for instance with the Symbol.iterator example, and destructuring should be valid. The only times an error should be thrown is when trying to do private(somethingWithoutAccess).field; (a simple cannot access property 'field' of undefined would be more than enough) or return private;.

Disallowing private to be used whenever not followed by .field or ['field'] is a heavy-handed answer to only prevent return private; from happening. But that specific feedback really doesn't belong in this issue.

Edit: Note: I just updated the shim example to handle privates(somethingWithoutAccess).field;with the behavior I suggested.

zocky commented 6 years ago

@mmis1000 - Yes, that proposal as written disallows destructure, but I don't think that that restriction is actually necessary.

mmis1000 commented 6 years ago

I think there is a similar mechanism actually implemented by Firefox, every constructed object have its own security context info attached, access object property from different context that the caller does not have correct permission always produce an error.
You can send your private data to whoever you want, but others still has no way to read it, the engine will stop you from reading the data that you don't have permission. Reading code that write in that way is easy, as it is just a object. But debugging will be difficult, because its debugger does not say what security context this object is attached to. Just another way of private though.

bakkot commented 6 years ago

@kyranet

In case the First-Class Protocols lands in JavaScript, we may want to be able to use both features together.

The point of the protocols proposal is to define public shared APIs. There wouldn't be any particular relationship to this proposal, regardless of syntax.

@bdistin

private(private.containingClassInstance).a === private.a; // this.#containingClassInstance.#a === this.#a; // seems like it should work (there is no question of what it means as expressed), but due to implementation details using symbols, it just doesn't work

I don't understand what you mean by this. It seem to me that the current syntax works fine:

class Outer {
  #a = 42;
  constructor() {
    class Inner {
      #outerInstance;
      constructor(outerInstance) {
        this.#outerInstance = outerInstance;
      }
      m() {
        console.log(this.#outerInstance.#a);
      }
    }
    const inner = new Inner(this);
    inner.m();
  }
}
new Outer; // prints 42

Also, there are no "implementation details using symbols". This proposal is not specified in terms of symbols. I don't know what that's referring to.

That's a detail I don't necessarily agree with, as there are many times when you should be able to use private as a construct. for(const metaData of private) or for(const metaData of private(other)) for instance with the Symbol.iterator example, and destructuring should be valid.

The fact that this proposed syntax implies this sort of thing is possible is, in my mind, a strong argument against it. I don't think it is a good idea to reify an actual "private data record" object, nor to have other syntax constructs work as if there were one (as for(const metaData of private) or const { x, y } = private surely does).

bdistin commented 6 years ago

I don't understand what you mean by this. It seem to me that the current syntax works fine

If and only if the Inner class does not have it's own #a field. If the Inner class defines it's own #a field, the meaning of #a is changed within the scope of the Inner class, making the Outer #a inaccessible even though you can clearly express this.#outerInstance.#a === this.#a. Doing so would (and currently does in V8) throw an error.

img

Edit: Also I notice that you have only listed versions where private is used bare... If in those cases you had to use private(this) instead, just as you would private(other), does that make a difference in your mind? Or were you only making a short-list?