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

Why are #private fields not source-positional? #328

Open trusktr opened 3 years ago

trusktr commented 3 years ago

I.e. why are they not scoped at the module level instead of at the class level?

This means we can't use #private fields along with class-factory mixins.

This works fine:

class Foo {
    #foo = 123
    test(other) {
      console.log(other.#foo)
    }
  }
}

class Cat extends Foo {}
class Dog extends Foo {}

const c = new Cat
c.test(new Dog) // logs 123.

Now we want to make the Foo class mixable (very beneficial for code organization):

function Foo(Base) {
  return class Foo extends Base {
    #foo = 123
    test(other) {
      console.log(other.#foo)
    }
  }
}

class Cat extends Foo(Object) {}
class Dog extends Foo(Object) {}

const c = new Cat
c.test(new Dog) // ERROR, Cannot read private member #foo from an object whose class did not declare it

Typescript playground example (maybe TS team overlooked this, because there is no type error).

bakkot commented 3 years ago

Because they're different classes. Just because two classes were created from the same source text does not mean they're the same class. It's the same reason that the last line is undefined in

function Foo(name) {
  let x = Symbol();
  return {
    [x]: name,
    name(){
      return this[x];
    },
  };
}

let one = Foo('one');
one.name(); // one

let two = Foo('two');
two.name.call(one); // undefined

Incidentally, https://github.com/tc39/proposal-private-declarations would make it easy to hoist the declaration out of the function, which would make it easy to make this pattern to work.

trusktr commented 3 years ago

Hello @bakkot! Thanks for answering, but you only explained what is happening. I know, that the private fields are scoped to each class instance.

I'm just wondering why wasn't it chosen to be module-scoped? For example, to describe it with WeakMap semantics, the WeakMaps would be at the top level of the module, not inside the mixin functions.

To explain it with Symbols, why was it not decided for it to be like the following:

let foo = Symbol(); // at the top level
function Foo(Base) {
  return class Foo extends Base {
    [foo] = 123
    test(other) {
       console.log(other[x])
    }
  }
}

?

That would be lot more practical for patterns like class-factory mixins. :grinning:

bakkot commented 3 years ago

If you're interested in the history, see https://github.com/tc39/proposal-class-fields/issues/60, especially https://github.com/tc39/proposal-class-fields/issues/60#issuecomment-345540567.

trusktr commented 3 years ago

So why not choose the design that would be more practical for most people (source-positional), and let those people who want particular security guarantees use WeakMap to do whatever they want? Why did we choose to do things opposite?

Now we have disparate and incompatible language features.

ljharb commented 3 years ago

"mixins" aren't a first-class language feature - otherwise class syntax would have some affordance for it.

The linked issue/comment explains some of the barriers to having private fields be reified as first-class values (Symbols or otherwise).

trusktr commented 3 years ago

The fact we can mix functions with classes was a feature of the language before #private fields was.

You always like to play on words only to be in an opposite position. Why do you do that? How can we make progress when you always do that?

The linked issue/comment explains some of the barriers

As I mentioned there, the barriers are for rare use cases, and people in those situations already have WeakMap to solve those rare needs.

It would be great not to make a language feature satisfy those rare use cases at the expense of practicality for majority use cases.

trusktr commented 3 years ago

I've been writing UI code, looking at other people's UI code, and working in Node.js applications for plenty of years now, and from this experience I believe source-positional private fields would have been a lot more practical and more beneficial for all the code bases I've ever worked in.

ljharb commented 3 years ago

The current private fields proposal will compose quite nicely with a future reification, which would cover your use cases.

The design you want would have resulted in no private fields feature at all, which would have prevented your use case as well as every other use case private fields supports. Nobody's disputing the value of "this feature plus one more", but you seem to be suggesting that adding private fields as-is - the foundation for the feature you want - is somehow indicative of use cases being ignored or somehow obstructed.

Private fields are analogous to a closed-over WeakMap per field. As such, absent the proposal, this code is roughly equivalent to your second example in the OP:

```js function Foo(Base) { const fooField = new WeakMap(); return class Foo extends Base { constructor(...args) { super(...args); fooField[this] = 123; } test(other) { if (!fooField.has(other)) { throw new TypeError('Cannot read private member #foo from an object whose class did not declare it'); } console.log(fooField.get(other)); } } } class Cat extends Foo(Object) {} class Dog extends Foo(Object) {} const c = new Cat c.test(new Dog) // ERROR, Cannot read private member #foo from an object whose class did not declare it ```

If you wanted both Cat and Dog to have access to the same WeakMap, you'd have to wrap them in a scope such that Foo, Cat, and Dog all shared lexical access to the same map. You can currently do this without private fields like so:

```js const { Foo, Dog, Cat } = (function () { const fooField = new WeakMap(); function Foo(Base) { return class Foo extends Base { constructor(...args) { super(...args); fooField[this] = 123; } test(other) { if (!fooField.has(other)) { throw new TypeError('Cannot read private member #foo from an object whose class did not declare it'); } console.log(fooField.get(other)); } } } class Cat extends Foo(Object) {} class Dog extends Foo(Object) {} return { Foo, Dog, Cat }; }()); const c = new Cat c.test(new Dog) // logs 123 ```

Doing it with private fields is a bit more involved; writing it out here wouldn't help the discussion :-)

It would quite obviously be much more ergonomic to have reified private name values of some kind, and I hope we do get them someday. However, the lack of them now doesn't mean your use cases were "excluded" - it just means they aren't yet ergonomically satisfied.

rdking commented 3 years ago

Yeesh. Why does everyone choose the hard approach? There's a 1 line fix to the above code to give @trusktr what he wants.

function Foo(Base) {
  return class Foo extends Base {
    #foo = 123
    test(other) {
      console.log(other.#foo)
    }
  }
}

var FooObj = Foo(Object);
class Cat extends FooObj {};
class Dog extends FooObj {};

const c = new Cat
c.test(new Dog) //logs 123

The whole and sole of the problem is that each call to Foo() generated a new instance of the class definition. So each base class became distinct despite having the exact same shape. There are exactly 2 generic solutions to this. The first is the solution above. The second is to alter Foo to add in memoization so that it is a pure function.

ljharb commented 3 years ago

@rdking ha, thanks. I avoid mixin patterns, so your better approach didn’t occur to me.

rdking commented 3 years ago

@ljharb Np. Thing is, I don't use mixins either. I tried them back in the early ES5 days, but grew to hate them quickly. What's more, the so-called "mixin" approach above is more akin to templates or generics in other class-supporting languages. As for things being source positional, in my experience, if you want things to be source positional but they're not, there's usually another approach that will get you what you want.

trusktr commented 3 years ago

@rdking @ljharb I know I can do what I want with WeakMaps. I just feel that people with more obscure cases can also do the more obscure things with WeakMaps while most other people can just have source-positional simplicity.

I'm not familiar: what does `"future reification" and "reified private name values" mean?

@rdking I know about the approach you suggested, but that only work in that case. This doesn't:

function Foo(Base) {
  return class Foo extends Base {
    #foo = 123
    test(other) {
      console.log(other.#foo)
    }
  }
}

class One { ... }
class Two { ... }

class Cat extends Foo(One) {};
class Dog extends Foo(Two) {};

const c = new Cat
c.test(new Dog) // Error

@ljharb To show what I mean by source positional, only Foo's scope needs to have it, and we don't need to wrap all the classes:

const { Foo } = (function () {
  // suppose this is module scope

  const fooField = new WeakMap();

  function Foo(Base) {
    return class Foo extends Base {
      constructor(...args) {
        super(...args);
        fooField.set(this, 123);
      }

      test(other) {
        if (!fooField.has(other)) {
          throw new TypeError('Cannot read private member #foo from an object whose class did not declare it');
        }
        console.log(fooField.get(other));
      }
    }
  }

  return { Foo };
}());

class One {}
class Two {}

class Cat extends Foo(One) {}
class Dog extends Foo(Two) {}

const c = new Cat
c.test(new Dog) // logs 123, yay.

I'm curious about the "reified private name" idea, as I'm not sure what that means.

trusktr commented 3 years ago

Oh, I forgot to say: I don't see why elevating private fields to module scope means we can't ship them. It merely swaps the use cases that the feature is available for to those where it currently isn't available (f.e. mixins) which would benefit more people. It is always possible to add a new feature with the per-class privacy later too, to satisfy more unique and rare cases.

rdking commented 3 years ago

Think of the semantics. A "private" member is one that can only be accessed by its owner. If you "elevate" that from class scope to module scope, you've essentially just got a module variable that's constrained to it's closure. If you're only considering "elevating" the name of the "private" member, you've essentially just got a module-scoped Symbol. Both of those features already exist.

rdking commented 3 years ago

I know about the approach you suggested, but that only work in that case. This doesn't: (example omitted)

You do realize that you're asking 2 different classes to have access to each other's privates, right? That should never work... by definition. Just because you used a factory to template your class generation doesn't and shouldn't imply that they're somehow going to have access to each other's private members. What you're looking for is something very different than "private".

trusktr commented 3 years ago

Think of the semantics. A "private" member is one that can only be accessed by its owner. If you "elevate" that from class scope to module scope, you've essentially just got a module variable that's constrained to it's closure. If you're only considering "elevating" the name of the "private" member, you've essentially just got a module-scoped Symbol. Both of those features already exist.

I would disagree, because this,

const foo = Symbol()

class Foo {
  [foo] = 123
}

and

const foo = new WeakMap

class Foo {
  constructor() {
    foo.set(this, 123)
  }
}

are not the same in terms of discoverability (the value in the first example can be discovered by external code, while this is not true in the second.

You do realize that you're asking 2 different classes to have access to each other's privates, right?

Actually, I'm not! Just like transpiler tricks in tools like Babel, this

class Foo {
  #foo = 123
}

class Bar {
  #foo = 123
}

would effectively turn into the equivalent of the following at "compile time" (or if a transpiler was polyfilling it):

const foo_UUID = new WeakMap

class Foo {
  constructor() {
    foo_UUID.set(this, 123)
  }
}

const foo_UUID2 = new WeakMap

class Bar {
  constructor() {
    foo_UUID2.set(this, 123)
  }
}

The actual variables would never be visible to userland code, they would never collide, and they are not shared across classes (Foo's #foo is accessible only in Foo, and Bar's #foo is accessible only in Bar).

They would be source-positional.

trusktr commented 3 years ago

Sidenote, the reason I said "moved to top-level module scope" is because that's the only way in which transpiler (like Babel or Buble) would be able to polyfill it.

In reality, they can be even moved to the top level module-graph scope (the same place where exported vars and functions get hoisted to. A bundler (like Rollup or Webpack) can hoist anything to the top of the module graph (of a bundle).

It doesn't matter where they go, so long as they are in a scope that is high-enough above the class definitions so that the field store is created only once per class definition. The references to them (in the internal engine implementation) are unique by whatever means (like transpiling to variables with unique IDs).

rdking commented 3 years ago

I would disagree, because this, {omitted} and {omitted} are not the same in terms of discoverability (the value in the first example can be discovered by external code, while this is not true in the second).

I get that the difference between using Symbols and "source positional" private names would be that private names wouldn't leave a discoverable trace on the object. I omitted that point in my original response. However, everything else I said still stands. The use of private names from a module-level scope still doesn't satisfy the typical use case of "private" members. Hence elevation to module scope to make it "source positional" would fail the definition of "private". It would be a completely different kind of mechanism. That was my main point.

You do realize that you're asking 2 different classes to have access to each other's privates, right? Let me decompose your original example to show you what I mean. Here's your example...

function Foo(Base) {
  return class Foo extends Base {
    #foo = 123
    test(other) {
      console.log(other.#foo)
    }
  }
}

class One { ... }
class Two { ... }

class Cat extends Foo(One) {};
class Dog extends Foo(Two) {};

const c = new Cat
c.test(new Dog) // Error

The equivalent WeakMap version of this code has Foo looking like this:

function Foo(Base) {
   let foo = new WeakMap();
   return class Foo extends Base {
      constructor() {
         private.set(this, 123);
      }
      test(other) {
         console.log(foo.get(other));
      }
   }
}

Every new invocation of Foo has a different WeakMap, so of course you'll get an error at c.test(new Dog) since there are no Dog instances in the Cat's WeakMap. To paraphrase what I said before, if you want some set of classes to share the same WeakMap so they can access each other's private data, you're talking about a concept fundamentally different than the common use case of "private". Essentially, you're looking for something akin to "friend"(C++) or "internal"(Java). That's a different concept than "private".

trusktr commented 3 years ago

The use of private names from a module-level scope still doesn't satisfy the typical use case of "private" members. Hence elevation to module scope to make it "source positional" would fail the definition of "private".

Whose definition of private are you talking about? It seems you're talking about the definition of private as seen in this proposal. The definition is subjective. There is no one definition.

JavaScript doesn't exactly match Java, therefore by your logic, Java must be incorrect, despite that it came before JavaScript, but we know this is not true.

The definition of privacy can be whatever we want it to be so long as it is similar to concepts (plural) of privacy that most people would find agreeable

I've described which form of privacy, in my opinion, would be more useful for more people, and in that definition private fields are source-positional.

paraphrase what I said before, if you want some set of classes to share the same WeakMap so they can access each other's private data, you're talking about a concept fundamentally different than the common use case of "private". Essentially, you're looking for something akin to "friend"(C++) or "internal"(Java). That's a different concept than "private".

Like I described, that's subjective and there is no one definition of privacy.

Under the above definition of privacy with source-positional fields, more people would be able to find it useful in more cases, and less people would need to resort to using WeakMap.

Your definition of privacy could still be achieved with closed-over WeakMaps had #private been source-positional.

ljharb commented 3 years ago

There is only one definition that’s relevant here, because it already exists in JavaScript: reachability, as determined solely by lexical scope (initially, of course; anything can choose to expose a private thing).

rdking commented 3 years ago

@trusktr When I say "private", I mean as defined by most languages that support the concept (not including ES which only finalized its acceptance a few days ago), which is invisible and inaccessible from outside the defining class. This of course does not preclude the language from offering features that can weaken this definition (like reflection in Java).

guidobouman commented 3 years ago

Under the above definition of privacy with source-positional fields, more people would be able to find it useful in more cases, and less people would need to resort to using WeakMap.

That's also your subjective experience & opinion. I've rarely used class mixins, and hate using them. Each company I've worked at and each client I worked with, we tried to not use these. Just like the concept of HoC's, they're not easy to understand in the long run. Which is then my personal experience and opinion. What I'm trying to say: None of us here can be sure what percentage of all JavaScript engineers expect privates to be shared among new class definitions within the mixin.

The only thing we can lean on is the fundamentals of the language, in this case scope. I think we can all agree that the following code works as expected.

function foo() {
  const bar = 'baz';
  return function log() {
    console.log(bar);
  }
}

log = foo();
console.log(bar); // logs: undefined
log(); // logs: "baz"

Then why would we expect private members to be hoisted out of this scope to share? Even if it's just for the scope of the class instances? It would trip up new engineers learning the language.


Actually my personal opinion is that shared variables for instances of the same class definitions is also unexpected, but that's not part of the topic here.