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

Accessing private fields without the need of a sigill #52

Closed
dfahlander opened this issue Sep 14, 2016 · 28 comments
Closed

Accessing private fields without the need of a sigill #52

dfahlander opened this issue Sep 14, 2016 · 28 comments

Comments

@dfahlander
Copy link

dfahlander commented Sep 14, 2016

As discussed in #14, many people advocates for using the private keyword for declaring the private fields. But that discussion has went on further into how we should access the properties. It seems that the sigill (# or @) is something that could also be questioned for accessing the fields. So let's bring that up in this separate issue.

@eggers came with a very interesting solution that I've been examiniting a little. It allows private access without a sigill and without changing the property lookup algorithm in any way. My own working example can be found here - uses a WeakMap to store the properties. You need a modern browser to open it!

This way, it would be possible to:

  • Access private properties precisely the same way as public ones.
  • Shared namespace between private and public properties (as in other languages) (don't know if this is good or bad though)

...but also:

  • Allow this-less access to private properties from class lexical scope (optional 'addon' to the idea).
  • Let developer choose when to use this.x and just x.

EDIT: this-less access would also make optimization possible because it would be obvious at parse time what property is being accessed and the parser already knows that it is allowed and can omit having to check against the white-list

class A {
    private buffer, pos;

    method () {
        buffer[pos++] = 'hello'; // instead of this.buffer[this.pos++] = 'hello'.
    }
}

Another sample:

class Greeter {
    private greeting;

    constructor (greeting) {
        this.greeting = greeting;
    }

    greet () {
        return greeting; // Or this.greeting
    }
}
@zenparsing
Copy link
Member

Implies a run-time cost on all private property access. Non-starter.

@zenparsing
Copy link
Member

Apologies, that we dismissive. Continue on! : )

@domenic
Copy link
Member

domenic commented Sep 14, 2016

It really would be nice if people read the frequently asked questions though: https://github.com/tc39/proposal-private-fields#frequently-asked-questions

@yanickrochon
Copy link

@domenic the two FAQ topics point to very long and exhaustive debate on the matter. There are people that actually try to suggest ideas in separate threads to help make things easier to follow.

As far as this ides goes, it is actually compatible with Crockford's Private Members in JavaScript. Where

class A {
    private buffer, pos;

    method () {
        buffer[pos++] = 'hello'; // instead of this.buffer[this.pos++] = 'hello'.
    }
}

behaves somehow similar to

function A() {
  var buffer, pos;

  this.method = function method() {
    buffer[pos++] = 'hello'; // instead of this.buffer[this.pos++] = 'hello'.
  };
}

Except that method would not be part of A.prototype.

@domenic
Copy link
Member

domenic commented Sep 14, 2016

I disagree that starting new threads to rehash old grounds is in any way an improvement.

@dfahlander
Copy link
Author

@domenic I've understood the reasons. But this idea will not affect property access algorithm at all. So that FAQ does not apply.

@zenparsing It's one single check against a Set. And it's only done from within the getter and setter functions. And in 99% of the cases, private properties can be accessed without this, which makes it possible to shortcut the access at parse time directly to the backing WeakMap.

@domenic
Copy link
Member

domenic commented Sep 14, 2016

If you add a hash-set lookup to every property access in JavaScript, you're going to make the web much slower. This is indeed affecting the property access algorithm, and this is the same argument as before. It's the same non-starter as before.

But indeed, continue on!

@dfahlander
Copy link
Author

dfahlander commented Sep 14, 2016

@domenic Not affecting any existing property access.

@yanickrochon
Copy link

Seriously, what is the difference between

class A {
  private buffer, pos;

  method () {
    buffer[pos++] = 'hello'; // instead of this.buffer[this.pos++] = 'hello'.
  }
}

and

const A = function A() {
  var buffer, pos;

  return new class A {
    method () {
        buffer[pos++] = 'hello'; // instead of this.buffer[this.pos++] = 'hello'.
    }
  };
}

and

function A() {
  var buffer, pos;

  this.method = function method() {
    buffer[pos++] = 'hello'; // instead of this.buffer[this.pos++] = 'hello'.
  };
}

if all of them need the lookup to bubble up to the parent context? The second example could clearly be a potential implementation of the proposal, here.

@shelby3
Copy link

shelby3 commented Sep 14, 2016

@yanickrochon wrote:

The second example could clearly be a potential implementation of the proposal, here.

That is essentially the pattern I have always used for private members in JS.

But it has a problem. You can't access the private members of other which this proposal requires.

@zenparsing
Copy link
Member

Let's say that adding a private field means adding a getter/setter pair to the prototype, which does some kind of access control magic. Some issues:

  • Access to private fields is still slower than public properties.
  • You have to do additional checks to make sure that you aren't traversing the prototype chain to get the private field (private access should not traverse prototype chains).
  • Private field names are no longer collision-avoidant.
  • They can't be used to implement built-in Javascript classes (because the name leaks out as a property name).

The last one kills it for sure, because one of the goals of the proposal is to allow the full implementation of built-in classes and platform classes directly in Javascript itself.

@yanickrochon
Copy link

@shelby3 I personally use Symbols to create pseudo-private fields. They are not directly accessible from enumeration, and are not visible less they are accessed through acquiring the Symbols via Object.getOwnPropertySymbols().

Personally, this is a minimal argument, as private fields are accessible via reflection in pretty much all programming languages anyway. But I believe most people would rather have hard privates than soft privates...

Again, personally, I'd be alright to have private symbols and protected symbols, thus avoiding name collision, etc. IMO, private fields is pretty much just a semantic feature, just as class vs function constructor, and async/await vs Promises.

@dfahlander
Copy link
Author

@zenparsing I see the downside with sharing namespace with public properties even though I'm not yet convinced it's a showstopper. You may be right as you usually are. But I hope you understand that there are some of us that are concerned about the implications of introducing a new sigill.

What we're trying to achieve is to offer a surface for the end user developer that is in line with the user expectation, is uncomplicated, self-explanatory and is trying to avoid new syntax rules as much as ever possible.

The idea is to combine @eggers proposal with this-less access as an optimized block-level shortcut to the private properties. This-less access would solve your current concerns, but there seem to be a requirement to allow methods like compare etc where access is needed to private props on another instance. The idea is to allow that in an intuitive way and also to avoid errors due to using this.prop instead of just prop.

Access to private fields is still slower than public properties

I see. But this-less access will shortcut the lookup and make them as fast sigill-based this-less access. Dot-based access only optional.

You have to do additional checks to make sure that you aren't traversing the prototype chain to get the private field (private access should not traverse prototype chains).

Also here, it only applies to dot-based access. The use case for dot-based access is primarly a for accessing other.x. Do you mean that if other turns out to have overridden the private property, the original method would not do the job it believes its doing? So when extending a class you would need to avoid overriding its private props. I see the issue.

Private field names are no longer collision-avoidant.

True.

They can't be used to implement built-in Javascript classes (because the name leaks out as a property name).

Why couldn't they? Does the standard ever forbid the addition of reserved property names?

@eggers
Copy link

eggers commented Sep 15, 2016

@dfahlander Yeah. I'm really leaning toward this-less access. It mirrors how people do private fields in js today so is a small context switch. You could even let it be prefixed with let or const.

@DvdGiessen
Copy link

Disclaimer: I'm new to the entire proposal, been reading this repository for the last hour. Not sure if this is the right place to put this but perhaps it contributes something to the discussion.

So, I was wondering, if we'd rather not introduce a sigill and may even like using the private keyword because of its clarity when defining, why not use that same keyword for access as well?

Have been tinkering with this idea for a while before I came across this proposal. A quick draft of what it might look like:

class Foo {
    /* Use `private` keyword to define a property only accessible from within this class */
    private x;

    /* Supports initial value */
    private y = 5;

    constructor(x, y) {
        /* Access from anywhere within the class via the `private` keyword */
        private.x = x;
        private.y = y;
    }

    hello(name) {
        /* In standard methods as well */
        console.log('x equals ' + private.x + ' and we\'re saying hello to ' + name);
    }

    /* Normal (non-private) getter we'll use later */
    get y() {
        return private.y;
    }

    /* We could implement `private` for methods as well */
    private multiply(a, b) {
        return a * b;
    }

    /* Private getter, would be accessible as `private.xtimesy` */
    private get xtimesy() {
        /* Accessing private method, private field and normal instance field */
        return private.multiply(private.x, this.y);
    }
}
  • The private keyword would represent an object containing all private fields, making them accessible just like this.
  • We don't mix the namespaces, so in above example this.x would not map to private.x but be undefined.
  • The reference to private would only be available within the scope of the class. (Since a developer could (if he wanted to) share the reference to private from within the class we can still have the advantages noted in Hard-private vs soft-private #33 for soft-private.)
  • This approach should also be fairly straightforward to polyfill and as far as I can see this wouldn't introduce any significant overhead for native implementations.
  • And importantly, at least from my perspective, this looks intuitive and use of the private keyword does pretty much what a developer would expect.

@dalexander01
Copy link

@DvdGiessen this has been suggested before in #14

"private" cannot be used for access since "private" is a valid property name.

@littledan
Copy link
Member

@DvdGiessen The problem with the private keyword like this is that it could only let you refer to the receiver (the current this value); you would need a different syntax to access the private field/method on other instances. It's hard for me to picture what that syntax should be, while being analogous.

@DvdGiessen
Copy link

@dalexander01 That only seems to be a problem when considering how we access private fields of other instances (as @littledan noted)? In the discussion at #14 you mentioned a solution like this.private for accessing private fields of other instances, which indeed would not work because collisions with property names. The cases within a class (accessing a single instance) described in my example would work fine; private is a reserved keyword and one could never define private as a variable in scope.

@littledan Perhaps we could use the keyword in a language construct, parametrizing the target instance? Doing so wouldn't introduce any new keywords and does not overlap with any existing valid syntax.

class Bar {
    private x;
    constructor(x) {
        /* `private` is in this case equivalent to `private(this)` */
        private.x = x;
        private(this).x = x;
    }

    equals(other) {
        /* Accessing private fields of other instance */
        return private(other).x == private.x;
        /* `private(obj)` construct checks if(!(obj instanceof Bar)) throw new TypeError(); */
    }
}

let a = new Bar(5);
let b = new Bar(10);
let c = new Bar(10);
a.equals(b); // false
b.equals(c); // true

@bakkot
Copy link
Contributor

bakkot commented Nov 10, 2016

@DvdGiessen
Copy link

@bakkot Both points mentioned in your comment in #14 (as well as related comments in that thread) are addressed by the notion that private would represent a object (might be further specified as i.e. a Map type).

The return value is known. As noted that private object could be passed around like any other, so interpreting private(obj) as a function would yield expected behaviour.
(From a visual standpoint, the only issue would be that private itself is not a function we can pass around, but given that the private.x syntax would be the most often used it can be expected developers have no trouble with it being a language construct.)

And, regarding nesting, it would look like private(private(obj).left).right, as noted in your comment but without the sigills?

@bakkot
Copy link
Contributor

bakkot commented Nov 10, 2016

the only issue would be that private itself is not a function we can pass around

This is not a minor issue; we shouldn't be introducing things which aren't functions which look like functions.

private would represent a object

Having private be an actual object would introduce a host of complexity - could you define accessors on it? Would lookups go up its prototype chain? Could fields be accessed dynamically (a la private['foo']? If you can leak the object (return private;), all of those would have to be true to work with the rest of the language. I don't think exposing an actual object representing private fields of some other object is a good model.

And, regarding nesting, it would look like private(private(obj).left).right, as noted in your comment but without the sigills?

My point was that this is aesthetically displeasing and difficult/confusing to read (compare what you've written toprivate(private(obj.left).right); the distinction is subtle), relative to the current proposal.

@Ltrlg
Copy link

Ltrlg commented Nov 10, 2016

the only issue would be that private itself is not a function we can pass around

This is not a minor issue; we shouldn't be introducing things which aren't functions which look like functions.

@bakkot There is a stage 2 proposal for import() already.

@rwaldron
Copy link

This is not a minor issue; we shouldn't be introducing things which aren't functions which look like functions.

// while(...) looks like a function call:
while(0) 
console.log(1);

// for(in) looks like a function call: 
for(p in {})
console.log(p in {});

// super() looks like a function call:
super(1)
console.log(1);

@yanickrochon
Copy link

yanickrochon commented Nov 10, 2016

This would actually make a lot of sense. And is consistent with the language. A lot more than a sigill. However, I'd rather have it be more consistent, always using parenthesis.

class Bar {
    private x;
    constructor(x) {
        private().x = x;       // same as private(this).x = x;        
    }

    equals(other) {
        /* Accessing private fields of other instance */
        return private(other).x == private().x;
        /* `private(obj)` construct checks if(!(obj instanceof Bar)) throw new TypeError(); */
    }
}

let a = new Bar(5);
let b = new Bar(10);
let c = new Bar(10);
a.equals(b); // false
b.equals(c); // true

@yanickrochon
Copy link

The only issue with this is to avoid these cases

class Bar {
  private x;
  a() { 
    // should throw TypeError (or the non-standard InternalError)
    return private();
  }
  b() {
    // should throw TypeError (or the non-standard InternalError)
    const p = private();

    return p;
  }
}

Basically, a variable cannot be assigned whatever private() returns.

@zenparsing
Copy link
Member

It's not a bad alternative, but there are two usability issues:

  • The declaration syntax looks very familiar, and yet the access syntax is completely unique. This discrepancy is unfortunate and would probably cause some amount of discomfort for new learners.
  • It's just a lot to type, especially for an expression syntax that we know will be very common.

@yanickrochon
Copy link

@zenparsing

It's just a lot to type, especially for an expression syntax that we know will be very common.

I don't necessary agree with this. Even the current source code is using only but a few underscored variables. Private variables should not be overused anyhow. They represent a state, and there is so much states an object can hold.

@littledan
Copy link
Member

I think we're discussed this issue pretty thoroughly, and I don't see a way we can make this work consistently from a technical perspective. Let's stick with the sigil.

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