Skip to content
This repository has been archived by the owner on Jan 25, 2022. It is now read-only.

What about protected methods, better declaration of private fields, private async functions, and destructuring? #59

Closed
eddyw opened this issue Nov 14, 2017 · 35 comments

Comments

@eddyw
Copy link

eddyw commented Nov 14, 2017

I opened an issue in tc39/proposal-private-methods#21, however I just realized that further discussion about the topic should be here. So, I will put again what I wrote there:

How could we allow a private field to be shared only when subclassing but not when it's an instance? Why not protected fields were not proposed? Have you thought if in the future somebody does propose them? (we are running out of characters)

I know it was discussed many times why # was chosen (I read the FAQs). It's ugly but I get it. However, that doesn't mean there cannot be another way of declaring them. What about this?:

class A {
  static prop0 = 123
  private prop1 = 123
  prop2 = 123 // public property
  method() { // public method
    console.assert(new.target.prop0 === 123) // static
    console.assert(this.#prop1 === 123) // private <- accessing changes
    console.assert(this.prop2 === 123) // public
  }
}
console.assert(A.prop0 === 123) // static (devs know it's not A.static.prop0)

Declaring them that way makes more sense besides it doesn't break the current proposal's idea.
However, if somebody comes up with "this may be confusing..." or similar, devs do not seem confused using static [prop], they know how to access a static var (new.target[propName]). In a similar way, private [prop] won't make it more confusing, devs will know to access with this.#[prop].

An example where it will look better:

class A {
  #$ = jQuery
  #_ = lodash
  method() { this.#$(...) }
}
class A {
  private $ = jQuery // You still have to access with this.#$
  private _ = lodash // You still have to access with this.#_
  method() { this.#$(...) }
}

What about async functions?

class A {
  #async method() {}
  // or
  async #method() {}
}

How will it behave with destructuring. For instance (with no Private fields):

class A {
  prop = 10
  constructor() {
    const { method } = this
    method()
  }
  method() { console.log('Not bound to "this" but works') }
}

With private fields:

class A {
  prop = 10
  constructor() {
    const { #method } = this // or { method } = this# ???
    #method() // ??? or not allowed, throws? (but what)
  }
  #method() { console.log('Not bound to "this" but works') }
}

Are private fields not enumerable?, I mean, is it possible to get all the names of private fields from inside a class? If they are not enumerable, how does it affect applying a function decorator and changing the descriptor to enumerable: true? got it :P

How does it act with Proxy?

class MyProxy extends Proxy {
  constructor() {
    return new Proxy(this, {
      get(target, propName) {
        if (typeof target[propName] === 'function')
          return target[propName]
        console.log(`Accessing [[${propName}]]`)
      }
    })
  }
}
class A extends MyProxy {
  one = 5
  prop = 10
  #prop = 20
  method() {
    this.prop
    this.#prop
    this.#method()
  }
  #method() {
    this.one
  }
}
new A().method()
// outputs: Accessing [[prop]]
// outputs: ??? Accessing [[#prop]]
// outputs: ??? Accessing [[one]] <- as a result of calling this.#method, does the trap still work?

So far I have more questions that actually use cases since I'm not pretty sure how much does it affect everything we know about JS. Now we have block scope vars and function scope vars. Maybe explaining how to categorize private block/function scope vars will clarify better all questions.

@ljharb
Copy link
Member

ljharb commented Nov 14, 2017

  • having a different way to declare and use private fields would be confusing.
  • Proxies do not forward private fields, just like they do not forward internal slots currently.

@eddyw
Copy link
Author

eddyw commented Nov 14, 2017

I disagree with your first point.
Again, same example:

class A {
  static prop = 123
  method() {
    new.target.prop // <-- Is it less confusing than this.#someProp?
    A.prop // <-- Do devs wonder why not A.static.prop? or this.static.prop?
  }
}

That's mainly the reason. Devs are not dumb, that's my point.

class A {
    static prop = 1
    static #prop = 1
    #prop = 1
    static method() {
        return this.prop + this.#prop
    }
    other() {
        // Btw, can we access new.target.#prop?
        return new.target.prop + this.#prop
    }
}

Accessing static fields is already different and devs get it. There is no confusion why not using this.static.prop for instance.
Besides, private is already a reserved word. Also, one question I asked was about protected fields or possible future proposals that add ... a something. Do they need to come up with a new not used character? Let's suppose a new proposal appears:

class A {
    static prop = 1
    #prop = 1
    ℃newKindOfProp = 1 // <- ℃? because it's another random char available?
}

I get why this.#prop. However, I don't get why the obsession of uglifying declaration. You say it's confusing, for who? People coming from other languages get what private means. Even somebody without knowledge of JS could infer this code:

class A {
  private a = 0
  method() { return this.#a }
}

However, what this one tells you?

class A {
  #a = 0
  method() { return this.#a }
}

The code stops being auto descriptive, it confuses everybody at first sight, it's inconsistent with how static fields are declared (I mean, I would ask, "why 'static' and not some random symbol then?").

At the end, even this alternative way is better:

class A {
  private #a = 0
  private #method() {}
  a = 0
  method() {
    return this.#a
  }
}

At least it's clear. It's not redundant to write private and #, it simply tells the developer that private fields are accessed in different way than public fields. The code speaks, describes itself as it should be.

@jridgewell
Copy link
Member

class A {
  private a = 0
}

That will lead people to believe it's accessed by this.a, as in every other language. That's very problematic.

class A {
  private #a = 0
}

Will lead people to thinking they could use #a = 0 to declare a public field with #a which is again problematic.

@eddyw
Copy link
Author

eddyw commented Nov 15, 2017

I buy your first argument against private a = 0, ok.

Your second argument is just wrong. Why?

class A {
  validProp = 0
  $validProp = 0
  _validProp = 0
  #validProp = 0 // <- (I'd think) if method begins with $ and _, so # is also public, right?
  #$validProp = 0
  #_validProp = 0
  #$#_validProp = 0 // <- Why people won't think that this is possible seeing previous declarations?
  static $validProp = 0
  static _validProp = 0
  static #validProp = 0
}

Could you clarify how this is less confusing or problematic?
How it works now? public fields are declared without the public keyword (properties and method). static methods are declared with the static keyword (properties and method). Why private fields need to be defined the same way public methods are?

That's the reason why I think the hate of using private keyword is just a mere obsession without a valid argument.

Once again, the static keyword doesn't seem to confuse people how to access a static field from inside a class or outside from it.

I don't like so much the idea of this proposal. However, I understand what it tries to solve and prioritizes library devs over consumers or users of libraries. So, a dev building a library is not a dummy learning JS, they will get what private means, the same way they get what static means and how to access it.

How wrong can something simple as this be:

class A {
  validProp = 0
  $validProp = 0
  _validProp = 0
  private #validProp = 0
  private #$validProp = 0
  private #_validProp = 0
  static $validProp = 0
  static _validProp = 0
  static private #validProp = 0
}

The $ and _ chars are widely used. $ with jQuery and in some methods inside several libraries (e.g: Mongoose query.$where and doc.array.$shitf()). '_' is used mainly by two popular libraries (lodash, underscore) and sometimes used to denote that a variable is not used (_, arg2) => {}
To add, now many people use _method or _prop in libraries. Most of the times for fields that are supposed to be private which this proposal should fix. However, let's say once this proposal is implemented in ..let's say NodeJS.., I would like to display a warning when people access _field stating that accessing internal properties would be unavailable in future releases:

class A {
  #privateField() {
    // do your stuff
  }
  /* @deprecated */
  _privateField() {
    displayWarning('Accessing internal properties would be unavailable in future releases')
    this.#privateField()
  }
}

That looks just like renaming my method. It isn't obvious what the code does.

I would like to know what kind of "people", you refers to, that would be confused that much by adding a keyword that will tell a var is private.

@Davilink
Copy link

i guess i will continue to use Typescript, the compiler will translate the private -> to the # and what so ever other symbol they (TC39) will decide to use . Because for me : private, protected, public, static are more meaningful than #.

@eddyw
Copy link
Author

eddyw commented Nov 23, 2017

@Davilink the private # is somewhat different from the ts private. For instance, the # private can be used in object literals (as I understood reading the docs that are dispersed everywhere):

const obj = {
  #prop = 0
  get prop() {
    return this.#prop
  }
}

That is also supposed to avoid collision with field names between public and private fields. Here tc39/proposal-decorators#33 (comment) I mentioned another idea I had for accessing private fields, such as:

class {
  static prop = 0 // static
  private prop = 0 // private
  prop = 0 // public
  method() {
    new.target.prop // refers to static field
    new.self.prop // would refer to private field
    this.prop // would refer to own
  }
}

That doesn't cause collision between fields with the same name. It also avoids introducing a new symbol (#). However, that leaves out private static fields because the new operator is only available when a class is initialized with new. If anybody knows how to fix that on top of this idea, then we could suggest a change. However, as for now, I have no other ideas ...but to accept the # ugly char and the ugly declaration as well.

@bakkot
Copy link
Contributor

bakkot commented Nov 23, 2017

@eddyw

For instance, the # private can be used in object literals

This is not part of the current proposal, no.

It's true that TypeScript's private fields don't prevent collisions (and also don't provide any actual privacy), though, yes.

@eddyw
Copy link
Author

eddyw commented Nov 24, 2017

@bakkot I guess the docs are not updated everywhere. I guess that simplifies things. I would still prefer to access private fields in a different way.. or declare them in a different way.

@littledan
Copy link
Member

@eddyw Which doc still uses private in object literals? I was trying to clarify that this is not supported.

About declaring and using private fields differently, isn't the # a difference?

@eddyw
Copy link
Author

eddyw commented Nov 24, 2017

@littledan here https://github.com/tc39/proposal-decorators/blob/master/DETAILS.md
It's so difficult to get around the docs and those many repos.

The thing is that the # char is too much of a difference. For instance, it's so weird to think the number sign now means private in JS: https://en.wikipedia.org/wiki/Number_sign

class {
  #color = '#aabbcc'
  #smallNumberSign = '&#65119'
  #match = /\#([^#]+)$/g
  #hash = location.hash
}

Besides, it is not the first time the # is used in JS:
https://developer.mozilla.org/en-US/docs/Archive/Web/Sharp_variables_in_JavaScript
Which previously had other meaning other than private.

So, based on the idea of new.target which is a meta property. Wouldn't adding a new meta property to an operator be better?

class {
  private prop = 0
  method() {
    in.self.prop // private .. or
    in.this.prop
  }
}

Why? It can't be used as a field name and ..

The in operator returns true if the specified property is in the specified object or its prototype chain. - MDN

So a meta property there makes kind of sense. It's kind of in self/this get prop.

This would also make possible other things. For instance, to suggest in the Reflect Metadata proposal (in the future) to add possibility to access metadata using in.meta.[metadataKey]:

class C {
  @Reflect.metadata('key', 'value')
  method() {
    in.meta['key']
  }
}

Anyway, the thing is. This proposal struggles where to put private fields without causing collision with public fields. Future proposals may also have this issue and I doubt we can handle having another new char similar to # added to this..

@littledan
Copy link
Member

Thanks for pointing at that file; I will update it to move private in object literals to the future work section. In general, you should only need to look in this repository to understand this proposal, but I think the repo needs a big "detailed rationale" blog post to tie it together. On my to do list.

We have covered the two points here (# is ugly, and using metaproperties which skip this) in several other bug threads already.

@eddyw
Copy link
Author

eddyw commented Nov 30, 2017

@littledan I couldn't really find a bug thread about metaproperties. I would really like to read about it. Were they covered somewhere in this repo? A Google search doesn't return anything useful.

@alphanull
Copy link

+1 for protected. +1 for using proper keywords instead of "#". If you are doing heavy subclassing, private alone is almost useless, so protected is needed.

@rattrayalex
Copy link

What about both?

class Foo {
  private #x = 1;
  protected #y = 3;
  get x() { return #x; }
  set y(newVal) { #y = newVal; }
}

Advantages:

  • Educates a reader about the language feature #.
  • Allows for protected and private to coexist.
  • Makes it more obvious that you will always need to prefix access to the private field with #.

Downsides:

  • More verbose, redundant.
  • May interact strangely with TypeScript, though I assume this could be resolved.

@bakkot
Copy link
Contributor

bakkot commented Dec 10, 2017

@rattrayalex See previously and previously.

@rattrayalex
Copy link

Thanks! Sorry for the repost.

@mbrowne
Copy link

mbrowne commented Dec 31, 2017

Much of this was discussed previously in this thread (I realize it's very long).

I think one of the best points that was brought up (by @bakkot) is that the best way to figure out good ways of doing access control in JS is to provide user-land features that the community can experiment with, and use those to inform future proposals. Hard private fields are an exception to this because there's no way to provide truly private fields with decorators or any other proposals currently on the table. If I understand correctly, the introduction of the # modifier would not preclude the introduction of other keywords in a future proposal:

class Foo {
  #hardPrivate
  protected a
  friend b
  internal c
  softPrivate d
}

(These are just examples of course; naming, meaning, and feasibility of the specific access levels would need to be discussed.)

@littledan
Copy link
Member

littledan commented Jan 2, 2018

@mbrowne I agree that this proposal does not preclude any of those. The only thing it does, due to the choice to have ASI, is say that those cannot have a newline between that token and the field or method it is modifying. This is already the syntax for async methods, so I don't think the restriction is so bad.

@rdking
Copy link

rdking commented Jan 13, 2018

@mbrowne
Actually, it's possible to do everything @littledan wants even in ES6. The more I've thought about what he's trying to accomplish, the more I'm sure it can be done as a mere extension of the syntactic sugar of class instead of a fundamental change to how the language works. Here, I again suggested a change to the undesirable syntax. However, this time I did so with an eye to how it could be de-sugared, and the requirements for implementing his proposal using my syntax. What's more is that protected could also be fairly easily implemented following the same line of thinking. In fact, I'm working on a re-implementation of my Class.js (with partial documentation here) library to treat private fields using this private names approach. I've tried implementing this approach to classes by hand and found it a bit simpler to manage than my original approach.

@doodadjs
Copy link

doodadjs commented Jan 13, 2018

Damned, "public", "private" and "protected" are already reserved keywords. What is the point to use a sigil for "private" to just break "protected" and potentially others like "friend" ?

@rdking
Copy link

rdking commented Jan 14, 2018

If you look at the suggestion I linked to before, you'd notice I want to use the already reserved words and the existing syntax to accomplish everything. Here's a summary of what I'm suggesting.

  • Private field declarations: private [[name]] = [[value]]; -- creates a Symbol within the context of the owning constructor function named [[name]], flags [[name]] as private, and creates a [[PrivateScope]] object if there isn't one already. Also, the following assignment in the constructor: [[PrivateScope]][ [[name]] ] = [[value]];.
  • Private method declarations: private [[name]]([[parameters]]) {...} -- syntactic sugar for a field declaration where [[value]] is a function. Any function private is bound to this using
    Function.prototype.bind at the time of assignment.
  • Private field access: this[ [[name]] ] -- because [[name]] is a Symbol, this.[[name]] and this["[[name]]"] are automatically incorrect. Such access would instead return the "public" property. this[ [[name]] ] is translated to [[PrivateScope]][ [[name]] ] under 2 conditions:
    1. [[name]] is currently a Symbol in the scope flagged as private.
    2. The [[PrivateScope]] associated with [[name]] is accessible in the current scope.

As for protected, because of the way class instance prototype chains are set up, it becomes very easy to add support for protected members. Here's the summary:

  • Protected field & method declarations: Same as for private declarations, but using the protected keyword and having the following additions -- Additionally, flags [[name]] as protected.
  • Protected field access: Same as for private declarations.

To make use of such protected members, after processing the extends [[base]] portion of a class declaration:

  • Creates a [[ProtectedContext]] object and adds to it all private names of [[base]]'s context that are also flagged as protected.
  • Inserts [[ProtectedContext]] behind the context of the derived class within its context chain.

In the constructor, when super returns:

  • A new [[PrivateScope]] object is created with the [[PrivateScope]] of [[base]] as its prototype.

With only that much, protected members would be fully functional.

@rdking
Copy link

rdking commented Jan 14, 2018

@doodadjs Just a note...
For me, friend is in the same category as goto. Using it is usually a good sign that you didn't give enough thought to your design.

@doodadjs
Copy link

@rdking I don't know for "friend", but that's potentially something that can come up. And it has no link with "goto/gosub" in BASIC ?

@doodadjs
Copy link

@rdking C# Internal

@rdking
Copy link

rdking commented Jan 14, 2018

@doodadjs I don't really understand how internal would be a good idea as a keyword in ES, it doesn't do anything that makes sense for ES. Consider that languages supporting such a concept do so because they support things like packages for Java and assemblies for C#. For ES, the closest concept is a module, but every implementation of modules in Javascript already make the context inside the module private to the module and public to anything declared in the module.

/*ExampleModule.js*/
var moduleCommon = 0;

export class A {
  constructor() {
    console.log(`moduleCommon: ${++moduleCommon}`);
  }
}

export class B {
  constructor() {
    console.log(`moduleCommon: ${++moduleCommon}`);
  }
}

/* ExampleTest.js */
import {A, B} from "ExampleModule";
new A(); // => 1
new B(); // => 2

So I'm not sure what there is to gain from an internal keyword.

@mbrowne
Copy link

mbrowne commented Jan 15, 2018

One possible use case for internal is in scoped npm packages, where you might want to have related code in separate modules and share some things internally between them without making them globally public...but practically that may not be feasible given how ES modules work. Also, I think Apple makes a valid case in this blog post for internal is in some ways preferable to protected ('protected' meaning accessible to the class and subclasses) - and that you no longer really need protected:
https://developer.apple.com/swift/blog/?id=11

However, I understand the argument for protected as well, and while I would want to encourage everyone to not limit themselves to inheritance for polymorphism, I suppose it would still be good to provide protected for those use cases that can benefit most from it.

@rdking
Copy link

rdking commented Jan 15, 2018

I'm not saying that something like internal should not be implemented. What I am saying is that it shouldn't be an ES keyword. Modules are not an ES concept. They are something implemented by ES environments and libraries (like web browsers, node, and the commonjs spec). We should look to those things to support the concept of internal instead of burdening the language with something outside its scope.

@mbrowne
Copy link

mbrowne commented Jan 15, 2018

Well you would still need something in the code that the module system could pick up on in order to be able to mark some class properties as internal and others as globally public, private to the module, etc. I suppose decorators would be one option (it could be a decorator with an empty implementation since it wouldn't be implemented in userland)...or special comments.

@ljharb
Copy link
Member

ljharb commented Jan 15, 2018

(fwiw, modules are an ES6 concept - everything is implemented outside of ES, but the spec does mandate the existence of, and semantics of, modules)

@rdking
Copy link

rdking commented Jan 15, 2018

@ljharb I just noticed that in the spec myself last night.

There's still the issue that, given the usage suggested by @mbrowne, some concept of a "package" would have to be developed to limit the sharing of internal elements among modules. There's no harm in doing so as all such implementations would exist outside the engine. However, that also means that this proposal is still not a good space for discussions of adding internal.

@mbrowne
Copy link

mbrowne commented Jan 15, 2018 via email

@eddyw
Copy link
Author

eddyw commented Jan 17, 2018

So, I actually couldn't find any threads that cover or discuss why meta-properties are not a good option to get around the ugly syntax and, at the same time, allow future proposals to be considered (protected, friends, [and what not]). For instance:

class A {
  private prop = 123
  method() {
    in.self.prop = 234
    in.this.prop = 234
  }
}

Or any other way of extending in. We already have new.target for accessing the constructor from inside the instance of a class. However, new.target only works on the instance and not from inside the static methods, so new.privateProp is not an option.

On the other hand, if in meta-properties are a kind of this for special purposes, then it's possible to extend and allow future proposals as well. Besides that in is a reserved word and it's also used to iterate through an object's properties in a for loop, so it kind of makes sense.

There is a confusing thing as well. # is supposed to be a "hard" private which means there would be a difference between a (soft?) private which could be implemented with function decorators. So...

class A {
  @private prop = 123 // let's pretend deco. fn. private exists
  @friend prop = 123 // there's collision between names
  #prop = 123 // no collision
}

That means, even though hard private is supposed to have a unique name, for friend, protected, or whatnot, it doesn't solve that problem if using function decorators. Using meta-properties could help:

class A {
  #prop = 123
  private prop = 123
  protected prop = 123
  friend prop = 123
  method() {
    in.#prop // hard private
    in.private.prop
    in.protected.prop
  }
}

On the other side, I don't believe we really need all of those being implemented (e.g: protected, friend, package, etc). So, there is a reserved word for private, and if soft private fields could be implemented through function decorators. Then, I don't see why the private keyword can't be used for hard private. Kind of:

class A {
  private prop = 123 // hard private
  @private prop = 123 // your own implementation of soft private fields with fn decorators.
  method() {
    in.this.prop = 234 // hard private
  }
}

Anyways, just my thoughts. I'd really want to know if meta-properties were discussed before and if there is a thread about it. I couldn't find it for some reason.. (maybe it's in one of the many repos?)

@adrianheine
Copy link

One thing I don't like about in.this.prop is that there is no reference to this anymore. in(this).prop would work around that.

@littledan
Copy link
Member

I share @adrianheine 's concern here. For reasons described in other threads, I'd rather not switch to @rdking 's [] proposal. The discussion here leads me to conclude that we should stay with the current proposal.

@bakkot
Copy link
Contributor

bakkot commented Jan 17, 2018

@eddyw Meta-properties and similar proposals are discussed in tc39/proposal-private-fields#14, notably here.

Briefly: you'd still need a new kind of declaration, I think, since private x for declaration will strongly imply this.x for access to people coming from many other languages, and you also need to some syntax to access private fields of objects other than this, which is a goal of the proposal.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests