-
Notifications
You must be signed in to change notification settings - Fork 113
Use of this pointer with private fields #17
Comments
I suppose you could think of a private field definition of a 'class' as being hooked up to the prototype but not accessible to anything other than the methods of a 'class', and so in turn doing a this.#someProp instance level (own) property overwriting said defined private prototype value. That could work, although would that mean that not using keyword 'this' would set the value for all instances (on it's prototype)? And that still doesn't really solve the issue in example 1 where the 'this' pointer is not actually defined or not an instance of the object in question. |
These are existing problems, I don't know that this syntax makes the problem any worse.
Calling functions with the proper context has always been the responsibility of the user. Agents could certainly make this a clear error message at least, like "method called with improper 'this' value" or something. I don't think there's really a fix for this. The function call is just a normal function call, there are no other channels of data that would tell you which instance of the class was being used.
This also already happens now if you extend any type that requires internal slots, like
ES6 method cannot be called as constructors. |
Also, attempting to access a private field on a receiver which lacks it will throw an error at runtime. |
It's a private field, not protected. If you try to access it from another class, it must throw an error. Also, if |
@seansd You can do subclassing here like this to set up the private fields as desired: function Point3D(x, y, z)
{
var self = Reflect.construct(Point2D, [x, y], new.target);
self.z = z;
return self;
}
Point3D.prototype = Object.create(Point2D.prototype); |
I concur that the fact that this is not necessarily what you'd expect is already a problem, however adding this functionality i feel would exacerbate that problem further, hence the discussion.
I'm not sure what the spec says, but try it in Chrome right now, it absolutely works, there is nothing forcing an plain old function with prototype that has an ES6 class from being called as a constructor and thereby not calling super. . . And the options are not ideal:
Another example:
the this checks in the Point2D.toString method will pass b/c Point3D inherits the prototype, but everything will be wrong. |
In the above example, i suppose that could still work since the toString method would first look at a private value it has access to, and therefore, the Point3D evil property override won't work (that's good), but it still would have only an uninitialized / default value. . . so again I say that we need to find a solution somehow. .. and forcing a user to always validate the 'this' pointer is not great, especially when it's not really easy to validate |
i suppose at the very least you'd have the option of adding another private member to ensure the construtor was called (given my comment above would be correct). |
No, they would not. Edit:
Yes, it would; this proposal does not give any new semantics to the |
I should clarify that with "successfully" I guess :P You can use ES5-style inheritance to inherit the prototype of an ES6 class you want in the prototype chain of an object, but there is no guarantee that it will work. The same is already true in ES5 with
because the object you created isn't actually an array, it's just an object that kind of looks like an array. The same is true for ES6 classes when extended using the ES5 approach. If you want to correctly extend an ES6 class from an ES5 constructor, you need to use |
Right yes I get that (in terms of ES5/ES6 construction), and hence that's my point, b/c that could be done, checking the 'this' pointer to make sure it's an 'instanceof' is not particularly helpful. If (given my example), an improper inherited object called said method and as a result an error was thrown like you suggest, then (assuming we write that in the spec), I think that would be ok, since developers would not need to constantly do said checks (unless they wanted to silence the error and just default). I want to make sure said messaging is clear though. . . With that said, I wonder if we could also add something to do said check more easily. . .what does this.hasOwnProperty('#foo') or Object.getOwnPropertyDescriptor(this, '#foo') do inside class methods? Always return false? What about the 'in' operator? Might be nice to allow developers to check and instead of throwing do something else. . . |
The behavior I described is already what's specified. In this proposal there's no way to check for a private field except a |
Those are unchanged. You can already have a property named
creates an object with that property name. Because of the usage of quotes, the It could potentially work to allow |
right, only inside a class method of course :), another question are you required to always access private members via '.' (dot) notation? Or would this['#foo'] still work so long as it's inside a given class method and said field was declared in that class? |
You are required to use dot notation - see the faq I linked above. |
I also realize my example is tough because
It would actually check that |
@loganfsmyth, that's kind of an argument against the shorthand syntax, actually. Albeit a quite weak one. |
Actually your example is still ok, b/c in that case the 'this' pointer still MUST be a constructed instance of Example (and actually an object), it was just defaulted with a value. And as a developer you could just do:
Would that not work? I'm not so sure i like requiring DOT notation though, could lead to big code. . . |
In your case if you provided a default value statically for all instances, then it would still return true, but that's ok b/c the developer would have the option. . .(assuming this is actually a proper instance of example) |
and just back to requiring DOT notation, I have worked on 2 separate apps, where we have a ton of code, and we have a job that converts long property names to using [] syntax for size savings which ends up saving over 15% in size without requiring gzip and with basically no discernable run-time penalty. . . so that's a nice thing. . granted it's only useful in very large codebases. . . |
The FAQ entry linked above explains our rational for that decision. We are not, in general, particularly concerned with file size, especially uncompressed - but note that this feature allows minifiers to rename private fields safely (because they cannot be accessed from outside the class and because it is possible to statically determine which field is being accessed), which is not possible for public properties. |
The issue with my example is that |
Right i get that is the case now, but what i'm saying is that could still be ok if you modify what the 'in' operator does. .. if statement is Again I'm pointing out that the modified 'in' operator behavior would only apply inside a class method, and therefore would not evaluate a |
Ah good call, a minifier would always be free to change the private property name to something very short since it can only be used inside the given class. . . yeah ok that means |
Also curious would private properties/fields be accessible via arrow functions similar to how super is. . . ?
So would the same rule apply to privates?
|
The issue is that the
Those last two behave identically, vs
which under your proposal would evaluate to two different checks, which would be against general expectations for JS as a language. Alternatively that would mean
Yes they are usable from arrows. Someone else can chime in, but I believe they work in any nested scope in the class, as if they were like variables. |
Yeah I get your concern although I also don't think it's that problematic since we are talking about fields inside a class method. . .I suppose you could just do:
It's just a shame that we can't make such handling simpler. . .I suppose you could have a new operator, like if JavaScript had a |
Another question. . .is there a way to lock a private field / make it read-only later? I think I'm right that you can have them be defined as constants initially (yes/no?), but given the above could I force |
I think a piece you are missing here is that the prototype is not the important part for this feature. The object must actually be created by running the class constructor. You don't need your check because accessing
There is not. |
Hmmmm OK that would make sense but if you wanted to handle the given error case of a bad this pointer yourself you'd have to do something like:
Again it's just a shame there's no way to make it so that classes that wanted to fail gracefully without throwing could have to implement something like the above in each method. . . but again as long as the error messaging is decent i suppose it solves the bulk problem of folks calling class methods improperly and thereby breaking a function unexpectedly. Does that also mean that you cannot change a private value for all instances?
Could you do |
Yeah, the catch would work well as an approach.
There is no scope where a private is common across all instances. Each instance has their own copy of all private fields.
For public fields, setting it on the prototype would modify the prototype, but the field set with the class property is unaffected.
so even through it is on the prototype, references to
That would throw because |
Ok got it, so does a public property definition of the same name as a public field, happening later then just override the given field (assuming the correct this pointer)?
And also I'm guessing that means field definitions can't be getters/setters ie.
Or could they? |
Sorry, I don't follow what you're asking there.
Correct, they are only values. |
What I'm asking is does a public field automatically become a property (and thereby be configurable, enumerable, etc)?
|
Public fields are properties on the instance, yes. (And not on the prototype.) As you can see in the spec, they're currently writable and enumerable, but not configurable. I'm not sure how set we are on that last part. In your example: console.log(super.foo); // undefined, since there is no `foo` on Example.prototype
super.method(); // 200; there is only one property `foo`, and it's redefined to be 200 by `SubExample`'s constructor
Object.defineProperty(...); // fails; the property is not configurable. In other words, a public field definition class A {
constructor() {
Object.defineProperty(this, 'f', {configurable: false, writable: true, enumerable: true, value: 0});
}
} This has gotten somewhat off topic, however. |
Class fields are normal properties on the class instance, that are set on the instance when the given classes's constructor initializes the instance.
|
Yeah it's somewhat off topic but I was just trying to understand if fields are unique to a class. So basically a public field definition on a parent class is forced down to all sub classes as an 'own' prop. (and could even be non-writable). Anyway I think we can close, so long as we can clearly document then error messaging (eventhough i'd still like a better way to handle a bad 'this' pointer more gracefully, but at least it's better) |
Sorry, what's the error you're concerned about? |
The error would be the TypeError in I'm assuming the error would be like the current behavior for something like
which on Chrome throws
but since missing private fields aren't something people are used to, having a good error message for it could easily be helpful. No-one just starting out will know what "incompatible receiver" means. Does JS really specify much in the way of error text for issues at the moment? I don't think I've seen any of that in the spec currently. |
@loganfsmyth Yes, that's the current behavior--it throws a TypeError, but the spec doesn't say exactly what the message is. Descriptive messages are up to the implementations, many of which are actively working on improving things. Would the error be less unexpected if you had to access private fields through |
actually what would |
@seansd Yes. |
OK, it seems like there isn't anything to do about this issue; hopefully the discussion here will be useful for other people trying to understand the proposal. Closing for now. |
In all of the listed examples, a private field of a class is accessed / used within methods or the constructor by using the
this
keyword, which I'm not sure if that's supposed to be intended or not, but also has some nasty consequences. For example:Another example would be where a 'class' inherits from another class but with out using JS 'class' syntax and thereby not calling the super constructor
The first example is bad news imho, b/c there is no way to know the true value of this unless the dev writing the function always does a check up front every time to ensure a valid this pointer, but then even in that case we get to the 2nd example.
The 2nd example is a bit non-sensical, but it is still important b/c the
oPoint3D
will be aninstanceof
oPoint2D eventhough asuper
was not called. I assume that example would just return the empty string value? But the point is even if you were to validate thethis
pointer every single time, you still may not be able to do so properly b/c you have no way to truly ensure construction occured (at least not easily).I can think of other examples as well (such as what if a class method itself is a constructor). . .what are the thoughts around this?
The text was updated successfully, but these errors were encountered: