Closed rdking closed 5 years ago
@ljharb Do you have any idea why this problem wasn't targeted when class
was first being decided? Or is this the reason class
was released without data property support?
This is a pretty radical departure from how properties that need to be initialized to objects per-instance have always been handled up till now, even in pre-ES6 code that using a coding style that puts all properties on the prototype, e.g.:
function MyClass() {
this.obj = {}
}
MyClass.prototype.primitive = 1
MyClass.prototype.obj = null
Up till now, classes have mainly just been sugar for what we previously did with constructor functions and prototypes (and of course the more conventional version of that would initialize all non-method properties in the constructor, in contrast with the above example, but that's beside the point I want to make now...)
If you're going this far to change the behavior of prototype properties initialized to objects, I feel like you'd be better off going all the way and giving classes that use these new property declarations actually different semantics than objects as they exist in the current version of the language. I am making a devil's advocate argument; I don't think this is a feasible option nor a good idea at this point in JS's evolution. But what you're suggesting introduces magic and side-effects that developers would not be expecting at all, so in a way it would be better to just be up-front about it rather than offering a hybrid solution that pretends to compile down to familiar old constructor and prototype patterns when really it doesn't for special cases like this.
And this isn't even considering the fact that a large number of developers have already been using public property declarations (mostly in Babel 6, in the default loose mode with [[Set]] semantics—but I digress) and have a different expectation for the same syntax, namely that it only creates an instance property, not a prototype property and certainly not a proxy. And then there are performance considerations, even with a native implementation of this idea...
While I like how this would avoid the foot-gun of accidentally shared prototype properties and might be somewhat persuadable on this, I think there are much simpler solutions to this problem. IMO the current class fields proposal offers the simplest and best solution (methods always go on the prototype, fields always go on the instance, unless decorators change the placement). Alternatively I would be open to a hybrid solution equivalent to my above code sample, where properties initialized to objects receive special treatment...but affecting how the constructor works in that way goes against the stated goals of this proposal.
Up till now, classes have mainly just been sugar for what we previously did with constructor functions and prototypes
Haven't I been one of the loudest voices saying this? Instance-properties makes the same violation by trying to eschew the constructor with a pseudo-declarative statement inside the class definition, and that causes more problems than it solves.
If you're going this far to change the behavior of prototype properties initialized to objects, ...
This isn't going far at all. This kind of solution hits the fly with a swatter no bigger than the fly itself. The problem preventing developers from using public properties for this purpose is that public properties don't duplicate values to the instance on write when the value is an object (unless the entire object value is replaced). This technique just makes that happen. It only happens if the value is a non-primitive, non-function value. I've even considered adding the additional exception of not using this technique if the object is frozen, with the idea that freezing the object means that it is not expected to ever be modified and that the developer is willing to take the risk.
But what you're suggesting introduces magic and side-effects that developers would not be expecting at all...
I'm fairly certain this would be easily explained. It only took 8 words to explain it to my desk-mate at work: "It makes objects copy on write like primitives." He's fresh out of college with only about a year of JS experience.
And this isn't even considering the fact that a large number of developers have already been using public property declarations
One of the beautiful things about this approach is that the effective result is nearly exactly the same as what you'd get from instance-properties. As soon as an attempt to modify the object on any level is made, an instance-specific copy is made and the modification applied to it. From that point on, it's no different than an instance-property at all. So in most cases, no code changes will be needed.
I think there are much simpler solutions to this problem.
Yes and no. Truth is, the way ES is defined means that there's going to be complexity somewhere when trying to solve this problem. With instance-properties, the complexity becomes semantic problems. Over the past few years, the proponents of class-fields have had to make some difficult decisions because of this. With my approach, the complexity becomes 2 issues only:
I'd prefer it if there were some way that the engine could accomplish this without using a proxy, so that the original value could live on the prototype, potentially be modified before class instantiation, and still produce a unique copy as soon as a modification request happens.
In comparison to instance-properties, it produces almost exactly the same result with 4 exceptions:
Other than that, the results are identical to that of instance-properties. For me, the value of 1, 3, & 4 along with losing the inconsistency of instance-properties in a class definition, making the [[Set]] vs [[Define]] debate irrelevant, completely restoring the viability of decorators in all contexts without sacrificing inheritance features, etc... (all the semantic problems with instance-properties), all while getting rid of a foot-gun, are worth the small amount of added complexity.
Wow. That was long winded. After saying all of that I say this:
That's not the only solution I've come up with, but it is by far and large, the most complete one.
@ljharb The idea above should directly address airbnb's issue. But if it is not suitable, there's another way, but it's definitely a code change.
class A {
let a = {}; // This object is instance specific since this `let` statement is evaluated in an instance closure.
get a { return a; }
set a(v) { a = v; }
};
Something else occurred to me. What if we take @mbrowne's suggestion of prop
and modify it just a bit so the above could be re-spelled like this:
class A {
let a = {}; // This object is instance specific since this `let` statement is evaluated in an instance closure.
prop a(a);
};
The definition would be something like;
DefaultAccessorDefinition[Yield] :
`prop` PropertyName `(` IdentifierName `)` `;`
where the identifier name must be an instance variable of the class.
So you’re saying that the code on the RHS of the let
would be ran per-instance - presumably at a specific time, like “at the top of a base class constructor, or after super
” - but it’d be private. (note that this would make it a “private field”)
prop propertyName(something)
would define a public property called “propertyName” (how would you handle computed names, and symbols? presumably with brackets), and the something
inside the parens would be only a private field name or an in-scope identifier? Why could it not be any expression, including referring to identifiers in the outer scope and calling instance methods? Restricting it seems like it greatly reduces the usefulness, and does not in fact cover the actual use case - which is “any code” evaluated per instance - without forcing a private field to be created solely to make its value public.
So you’re saying that the code on the RHS of the let would be ran per-instance - presumably at a specific time, like “at the top of a base class constructor, or after super” - but it’d be private. (note that this would make it a “private field”)
Exactly.
how would you handle computed names, and symbols?
The definition I gave above was a syntax fragment. Doesn't ES define a PropertyName
like this?
PropertyName[Yield, Await]:
LiteralPropertyName
ComputedPropertyName[?Yield, ?Await]
Why could it not be any expression, including referring to identifiers in the outer scope and calling instance methods?
I just hadn't thought that far ahead yet. I was just throwing out an idea to see where it landed. As long as the expression in the parenthesis is valid for both the RHS and LHS, I don't see why it shouldn't work.
without forcing a private field to be created solely to make its value public.
This is a key point. It just seems like unnecessary overhead to require the creation of a getter and setter, even if there is a concise syntax to do so, simply to achieve an effectively public property.
I've been thinking about that this whole time, but the first suggestion I made really is the only way to make that possible without causing a bunch of semantic problems like class-fields has. I realize that the second suggestion isn't exactly what @ljharb is asking for because it requires making an accessor property. However, given a shorthand for defining the accessor property, and given the fact that an engine can optimize this into direct access to the instance variable when the accessor is referenced, I don't see any real downside to the second approach.
The spec can’t assume optimizations; if the optimization is necessary to be performant, then it should be required.
I don't think of the optimizations as necessary for performance. I'm just aware that an extra tick or two can be gained via optimizations. The simple accessor property will already be fairly efficient even without optimization. Sure, not quite as fast as direct access, but for a clean approach that doesn't introduce unwanted semantic issues, and still gives you everything else that you need and most of what you want, this is a good and viable approach.
What is airbnb doing that requires a publicly accessible instance-initialized structure?
React components, primarily.
What are the odds that when some version of private appears, that these react components will still need to have a public object initialization?
100%, since the interface react requires needs to be public for react itself to access it.
That might not necessarily be true if I included a means of handling protected in this proposal. I've already thought it up, and it fits right in as just an extension of what this proposal is already doing.
However, I don't want to add that here.
The only reason I brought it up is because the 1 interface that react has (that I'm aware of) which works like you're describing is state
. If I provided some kind of protected interface support, then react could change state
to be such a protected element (given that it's not meant to be accessed publicly, but needs to be shared with Component
subclasses. If that happened, then public initialization with an instance-specific object would no longer be needed and the problem would just go away.
If that is a viable possibility, then I'm willing to add my protected (keyword: shared
) idea to this proposal. Since shared
members would still be non-property members, they would still benefit from per-instance initialization.
Protected is imo inherently inappropriate for JavaScript, and it would be a mistake to attempt to include it in this proposal.
state is the only instance property; altho there’s many static ones. It will remain public for the foreseeable future, it’s not a viable possibility to make that use case (not a problem) go away.
As I stated before, I don't want to add it to this proposal, but...
Here's a topic where I'd like to have another one of those offline conversations. From where I sit, instance-properties are equally as out-of-place in ES as you seem to think protected is. What's more, while the implementation I'd provide for protected carries no semantic oddities, instance-properties threaten to either break nearly landed future proposals or break existing functionality, with no apparent happy medium.
In a language like Java, there's no doubt that state
would have been declared protected. I realize that's not an argument in favor of adding something like protected to ES. However, it does show that the concept of classes in ES is still not complete, even after adding a mechanism for privacy.
React's state
property shouldn't be public because it's not meant to be accessed outside the inheritance hierarchy of Component
. At the same time, it cannot be private since only Component
itself would have access. It's only public because "there is no other choice." React isn't the only library/framework with that issue. In fact, if you went to the same library/framework authors that were questioned about facets of class-fields and asked, I'd be willing to bet that they'd tell you that a large percentage of the fields they currently mark with an _
should have a protected-like status instead of simply private.
I get that you don't think protected has a place in ES due to your binary view of accessibility in ES. However, following that kind of thinking to it's logical conclusion, private object data that is not internal to the engine also has no place in ES, yet that's what we're all trying to add. The only thing in ES that even resembles private space is a closure. Closures are only viable when a function somehow returns a function. So only functions have access to closures, not non-function objects.
We're trying to extend the language to do something people want but can't do easily. That means adding capabilities to the language that aren't really part of its paradigm. Such was the case for Symbol
, let
, generators, WeakMap, and many other features. It won't hurt ES to complete the basic class model by adding a way to share privacy in a limited fashion.
Tangent: While I don't think it changes the principles being discussed here, since there are plenty of other libraries that rely on public instance properties and will continue to want to do so, I think it's worth noting that Facebook is planning to stop using classes completely for new React components they write going forward: https://reactjs.org/docs/hooks-intro.html#classes-confuse-both-people-and-machines
I strongly disagree with their reasoning because I think it's throwing the baby out with the bathwater; you shouldn't just throw out classes just because there's a learning curve. It's important to note that React will probably always continue to support classes: "There are no plans to remove classes from React." I would like to say I'm 100% confident that the team at my company will continue to use classes for non-trivial stateful React components no matter what, but I have to admit that if in the future, React adds a bunch of new useful features to function components and starts treating classes like second-class citizens (which may or may not happen), then we might consider switching to all function components.
@rdking I have verified that React (as currently implemented internally) needs to be able to replace the state
object from the outside, e.g.: someComponent.state = newState
. This is just the nature of React's FP approach to state management, where the end result of setState()
is a new state object, rather than mutating the existing one. (I think this allows them to optimize things better internally, similarly to how using PureComponent
optimizes things in userland by taking advantage of immutability. Also similar to redux of course.) The need for a publicly writable state
property could be avoided if React used Object.defineProperty
to set the new state rather than a simple assignment, but I kind of doubt that the React team would be inclined to make such a change.
(I just realized that React's need to reassign state might not change anything about your proposed solution of using protected state to resolve this. But perhaps what I said is still informative :) )
@mbrowne What you've said about React is indeed informative. When I looked at the code for state, it does indeed appear to be something that needs to not be an own property by good coding practices. In fact, a good portion of the complaints React is using to justify their Hooks idea is predicated in part by an absence of a solution to protected in ES.
@ljharb Why do you think protected is "inherently inappropriate for JavaScript"? I'm not a big fan of it either because it conflates access control and inheritance, but there are times when restricting access only to an inheritance hierarchy is exactly what you want to do...for example a library that wants to give user-written subclasses access to certain properties or methods without making them public. I could certainly live without protected in JS but what is the inherent problem with it?
@mbrowne whether you want to do it or not isn't the point; there's no way to robustly share privileged access while fully denying it to others short of declaring all classes/functions in the same scope.
"private" and "public" in JS aren't "access modifiers", they're indications of reachability - a binary status. A thing is either reachable, or not.
@ljharb it sounds like you're describing existing mechanisms for whether or not a variable is reachable in the current version of JS. Why do we need to be limited to current features? Obviously JS wasn't built for access modifiers from the ground up, but would it be impossible to add proper support for them? The whole reason for TC39's existence as I understand it is to extend and improve the language... There may be plenty of reasons not to add this feature but I don't understand your argument; it sounds like, "We can't add this feature because the language doesn't currently have the prerequisites needed for this feature."
@mbrowne it's not about the ability to add support for it; it's that it's impossible to do it in a robust way, as far as I know, so that you can dynamically subclass a base class forever, and have that base class be able to share "protected" data with all subclasses, but deny access to it from non-subclasses - because I'd thus always be able to make a subclass and explicitly expose to the world the ability to read the data. JS isn't strictly typed - and strict typing is what tends to prevent things from accessing "protected" data in other languages.
@ljharb Ah, that makes more sense now, thank you. What about object properties that are accessible only within the same module? (So not talking about protected
anymore, but rather a separate concept of internal module state.) I brought this up a while back in the class-fields repo and someone pointed out that there's no way to implement this given the current modules spec. But would it be possible to add such a feature in the future?
I'm not sure because no such proposal exists - but I'm pretty confident that the current class fields proposal, nor any alternatives, would have any impact on that in either direction.
@ljharb
because I'd thus always be able to make a subclass and explicitly expose to the world the ability to read the data.
Isn't that what a final
keyword would prevent? Is that not possible to also implement in JS?
@shannon what would be the point of making a class that you intend to subclass, and then lock it down from future subclassing? Subclassing must be possible at any point in the future, because the child class doesn't exist until the parent class is completely evaluated - unless I'm misunderstanding what final
would do.
@ljharb maybe I am misunderstanding it as well but I would have thought you could set up several classes and sub classes internal to your library that have a protected (shared) field. Then you add final to a subset of subclasses and export them.
final
is a keyword; it's not something you can "add" to a class later.
What you're describing works well if everything is defined in the same file/module, but in that case you can already use closed-over variables without anything added to the language.
@ljharb yea sorry I didn't mean add it at runtime. I just meant add the keyword explicitly when you are defining your public API.
What you're describing works well if everything is defined in the same file/module
Yes but there is a proposal to allow trusted communication between modules: https://github.com/tc39/tc39-module-keys
@ljharb just to clarify I'm not trying to argue one way or the other, it just sounds like you are talking in absolutes and it seems reasonable to assume that there may be a path to this kind of feature in a robust way in the future.
@ljharb
What you're describing works well if everything is defined in the same file/module, but in that case you can already use closed-over variables without anything added to the language.
...except that's not an ergonomic solution in the case of object properties. I think you'd still be stuck using WeakMaps to do it correctly. And I'm not just talking about protected
here, but any kind of shared internal state.
I'm not sure because no such proposal exists - but I'm pretty confident that the current class fields proposal, nor any alternatives, would have any impact on that in either direction.
This is mostly true except for the syntax. The class fields proposal does limit the options for future syntax for native intermediate access levels. At the risk of repeating myself too much, here's a reminder of what that might look like given the class fields proposal:
class Demo {
myPublic
#myPrivate
internal #myInternal
}
As opposed to the more consistent option (public is inconsistent but the rest are consistent):
class Demo {
myPublic
private #myPrivate
internal #myInternal
// others, e.g. friend?
}
@rdking I'd be curious to see how your proposed shared
modifier would look as an addition to this (class-members) proposal. It seems we all agree it should be a follow-on proposal if anything, but it would be good to see how it would compare to the class fields syntax.
@shannon I get what you're saying, and yes. That would work. It'd look like this:
SomeModule.js
class A {
let shared foo = 0;
...
}
export default final class B extends A {
bar = -1;
...
}
Anyone importing SomeModule.js would get B
, which has access to its inherited foo
. No one would be able to extend B (using the class
keyword) and no one would have access to A
. So there's no chance of a class
being written that leaks foo from all subclass instances of A
unless SomeModule.js was rewritten with that intent.
@mbrowne A shared declaration would put a shared
keyword after the static
if it exists. So:
class Ex {
let a = 0;
let static b = 1;
let shared c = 2;
let static shared d = 3;
const e = 4;
const static f = 5;
const shared g = 6;
const static shared h = 7;
}
These would be the available forms. The shared
declarations would be placed in separate closures (still divided by static
) and lexically chained onto the corresponding instance/class closure.
@rdking It sounds like shared
members would be accessible only to the class where they're defined and any subclasses, similar to protected
in Java/C#. Given that, I suggest using a different word - "shared" could mean shared with anything. For example, it could be called "inheritable".
I'm not particularly partial to any keyword. That would be something to decide later. As it presently stands, though I know for a fact that this particular feature would be the next highly requested item, it would only work the way I'm thinking of it if class-members is the proposal that is implemented. In either case, as this shared member support is not about protecting data, I don't think it right to add it to class members. 1 step at a time, right?
Agreed. It's a suggestion for the future, not for right now.
It occurred to me that our difference in perspective can be represented visually (I think):
The curly brace on the left (blue) is how I see it and I think @ljharb sees it as well. The curly brace on the right (green) seems to be how you see it.
I think this is why we continue to fundamentally disagree. If when you look at a class declaration, you think about it like a prototype definition, then of course having it do things inside the constructor doesn't make sense. But I don't think it was ever the intent of class declarations for them to be limited to the prototype, but rather to be a full and standardized replacement for traditional patterns using constructor functions and prototypes—in other words, the whole shebang.
@mbrowne And no. That's not quite it. For me, this:
//Your View
const Person = (function() {
function Person(name) { this.name = name; }
Person.prototype = {
constructor: Person,
greet() { console.log(`Hi, ${this.name}!`); }
};
return Person;
})();
and this:
//My View
const Person = (function() {
let prototype = {
constructor: function Person(name) { this.name = name; },
greet() { console.log(`Hi, ${this.name}!`); }
};
Object.defineProperty(prototype.constructor, 'prototype', { value: prototype });
return prototype.constructor;
})();
are 100% identical pieces of code functionally with slightly different arrangements. That's not where our views part ways. It's here:
function Person(name) { this.name = name; }
Where you 2 want to treat this expression as a statement, I see something like this instead:
var Person = (function(args, body) {
return new Function(...args, body);
})([], "this.name = name");
The difference is that I see a function as an object with special semantics to execute user-provided script. If the user doesn't provide the body, then the function has only the absolute minimum required to ensure it executes. Nothing more.
What does that have to do with class
? Simple. If constructor functions didn't create instances by creating a new native Object (or other native type) and attaching the target prototype to it, then there would never have been any reason to add the class
functionality to the language. The not-quite-trivial steps that have to be taken to properly chain inheritance and make instanceof
work were all about the prototype. ES is a prototype-based language, not a constructor-based or class-based one. The manipulation of prototypes is the core feature that makes ES as powerful as it is.
Sure, the constructor function is an absolute must, but if the developer doesn't provide a body for it, the constructor function will still do its job of chaining prototypes together when you call new
. The only part of the constructor function that class
should be able to modify due to the definition is the constructor function object. The only modification class
should be allowed to make to the function body is the injection of super
when needed.
It should tell you something seeing that super
is not injected when the developer provides a body. Think about it.
Even if you provide the constructor, there’s still things happening inside it that isn’t directly written user code inside the constructor body. Class fields are just another one of those things.
Note that decorators will provide even more hooks into the constructor, and this finalization, and that ability is also an important part of class fields’ design. Decorators wouldn’t be able to modify lexical variables, but they can modify fields.
As for super not being injected, that’s also to permit legacy behavior to be implemented in new code - not because such injection is problematic.
Even if you provide the constructor, there’s still things happening inside it that isn’t directly written user code inside the constructor body. Class fields are just another one of those things.
Now that was a good rebuttal, but class fields will be the only thing happening in those in-between places that's not required to ensure the proper assembly of the resulting instance. What I'm going to say next looks and sounds like hair splitting, but thats only because it's 2 closely tangled hair strands. Class members does the same kind of thing, except in a more explicit fashion, and the result remains a part of the object.
With class-fields:
class
definition are all prefixed with this.
or this
.With class-members:
What's the difference?
So even though they're close, there's some critical differences. The fact that decorators won't work on lexical variables is a plus! Decorators a leak that should be avoided for anything private.
As for super not being injected, that’s also to permit legacy behavior to be implemented in new code - not because such injection is problematic.
What legacy behavior? I'm not sure I'm aware of this.
Note that the execution of the constructor is already “paused” at that exact moment - either prior to entering the body (in a base class), or while executing super()
- either of which is necessary to make this
available. This is adding another step in the existing location where user code either has not yet taken control, or has explicitly yielded it with a super call.
The legacy behavior where you need not call super
or use this
, because your constructor returns an object - namely, whatever the constructor returns ends up as the instance, not the thing you might expect entire class` to produce (ie, not necessarily having the produced prototype either).
@ljharb I already knew about that first paragraph, that's the first and last place where the two approaches are the same. After that, there are only superficial similarities.
As for that 2nd paragraph, why would the injection of super
preclude that at all? The concept is to inject super
into the first line of a constructor body that does not have super
on a class that extends a non-null object. If the constructor returns an object other than the one constructed via super
, what does it matter if super
is called? Isn't that scenario best served by a normal factory function?
Because in that scenario, the dev does not want the superclass constructor to run at all, because they don’t want to use the this
it creates.
Then why use class
at all for that scenario? It's the wrong tool for the job!
That’s your subjective opinion; but because old-style constructors allowed it, and thus class
is automatically the right tool for the job, since it produces a constructor.
@ljharb @mbrowne
If I understand what's going on in #1, the 2nd problem being targeted is the need to declare a property initialized with an object, and having that object be an instance-specific copy each time. Is this what you guys really want? If so, this is something good that I see as worth doing, but I have a much better approach than the problematic implementation that exists in class-fields. It doesn't introduce "fields" of any kind. It doesn't require trying to make a declarative thing from something inherently imperative either.
Here's the idea: Since this proposal resolves all property initializers at the time of
class
definition evaluation, should any property be initialized with an object, that objects will be wrapped in a Membrane with a handler that traps any attempt to modify any portion of the object. When any such trap triggers, the property is duplicated onto the instance with the original value, and the modification operation is forwarded onto the instance.Basically, this:
would become this:
So what do you guys think?