-
Notifications
You must be signed in to change notification settings - Fork 113
Private property that shadows public property #147
Comments
class A {
private x;
method(obj) {
return obj.x;
}
} There is no way for the engine to know if Edit: That is to say,
It is not true that this is necessarily what is expected, especially in deeply nested classes. See the FAQ. |
@bakkot again ignoring technical limitations, I can read that and make a decision it means they are referencing the private x. As long as it's defined that way. So I'm not sure how there would be absolutely no way for the engine to also decide this. |
@bakkot sorry I kind of get what you are saying but I guess because it's shadowed I would expect it to check if it's an instance of A and if private x is defined. Otherwise just return public x of object. |
So does every Now if every
We've discussed these ideas over and over again. That the issues in this repo seem to go nowhere is because the ideas aren't tenable. The best alternatives have been the private symbols proposal, and classes 1.1, and the committee still feels this is the best option. |
That means that This is also in the FAQ. |
@bakkot @jridgewell with all due respect this use case is far less common than you seem to be letting on. In this case if I was accepting objects as parameters I would use a brand check. I would expect that shadow to behave this way based on what I know of JS and I would work around the other constraints accordingly. A brand check is easy. If I'm accepting objects, that's part of my public API anyways. I would verify my input.
Tenable to who? As others have said all of the proposals have limitations and for some reason the TC39 committee has a list of hard requirements that they haven't shared with the rest of the world. And they have decided that this proposal is the only way forward despite all of it's own shortcomings. I would say this proposal is untenable, yet here we are. |
@shannon Unfortunately , as some requirements which TC39 already has consensus, like hard private, undetectable, no runtime-check for performance... private access can never be Sigh... You already see, whenever you use syntax That's why I think classes 1.1 is better, it just have no such pain: no |
@hax that's sort of my question. Ignoring those requirements. Would this be something that developers expect or want? Hard private and undetectable are debatable. But no runtime-check is not something I expected to be part of the language here. I do expect the engine to optimize this mostly away so I don't have to think about it. I just want this to behave like I expect. And it seems like there is not much that behaves the way I would expect. I do think the classes 1.1 is a better approach but I expected |
The problem is you can't ignore these requirements... Of coz we could always argue for undetectable and debatable. But I don't think it's possible to change the semantic requirements. So the only part left is syntax. And classes 1.1 mostly about syntax and mental model. Because the semantic requirements makes Java/C# like private style impossible, so in my opinion, we should choose a syntax/terminology keep far distance to Java/C#, so programmers won't always ask why not Java-style 😂 About you specific syntax preference, |
@hax I've seen you pointed this in several threads:
Generally it's wrong. We could use |
@shannon The truth is that there are no technical limitations within ES engines to prevent this syntax. However, most of the objection you're going to receive is due to the oddly skittish nature of developers expecting the language to protect them from making mistakes. I don't know what happened, but somewhere along the way developers have shifted from expecting their tools to enable possibilities to expecting their tools to limit possibilities. This is disheartening to me. But back to the issue at hand, I can and have enabled almost exactly that syntax just by using a Proxy. Even inheritance continued to work as I expected. So there really is no technical limitation preventing the Using that syntax:
Would it be something developers want? Therein lies the rub. Want the syntax? Yes! Want the extra considerations and limitations? No-one's really done a good public test to figure that out, but I would wager "probably not". The same kind of trade-off exists in all of the proposals. The problem is whether or not the particular trade is a fair and acceptable one. The reason the current proposal has received so much discussion even into the 3rd stage is because many of us find the trade-offs require to be too extreme. Would the same be true for this syntax? I don't know. |
@hax
I think you would have the same issue with var. Yea it's mostly bikeshedding but I only say this because of how I expect it to look and work. And that means looking like other existing parts of JS.
I just don't know where these requirements came from. I'm really curious how many developers have asked for them and what types of use cases need them. It seems like every time a new proposal pops up it gets debated by the same two or three people in GitHub and then the committee goes and discusses it offline and then says it didn't meet the requirements. I have my own "requirements":
So far this proposal has failed at all of these. |
I agree with this statement. I don't know what happened either.
Does the TC39 have a mechanism for doing this public test? I see the nodejs modules team doing something like a questionnaire for the minimum implementation of the ES modules in Node. It's not a vote but if you ask the right questions you can get good feedback on what the larger developer community actually wants. Instead of all this anecdotal conjecture. I would highly recommend that the TC39 do something similar here. |
Even if we ignore the surprising changes in semantics, I don't think you realize how many property accesses are in JS code. Imagine slowing down every one of them by even 1% just to add a runtime check if for "is this a private property?"
That's a core issue, it's not possible to just "optimize" this away. It will affect every property access in all code, even where private state is not used at all.
Coming from JS? I already expect Other languages (any of which you're thinking of) have a compile time type checker to enforce property access, so there's no ambiguity. That's not possible in JS because there is not type checker.
To the committee. We have already discussed runtime semantics like this, and it is unworkable.
This is half true. Yes, we could add runtime checks and do exactly as this issue suggest. But the performance difference is a technical limitation. The breaking of hard-encapsulation this allows is a technical limitation. Etc. |
@jridgewell, no there is an option to make |
I don't think this is true. It would only apply to classes that have private fields and only when accessing a private field. It is pretty straightforward to manage this in a crude way with get/set and analyzing the stack. The engine can optimize more beyond that. I don't see how this really breaks hard encapsulation. Mistakenly accessing a property of a foreign object is not about encapsulation as I understand it in JS terms. |
Or as @Igmat has suggested with private symbols. It's not technically impossible to do. And this is something I would expect the engine to be able to do under the hood. |
The only solution I'm aware of is changing all This was also discussed when we talked about access semantics, and it was flatly rejected.
Unfortunately, it is true. There is no way to do runtime checking of the receiver (to decide if the property is public or private) without affect all public property accesses.
See tc39/proposal-private-fields#14 (comment), from the FAQ.
Private symbols are a different proposal (and one I actually support) than this issue's proposal. Doing |
This could be done with an API similar to Reflect (or even just Reflect.get itself) if this uncommon use case was important enough. Not sure why it would be flatly rejected.
I'm sorry I just can't accept this as truth. There must be some way to branch the accessor code in a way that is equivalent to the this.#foo access in terms of performance. |
@jridgewell What's with this scare tactic?
Right now, while no such feature exists, we as developers are using things like Proxy, closures, WeakMaps, and other such things to achieve data privacy and incurring far, far greater time penalties in the process. And yet, we're not complaining. Now if we say we incur a 1% time penalty for every private access, a language based solution would reduce that to a 0.001% time penalty scattered across every access. Would we even notice such a small penalty? Even if we did, would it be worth the trade-off to gain a reasonable, non-future blocking, non-feature breaking version of private data that's easier to use than what we're currently doing? I think most developers would say yes. |
Just imagine: class Example {
test(obj) {
return obj.name;
}
} Now if I introduce a private
Which only affects the private properties you're using. It does not affect the public properties external to your code.
Ok. My project has 199614 public property accesses, before transpiling and not counting spec algorithms. And then there are hot-for loops, which access properties on sequential objects, etc. It doesn't scare you to slow down every property access to make support for some private properties?
Yes, I think you would absolutely notice it. And so did every one of the implementers in the room.
Per my above above reply in this comment, I don't think this is easier to use at all. |
I disagree with this logic. If I'm adding a private field I almost always want to be then accessing the private field internally so no refactor is required. A refactor is only required on the off chance I sill want to access the public property of obj which sounds perfectly reasonable to me. I want to explicitly access the public property, not the private property declared here. If the obj is not instance of Example, then again no refactor and nothing changes in the lookup semantics.
I think this will work just fine for normal developers. |
Please refer back to the slowdown and encapsulation break in #147 (comment). Having |
Want other solutions?
Those are just 2 of the ideas I came up with in the last 30 seconds.
In that case, I'd just look at you funny, wondering why you did that. With this version of the syntax,
That sounds really scary.... wait.... no it doesn't. I would have to somehow run every possible pathway through your code to incur that many accesses. Even if I did, 200,000 * 10 = 2Million extra clock ticks. So that weird process running through every possible path which would take probably about a minute or 2 now, would take an extra 2 seconds. Justin, computers are scary fast. |
Please refer to the rest of this thread where these issues are discussed. |
And then compare this to the extra time it takes to compile the extra this.#x syntax semantics. I suspect the performance would even out as the engine optimized at runtime, which V8 is pretty great at. |
Every single implementer in the room balked. I can't stress that enough, every single one of them doesn't want to make access more complicated than it already is. Given those are both the same solution, I'll leave it at that.
I'm not talking about multiple private
What?
Not really, just any one of them in a loop.
I cannot be more explicit in my rebuttal of them every time it's brought up. |
Then why mention you how many you have? This would almost certainly be optimized away if you ran it in a loop.
No one wants to make things more complicated, but if that's what it takes to make the syntax/mental model clean and simple then that's what it takes. That's the job of the engine. Are there any bugs / issue trackers / forums with the various engines that track this discussion or was it just in a closed room with TC39? |
@Igmat Yes, your proposal may work, but there is a new pain: resolution rule is more complex, |
@shannon Code that I write in Java all the time: class Point {
// constructor omitted
private int x;
private int y;
boolean equal(Point other) {
return this.x == other.x && this.y == other.y;
}
} If the analogous JS code, replacing Keep in mind that you don't even need an explicit parameter to observe this, since you can |
I will guarantee that there is a "is this the same shape" check at every monomorphic site, because it has to ensure it is monomorphic. Even in the current
We disagree at this point. Accessing a public field when it's meant to private is breaking encapsulation. There is a strong difference between what you are proposing and what private symbols does. In private symbols, it is always a private access, and it's thus impossible to leak the private property afterwards. Guarding with branding fixes this code example, but does not change the semantics of "it's sometimes private and sometimes public". If the proposal doesn't guarantee always private, it's not encapsulated.
Because it is encapsulation. |
If we just jump into a method (for example, when debugging), and try to find whether Assume we are not using That means, when you see And, |
I don't know too. But I guess some come from library/framework authors, some come from the engine vendors, and some from the the preference of the TC39 members as how they understand the language in a language designer perspective. The last is not necessarily bad, most time they are good I believe.
While as a developer I don't have these requirements in most case, but I understand they may all have some good reason to have. I just don't want to deny the requirements from others, just like I don't want other deny mine. 😛 In the worst case, like how I am against public field, I never say public field is a false requirement, but: you don't have enough evidence to prove it's a must to have in current status, and even you want it anyway, you should not land it in such broken form. |
Of coz it's a communication problem. But, @littledan @bakkot and others who love current proposal, what I am really worrying about is, you guys talked to many others, and convinced some it's a good enough proposal. And when you do more, you see more agree with you, less disagree with you, so you just believe, yes, we're ok! And for those still make a noise (like me 😉 or some others here), you just think ok, we can't satisfy everyone, we already did our best to "communicate" with the objectors. But it's probably only a self-reinforcement, not a truth. Many who disagree just disappear because they just tired. And someone who disagree will never come here, they just choose use their own private solution and don't care anymore. So for a controversial proposal like this, the risk of community break will always there, we need to step back and find some root cause of the controversial parts, and try to eliminate or at least mitigate it. |
Again, not too bad if you think in
No, I'm not trying to say current proposal is ok (you will not believe me 😘)! I just try to take the position of the supporters of this proposal. If you just think in a way like they want, yes, it seems not too bad. (at least in the private syntax part) Unfortunately , the "when you learned, you will accommodate it" assumption is only partly true. The full chains are "only when you learned, agreed, or at least compromised, you may accept it, and accommodate it". So you (@bakkot @littledan ) see the problem, there are many (like us? 😉), who will hardly to agree it, and some of them don't want to compromise, so they never accept it, don't mention accommodate it. |
Actually they did, I truely believe, but the problem is... see my previous comment: #147 (comment) |
Though I agree with you in this case (see my previous comment: #147 (comment)) , you know what, as I always say you should think about why there are too many such tryings? You always think your solution is good enough, and you always think they eventually have to accept it and accommodate it, but the statistics in #100 and all these hopeless tryings must tell you something. |
And there are two branches:
This case is mostly on the branch 1, and I mostly on branch 2. In my opinion, branch 1 is hopeless. The only possible solution is on branch 2. Yes, classes 1.1, again. 😜 |
Can we clear up what we're talking about here. We've kinda gone into the weeds of "what if the syntax was class Example {
private x, y;
constructor() {
this.x = Math.random();
this.y = Math.random();
}
static print(obj) {
console.log(`obj.x = ${obj.x}`);
console.log(`obj.y = ${obj.y}`);
}
} I'm going to ask and answer a few things first:
That's just what I'm thinking. Can we all throw in our own 2 cents and see if we get a quarter back? That would at least be a better place than where we're at. |
If this is always an instance of the same class with private fields the shape of the code is always the same. It's redundant to do further checks on property lookups. This is how it can be optimized. The loop can be unrolled and it becomes a single check.
Fine we can disagree on what it should be called but I still think this particular problem should be solved in other ways. The current proposal tries to solve too many problems with the same hammer. Explicit brand checking or a follow on proposal for guards seems like a better approach for this issue. @hax in regards to it making my code harder to read, write, and understand. Unfortunately this proposal is not just about the sigil. I could get past the sigil. But lack of dynamic access really does make my code worse and the suggestions to work around this limitation via decorators are lacking and objectively worse. Especially now that class decorators no longer have access to private field names. Simple Example: I want to write this class Entity {
private a, b, c, x, y, z;
constructor(initState) {
for(const prop of ['a', 'b', 'c', 'x', 'y', 'z']) {
this[prop] = initState[prop];
}
}
private sendState() {
for(const prop of ['a', 'b', 'c', 'x', 'y', 'z']) {
if(this[prop] !== undefined) sendOverWire(this.id, prop, this[prop]);
}
}
private receiveState(state) {
for(const prop of ['a', 'b', 'c', 'x', 'y', 'z') {
if(state[prop] !== undefined) this[prop] = state[prop];
}
}
} I have to write this: class Entity {
#a, #b, #c, #x, #y, #z;
constructor(initState) {
this.#a = initState.a;
this.#b = initState.b;
this.#c = initState.c;
this.#x = initState.x;
this.#y = initState.y;
this.#z = initState.z;
}
#sendState() {
if(this.#a !== undefined) sendOverWire(this.id, 'a', this.#a);
if(this.#b !== undefined) sendOverWire(this.id, 'b', this.#b);
if(this.#c !== undefined) sendOverWire(this.id, 'c', this.#c);
if(this.#x !== undefined) sendOverWire(this.id, 'x', this.#x);
if(this.#y !== undefined) sendOverWire(this.id, 'y', this.#y);
if(this.#z !== undefined) sendOverWire(this.id, 'z', this.#z);
}
#recieveState(state) {
if(state.a !== undefined) this.#a = state.a;
if(state.b !== undefined) this.#b = state.b;
if(state.c !== undefined) this.#c = state.c;
if(state.x !== undefined) this.#x = state.x;
if(state.y !== undefined) this.#y = state.y;
if(state.z !== undefined) this.#z = state.z;
}
} This is a simplified real world example of me trying to use this proposal. See #75 for more details on the limitations I see with this proposal and how it makes for worse code. This doesn't event touch on the hell of trying to deal with proxies that monitor state change to send over the wire.
I see what you are saying but I also just want to scale back the scope. See above about forced brand checks. So I would say I'm a bit of both. The lack of compromise is what's really bothering me. The syntax and mental overhead of this proposal is worse than the alternatives that I have seen and it frustrates me that the TC39 committee has effectively said that they will not compromise in anyway because our members see X Y Z as non negotiable and must be included in this single proposal. Which is basically everything at this point. I just see a lot of pain ahead. @rdking I'm not sure I'm quite on the same page as you. I wouldn't expect a private property to collide with a public property. I would expect it to shadow. In fact that's a major reason my consumers would choose to use private, to avoid collisions. Answers to your questions:
|
Since
The only cost of this solution is small addition to P.S.
|
@shannon In the uncommon case that you need dynamic access to private data, you can do like this: class Entity {
#state = Object.create(null);
constructor(initState) {
for(const prop of ['a', 'b', 'c', 'x', 'y', 'z']) {
this.#state[prop] = initState[prop];
}
}
#sendState() {
for(const prop of ['a', 'b', 'c', 'x', 'y', 'z']) {
if(this.#state[prop] !== undefined) sendOverWire(this.id, prop, this.#state[prop]);
}
}
#receiveState(state) {
for(const prop of ['a', 'b', 'c', 'x', 'y', 'z') {
if(state[prop] !== undefined) this#state[prop] = state[prop];
}
}
} |
@nicolo-ribaudo This was suggested to me in #75 but it makes it harder to attach metadata to fields via decorators: class Entity {
@replicate('ownerOnly') @observe @etc private a;
@replicate('all') @etc private b;
...
} class Entity {
#state = ???
} Also it's not uncommon to want dynamic access to fields. |
That's not quite right. You'd actually need to modify object access semantics in general. Consider my earlier example, with class Example {
private x, y;
constructor() {
this.x = Math.random();
this.y = Math.random();
}
static print(obj) {
console.log(`obj.x = ${obj.x}`);
console.log(`obj.y = ${obj.y}`);
}
} Here, |
This is another problem. At the point where
but that's a brand check. If you don't do at least this much, you'll end up leaking your private data elsewhere. That's bad even if nothing else can access it. |
@rdking, according to my proposal your example won't throw, but will return public fields
|
So then, what happens in the case of [[Set]]? Given: class Example {
private x, y;
constructor() {
this.x = Math.random();
this.y = Math.random();
}
static setX(obj, val) {
obj.x = val;
}
static print(obj) {
console.log(`obj.x = ${obj.x}`);
console.log(`obj.y = ${obj.y}`);
}
} It's hard to see what this is supposed to do. If the notation in |
Because they are just static bindings, it seems hard to support dynamic access... But I think classes 1.1 just support destructuring. class Test {
var x, y, z
constructor(...args) {
[x, y, z] = args
}
} |
@rdking, Why does |
@Igmat under your proposal is Also would something like this be possible? class Entity {
private x, y, z;
constructor(state) {
for(const prop of [x, y, z]) {
this[prop] = state[prop.toNameString()]; //toNameString() returning 'x', 'y', or 'z'
}
}
} Any thoughts on destructuring? class Entity {
private x = 'x', y = 'y', z = 'z';
method(){
const { [x]: _x, [y] : _y, [z]: _z } = this;
assert(_x === 'x');
assert(_y === 'y');
assert(_z === 'z');
}
} It seems to me like this should be valid code under your proposal and would work as I expect based on my understanding of symbols. Is that correct? |
@shannon, yes both examples are valid and would work as you expect. |
@Igmat if that's the case then I don't even think this.x is necessary and it should just be this[x] but based on this proposals champion's insisting on private x / this.x relationship I'm fine with it. So I'm on board with your proposal as it meets most of my personal requirements and makes sense under my understanding of existing JS paradigms. |
@shannon thanks for that. I have to admit that consistent Luckily, linter may have a rule to enforce first coding style, which is good enough workaround. Also, my proposal most probably needs some additional investigation and much bigger amount of docs around it to be real alternative. The only reason why I'm not working on it now, is that committee doesn't seem to be willing to give at least a chance for any alternatives :( So now I'm concentrated on gathering some feedback that will prove assumptions of this proposal's opponents about community. Obviously, if feedback will show my mistake, I would stop complaining. If you're interested in ant related activities you may email me directly at ichulinda@gmail.com |
@shannon I sent an email to you. |
I think @bakkot in #147 (comment) and @nicolo-ribaudo in #147 (comment) have addressed the points raised in this issue well, so closing it. |
I'm concerned about this proposal and, as others have expressed, how the TC39 have handled community feedback. So I've been thinking a lot about the problem and the various different proposals and syntaxes and I just can't help but think that the JS "engine" should be able to do this the way I expected private properties to work before I knew this proposal existed. Even if crudely at first, it should be able to be optimized later. But then I don't know if what I expect is really what other developers expected so I figured I would ask while the discussion is still hot.
Anyways, I'm just wondering, if hypothetically there were no technical limitations about property access and property access performance, would the below syntax and mental model be desirable to most developers.
It's described simply as you can declare a private property that shadows public properties when accessed from inside the class.
I understand that there may be some situations where you need to access the public property from inside the class but this would align with any variable scope shadowing and would generally be a known mental model.
I also understand that you may want to access private properties of other objects of the same class (
obj.x
). So let's just ignore the technical limitation for a moment and assume that worked as expected (obj.x
=== 'private x' when inside the class). Would that mental model also be desirable?The text was updated successfully, but these errors were encountered: