-
Notifications
You must be signed in to change notification settings - Fork 113
Public class fields is problematic. Please hold. #193
Comments
Unfortunately, this broken proposal have already achieved stage 3, and Chrome have already shipped public field and will enable it by default in next month. I asked @mathiasbynens whether it's possible to postpone it, but seems have no luck. When it was enabled by default, this broken feature will be introduced to production. The sunk cost will be very huge, and I'm afraid we must just accept it as a "new BAD PART" of JavaScript. So I don't know there is any real way to "hold back". The only possible way may be some TC39 member decide to veto it in last minute and force google to stop shipping it. But I think it's very impossible, no one want to take the social cost of veto a stage 3 proposal. Technical part (though meaningless in current situation): There are several solutions, maybe none of them are perfect, but I believe they are all better than current broken state.
Note, the first two solutions also need change the syntax (add keyword), because current no keyword syntax just implies [[Set]]. And adding keyword also fix the issue of new ASI hazard. |
It would be a sorry thing if it will ship finally. I also see the current attitude of the committee, but I still want to ask if there's really equal people that are fond of the Is there possibility that we recall it if we get wide negative feedback after it's shipped? We still have several months before public release. (true?) @littledan
The first 2 solutions are trying to make the current design not so 'broken', but it is still hard to be a future proof design until you mimic out the concept of 'subobject', if said in c++ term, or you stay with properties and safely only introduce initializers. |
Yes the backing store still has to be on instance, but the backing store is private, so each class will have their own store for each accessor. Basically alternative 1 is just class X {
<keyword> x = 1
} desugar to class X {
#x = 1
get x() { return this.#x }
set x(v) { this.#x = v }
}
I don't understand your point, alternative 2 is just desugar to class X {
constructor() {
this.x = 1
}
}
Object.defineProperty(X.prototype, 'x', {enumerable:true, configurable:true, value: undefined}) Both solutions just solve the conflict of [[Set]] vs [[Define]] issue, because they use [[Define]] semantic on prototype. The only difference is the first one use private field as backing store, the second still use own property.
I would like to hear more details about that. In my opinion, the first one is just work like it works for most other programming languages. |
If private field works like each class have their own store for each accessor, that's exactly what I'm calling for. (I'm only familiar with the public field part of this proposal, so not aware of this.) It will be a competitor of the For 2, following your code, if class
I just know from above that private field enables 'subobject'. In c++, "base subobject" basically means "the base part" of an object of the derived class. With private field we have it. So I agree with you. |
@panlina To some degree, I agree with you. But the point is this solution at least avoid the footgun of current proposal. Consider current situation, I would like accept any solution which is practical work, even it have some small flaws. The root cause of the footgun is defining an own property which contradict the prototype-based accessors. The two factors are:
Solution 1 drop own property. There are many discussion about the problems of solution 3 (use [[Set]]), so I don't repeat it again. It's very hard to convince TC39 [[Set]] is better. The problem of solution 1 is some TC39 delegates want own property anyway though none of their reasons can convince me. So the solution 2 seems the most possible solution which may be accepted by TC39. (But I'm not sure now, it seems they just don't want to revisit this issue.) Both solution 1 and 2 still use [[Define]] which may satisfy many who never accept [[Set]]. Personally, I much prefer the solution 1, because it follows the common OOP practice in most other programming languages. We are designing features for classes, and ES6 classes was designed to follow the behavior and experiences of other OOP programming languages. It's very bad that we are now breaking such consistency between JS and other OOP languages. IMO, Solution 2 is not as good as solution 1, but at least avoid the footgun. So I can also accept it. The real problem is, some guys in TC39 just don't want to revisit it anyway, so anything we did is meaningless. Sigh... |
What you said is true. There is no one other than us discussing now. 😶 |
@hax @panlina If each constructor created its own instance object and populated it, with As an example, suppose you had 3 classes (A, B, C) with each successive class extending the former. Instantiating C would produce this prototype chain: iC, iB, iA, pC, pB, pA, pObject, null. This is just one of the paths that could have been taken when implementing But sadly, we're stuck with the current half-implementation, and the class-fields proposal seems unstoppable. It isn't like there aren't better proposals. It isn't like there's 100% acceptance within TC39. However, with the strange voting process they use, silence at voting is tacit agreement, even if the member has been otherwise vocal about dissent. |
The iC, iB, iA part is hopeful, but pC being the prototype of iA is not correct I think, because it means iA will have C's methods. The right structure should be a lattice, not a chain. |
I get what you're saying. Such a thing can be simulated with Proxy, but that's simply the path not chosen where ES is concerned. I'm curious to see what you think about this proposal. I've taken the best parts of the existing proposals (and a few not good but highly demanded parts) and rolled them into 1, eliminating the worst parts about each along the way. Feel free to post issues there for any suggestions you may have. |
Interesting. I'll take a look. |
It seems like you have concerns with the Set vs Define decision. I understand this has been controversial, but I think JavaScript developers will really benefit from public fields being shipped broadly to browsers, so I have not tried to hold proposal back. Chrome 72 ships this feature, with Chrome engineers being fully aware of this controversy, so I don't think there's much to do at this point. I'm a bit confused by @hax 's comment in this thread: defining a data property on the prototype would lead to strange, unexpected behavior, and defining an accessor on the prototype would lead to a continuation of the behavior where the superclass setter is not called. |
@littledan I'm very surprised that you close many issues related to [[Define]]/[[Set]] while you are confused by the comments of mine!
What I refer is #176 . It use semantic [[Define]] on prototype then [[Set]] on instance (for initializer) which will create own property on instance, so it will not lead to "strange, unexpected" behavior which is caused by only [[Define]] on prototype.
You should notice I also point out the syntax should be changed (add keyword), which eliminate the confusion of [[Set]] semantic, so "behavior where the superclass setter is not called" is consistent to the "override" semantic of current ES6 classes. |
Oh, sorry for my misunderstanding, but using TC39 spent a long time in 2016 and 2017 debating whether we should add a keyword for field declarations, and ultimately concluded that adding a keyword wouldn't clarify anything in particular about semantics--it has to be learned, either way. |
@littledan Seems like TC39 may have overlooked the ASI issues of not having a keyword in the declaration. What of those who prefer their semicolon free style. I don't get it, but they seem to love it. The absence of a keyword allows for many different kinds of mistakes depending on the chosen property names. There's really only 2 ways to fix this: either add a keyword, or restrict the namespace for public class fields. |
@rdking it wasn't overlooked; we explicitly decided that adding the ASI hazard is preferable to requiring semicolons or having a keyword in this case. |
Also, most of the hazards would not be fixed by a keyword. The only hazards which a keyword avoids are fields named class Foo {
x = 1
*y(){}
} and so on. |
@bakkot @littledan |
No, they aren't.
Keywords have their own problems discussed at length elsewhere. As to requiring semicolons, everyone I've talked to who uses the semicolon-free style preferred that there be a few more exceptional cases they (or to be more precise their tooling) have to deal with, rather than having to insert a bunch more semicolons. There's a lot of old discussion on github, if you're interested (x, x). And see notes and slides from one of the occasions it was discussed in committee. |
None of the discussions I've seen for the so-called problems of keywords are anywhere near as bad as having the engine mis-parse my code. So care to be more detailed about even 1 problem that is? Let's try this from a different angle. Without keywords, there are several new rules required to prevent ASI. With keywords, there's only 1:
Thats far easier to remember and follow than what you're currently offering. |
I already explain the semicolon problem several times. The core value of leading-semicolon (semicolon-less) coding style is you never need look backward. So But
Obviously anyone can have any problem. We need to compare the whole cost. Having keyword solve:
Is there any issue of having keyword will be worse than that three ??
Obviously I'm not in "everyone". And as I already explained many times, many people (include some who are using semicolon-less style) misunderstand the benefit of semicolon-less style. It's not about the count of semicolons, but how you decide to insert the semicolon. |
I already explain it several times. Let me try again. There are related but separate issues of current broken public field syntax and semantic.
Please apply this three to check the alternatives I wrote, you should understand what I mean -- With keyword + [[Define]] (on prototype), we just solve the first two. You could also ask @rbuckton to explain to you because I guess he know what I mean. |
That's only even approximately true given certain other style conventions. For example, you cannot insert a semicolon before the second line in if (foo)
(function(){ bar() })() As far as I know people are fine with that because if they use then no-semi style then they also enforce a rule which says that Well, just the same you can have a rule which says that unquoted class properties should not be named |
This is just the main stream coding style (both for always semicolon style or leading semicolon style), so it's very ok.
This is not practical, at least up to now. All major liner/formatter need new rules! (And it will bring trouble to prettier, because such rule is out of the scope.) And even it's possible, you are transferring the cost from committee to the community. The cost of committee only affect 100 people (TC39 delegates, to be true, it may only affect 10 people --- many TC39 delegates never mind this proposal). And the cost of community affect 1,000,000+ people. Especially, the whole community used 10+ years to make most programmers agree both coding styles are ok. Now you are introducing the storm to against leading semicolon coding style again. This is very harmful. |
Yes, we had a very extensive discussion about ASI; I believe it was discussed at 3 or 4 TC39 meetings, and especially in the second half of 2017. We also had extensive threads about it on GitHub (and Twitter) with very strong community support for the selected syntax. I don't understand the objective of re-raising this issue. Are you really unhappy with the result? I know some people find either decision there better, and some people would prefer we would use a keyword or something other than =, but here, I honestly believe we chose the community's preferred option (at the risk of some potential future language maintenance burden). Honestly, at this point, it feels like arguments are being brought up not for the sake of trying to help improve the feature, but rather just to make the generic case that this proposal should not be shipped. I hope I am wrong about that; such a discussion style doesn't seem so productive. |
@littledan This is false. I have already given speech about this proposal twice, 70%+ gave negative feedback to this proposal. And my impression is, the senior programmers gave more negative feedbacks. (Note, as I told you before, I tried my best to give all information I know and warned all audience that some TC39 delegates never agree my opinion and I may have bias, if they have any doubt about what I'm saying they should believe the decision of TC39.) And @Igmat also gave you a similar feedback. |
@littledan To be honest, I don't think we will have any productive discussion in current situation. You always told all objectors (inside or outside the TC39) we already know the issues and we already have the "consensus", and we won't revisit it. So the only choices are:
I already told you this is the process failure. Such process failure can never lead to any "productive" discussion style. We are just wasting the time both of us. |
To clarify, I meant the community's preferred option with respect to the ASI issue. |
You forgot the storm of tc39/ecma262#1062 ? |
The choice in this proposal was to not require a semicolon, and the storm was a reaction to a recommendation to use a semicolon. I imagine the people who don't want to be recommended to use a semicolon, would feel even more strongly that they not be required to use it. |
I will write them myself if that ends up being an issue. They'll need new rules around class fields anyway, as with most any new syntax.
Putting braces in my But as someone who is technically a collaborator on prettier, albeit a fairly inactive one, I don't think this is out of scope. (We wouldn't add an option for it, but could easily decide to enforce it unconditionally, or conditionally on using the no-semi option.) I'm not speaking for any other maintainers, of course. |
That's backwards. The proposal shouldn't be shipped because so many issues like this remain unfixed. If these issues didn't exist, if decorators was guaranteed to ship together with class-fields, then I wouldn't be putting up nearly as much of a fuss as I have. The fundamental truth is that given the paradigm chosen, TC39 has done the best they can. We want TC39 to recognize that the problem is the paradigm itself!!!! It is the very approach taken that has lead so many of us to keep boring you with the repeated re-raising of issues. @littledan @bakkot @ljharb |
This proposal ruin the core value of the semicolon-less coding style, and you told me people would feel more strongly they not be required to use semicolon??? I can't understand your logic. Actually it's even worse than that PR. The PR only do "recommendation" which the community of semicolon-less coding style already heard from JSLint days, nothing new. But this proposal add new "evidence" that semicolon-less coding style is "complex and impractical". |
Ok, you'd better start writing them now because it will land in chrome next week.
This is false. If public fields is keyword based, we never need new rules for dealing with semicolon, and only need to update current semi rule (add
I'm sure it's out of scope even I didn't use prettier much and never contribute to it. Prettier is a formatter. How a formatter can enforce you not write a field named as And as a formatter, it should never change the semantics. So code var a
[a] = [1]
if (x)
[a] = [1]
test
[a] = [1] will be formatted to var a
;[a] = [1] // leading semi is added even it's unnecessary
if (x) // assume this line is long enough to avoid the next line being collapsed into a single line
[a] = [1] // leading semi is not added because it will change semantic
test[a] = [1] // Note this! (you can run prettier to learn it, I also suggest you add comment after Similarly, class {
get
f() {}
} will be formatted to class {
get f() {}
} So prettier never solve ASI hazard by itself, it rely on the author (or linter executed before prettier) to write correct code in first place. Some team may use prettier without eslint/tslint, and even use both, a common practice is disabling stylistic options in eslint/tslint and leave them to prettier. See eslint-prettier-config and tslint-prettier-config. So basically many prettier users are actually writing code without semicolon check. 😂 It's ok because semicolon-less coding style is practical even without linter. But with this proposal, as I said, ruin the core value (no look backward), just make it impractical. |
@hax, I think you'll find that a lot of people will disagree with you about what the value of that style is. Many of its adherents just have an aesthetic dislike of line-ending semicolons.
It can choose to format
I helped implement prettier's behavior here. I know what it does, thank you.
I would expect anyone reading prettier's output there would understand that the code they'd originally written did not do what they intended, just as happens when it formats var a = 1
/b/u.test(foo) as
I continue to think "class field and property names which are keywords should be quoted" is not an impractical rule to learn. |
I think you'll find most people using semicolon-less style will agree me after listen my speech about semicolons. They "disagree" just because they never been told why semicolon-less style is invented in old days without linters.
As I explained, it doesn't solve the ASI errors because the author already write the wrong code. And the weird syntax
Impractical.
Yeah, it just prove my opinion. If prettier has problem on this case, it will have much big issue on the worse cases like this proposal.
Not as easy as you think. We need a new ESLint rule (prettier doesn't work as I explained) which report error for code like class X {
get
f() {}
} This is a new behavior because currently linter never complain. Normally such new behavior rule will only be added to recommended in new major version, so the adoption will be slow. It also can't be auto fixable because it will change semantic. And, as I commented before, you are just transferring the cost from 10 people of committee to 1000000 people of community. |
@hax the only other choices I'm aware of are 1) require semicolons on public fields, which would upset ASI users; 2) use a previously-reserved keyword, which has been discussed many times and decided against (and Class fields will warrant many new eslint rules, to be sure (as do most syntactic language features) - but major versions in a linting config (like airbnb's) tend to be adopted rather quickly, so I don't expect this to be a problem. |
A keyword for field declarations wouldn't eliminate all potential issues either, as you could have a field declaration followed by a method. We've discussed this in depth, and @hax has already referenced these previous discussions upthread. I'm not sure if it's productive to continue revisiting all of our past design work here. |
This issue has been going in circles, let's give it a break and try another approach. |
I'm not so involved in this proposal as many others here before, but recently I've been studying this proposal, including the
[[Define]]
vs.[[Set]]
debate. After that I find the idea of class fields is problematic in the current way of implementation.JavaScript is well known for its prototype based OO. It let every object have a prototype and property accesses goes up the prototype chain, and that makes it look like the object inherits properties from its prototype. This feature is natural to implement OO.
However, JavaScript does not tell you how this mechanism is mapped to OO concepts.
new
operator will do.Either is using the prototype to propagate property sets. Unfortunately, there's the restriction of prototypes in the language: an object can only have one prototype. We have only 1 channel but 2 threads.
Inevitably, we have to cut off one path. In es6, we choose to cut off the prototype chain of inheritance, and what actually happens is that we do not create instances for every base class, instead we let them be the most derived object itself.
As shown here, prototypes of instances form decent chain, but there's no such for instances themselves.
Now based on this selected model, we are introducing fields. We must not dismiss the fact that:
We do not have instance prototype chain anyway!
Due to the absence of the instance prototype chain, field becomes a false concept. Any operation performed by the language that
[[DefineOwnProperty]]
on the instance for fields makes no sense and/or introduce inconsistent behavior. Many examples are given in this repo.I'm not saying that fields are not possible in JavaScript. Conceptually, they are nearly trivial, and
[[Define]]
is the obvious choice to take--in class context, we are declaring it, so why[[Set]]
(To clarify,[[Define]]
is only for the declaration part, not the initialization part)?[[Define]]
ing data properties on each level on the instance inheritance chain should work consistently and intuitively.The conclusion to say field is a false concept is based on the object model since es6. It is the essential cause that is making public fields proposal so controversial, but we can not abandon it.
So it is hardly possible to let JavaScript have fields resembling other OO languages in next version. Currently my opinion of the public fields proposal is:
[[Set]]
semantics, and would be better to reconsider it asproperty initializer
.[[Define]]
be the standard. 😢PS:
To expand a little if it's not clear, another model can be to cut off the other path. We let instances form prototype chain, and manually copy methods onto instances. But this gives up the benefits of built-in
new
operator and methods sharing.To expand further, if we are not limited to only 1 prototype, for example, introducing
[[InstancePrototype]]
, this difficulty can be solved with less trade-off, but will bring complexity, though.The text was updated successfully, but these errors were encountered: