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

fix for "mallory.equals(alice)" #146

Closed lifeart closed 6 years ago

lifeart commented 6 years ago

https://github.com/tc39/proposal-class-fields/blob/master/PRIVATE_SYNTAX_FAQ.md#how-can-you-model-encapsulation-using-weakmaps

To prevent overriding we need to defineProperty instead of plain assign

const Person = (function(){
  const privates = new WeakMap;
  let ids = 0;
  return class Person {
    constructor(name, makeGreeting) {
      this.name = name;
     // instead of privates.set(this, {id: ids++, makeGreeting}); do:
      const config = {};
      Object.defineProperty(config, 'id', {
        value: ids++,
        writable: false,
        enumerable: false,
        configurable: false
      });
      Object.defineProperty(config, 'makeGreeting', {
        value: makeGreeting,
        writable: false,
        enumerable: false,
        configurable: false
      });
      privates.set(this, config);
    }
    equals(otherPerson) {
      return privates.get(this).id === privates.get(otherPerson).id;
    }
    greet(otherPerson) {
      return privates.get(this).makeGreeting(otherPerson.name);
    }
  }
})();
let alice = new Person('Alice', name => `Hello, ${name}!`);
let bob = new Person('Bob', name => `Hi, ${name}.`);
alice.equals(bob); // false
alice.greet(bob); // === 'Hello, Bob!'

let mallory = new Person('Mallory', function(name) {this.id = 0; return `o/ ${name}`;});
mallory.greet(bob); // === 'o/ Bob'

Before:

mallory.equals(alice); // true. Oops!

After:

mallory.equals(alice); // false. Expected.
nicolo-ribaudo commented 6 years ago

I don't get what the issue is. When simulating private fields, you should use one weakmap per field and this is the instance, not a privates object.

StGeass commented 6 years ago

But this creates a problem when even the internal methods of the class do not have the ability to affect private fields, no?

Maybe we could just bind the context to all functions entering the private space from outside of the class?

privates.set(this, { 
  id: ids++, 
  makeGreeting: isFunction(makeGreeting) ? makeGreeting.bind(this) : makeGreeting,
});
littledan commented 6 years ago

Agree with Nicolo, such an implementation doesn't provide the privacy you want.

nicolo-ribaudo commented 6 years ago

You can affect private fields using this.#id inside internal class methods.