tc39 / proposal-class-fields

Orthogonally-informed combination of public and private fields proposals
https://arai-a.github.io/ecma262-compare/?pr=1668
1.72k stars 113 forks source link

Suspend releasing class-fields to stage 4. #203

Closed rdking closed 5 years ago

rdking commented 5 years ago

Since we can't seem to make TC39 listen about the true severity of the issues in this proposal, at least suspend the release until there is some actual reality to the work-arounds that have been proposed for all of these problems. Class-decorators is still a stage 2 proposal. This means it is still subject to being tossed aside if an insurmountable issue is discovered. Only stage-3 seems to guarantee stage-4.

Meanwhile, in the current stage-3 proposal, many of the solutions for the numerous non-trivial problems in class-fields have been heaped upon class-decorators. Releasing class-fields to stage-4 without class-decorators is therefore a very good way to break the language. Anyone wanting to use class-fields would still have to use Babel to work around the issues. This is no different than if those same developers just used Babel to implement class-fields. There would be no advantage to releasing class-fields without class-decorators, but much harm can be done if class-fields is released alone.

rdking commented 5 years ago

@littledan If you're really interested, it would only take 1 additional internal slot on an object. That slot would hold either null or a reference to the constructor function that triggered its creation. With the constructor function, the private field map could be found. That would enable adding via definePrivateProperty and accessing via getInstancePrivateProperty.

Side thought: That would also provide a means for creating a non-faulty "instanceof"-like operation as well as a fool-proof means of brand-checking.

lifaon74 commented 5 years ago

@rdking: I totaly agree on your https://github.com/tc39/proposal-class-fields/issues/203#issuecomment-451960650 If possible put the private aspect on the prototype (which could allow retro-compatibility with existing es5 prototype/function code) and still allow the # notation for es6 classes. And keed a backdoor: a way to force access to these private properties to fix unmaintened code, fasten code lib execution and allow friend classes.

1) Yes, I'm for "soft private" (at least kind of): private field should be scoped with the instance (not accessible by child classes nor external calls/gets), but we should allow a backdoor for "friend" code, else we get back to a shared weakmap for the entire package/lib, which is less convenient and expressif for other developers than a # property. 2) Symbol are not "soft private" because they are shared with child classes and external code ! They are juste "hidden" properties in the case of a class.

We need and want instance encapsulation (what does weakmap) but a backdoor is necessary for special cases as enunciated previously

rdking commented 5 years ago

@lifaon74

Symbol are not "soft private" because they are shared with child classes and external code ! They are juste "hidden" properties in the case of a class.

That depends on how you use them. For instance:

let Example = (function () {
  let pvtField = Symbol("Private");
  return class Example {
    constructor() {
      this[pvtField] = 42;
    }
    print() { console.log(`My private field holds: ${this[pvtField]}`); }
  };
})();

new Example().print(); // "My private field holds: 42"

Unless you call Object.getOwnPropertySymbols or Object.getOwnPropertyDescriptors to retrieve the field name, you can't see and don't have access to [pvtField]. That's soft private. But like I said, not very ergonomic.

littledan commented 5 years ago

Re https://github.com/tc39/proposal-class-fields/issues/203#issuecomment-451965556 , Huh? You have a reference to the constructor function when you're outside of the class.

rdking commented 5 years ago

Maybe. Maybe not. The code trying to manipulate the private state may not be in the same scope as the constructor function, and the constructor on the prototype of the instance may have been removed.

littledan commented 5 years ago

I just don't know what you're proposing.

rdking commented 5 years ago

Not withstanding @lifaon74's desire for an ergonomic syntax for "soft private", it looks like the desire is to have a way to apply private data to non-class-instance objects. This is something I brought up 3 years ago, and again last year. While TC39 is so heavily focused on only providing private data support for class, there's a large sector of the community that would like to be able to use the feature without the constraints forced by the class keyword.

mbrowne commented 5 years ago

@lifaon74 You might be interested in #189. I'm not sure this thread is the best place to further discuss hard vs. soft private. It seems like this thread should be more about whether or not the absence of soft private should warrant changing or delaying this proposal. I understand why you are saying the current proposal is unacceptable without soft private, which is an issue I care a lot about too, but I think the committee's plan for handling this is reasonable. Decorators will make it possible to provide such a "back door" for private fields, and thanks to Babel it will be possible to go ahead and use them. And hopefully the period of time during which class fields are supported natively while decorators are still in development will be relatively brief (decorators are already in stage 2 and a lot of progress has been made on them).

rdking commented 5 years ago

@mbrowne

It seems like this thread should be more about whether or not the absence of soft private should warrant changing or delaying this proposal.

Not quite. Mine isn't an argument about whether or not soft private should delay this proposal. Of course it shouldn't. Soft private already exists in the form of Symbol. It just needs a more ergonomic syntax. That can be handled at any time via another proposal.

My argument is that TC39 knows of several issues that the community will have when trying to use class-fields in production code. TC39 has opted to nominate the decorators proposal as the solution for those issues. However, if class-fields is released now, with decorators still very much in development with no release in sight, that means that the promised techniques for resolving those issues is also no where in sight at least for the next few years. Meanwhile, production code development using class-fields will have to suffer either trying to work around the issues or simply not making use of class-fields due to the unsolvable issues. As such, it is detrimental to the community for class-fields to be released without also releasing decorators in parallel.

ljharb commented 5 years ago

@rdking we’ve got one group of users who wants to use class fields and doesn’t need any of the decorator solutions; and your group who needs both. you’re suggesting that forcing both groups to wait longer for class fields is somehow less detrimental than allowing one group to use them sooner?

lifaon74 commented 5 years ago

They can already use them sooner though transpilling... better than an incomplete proposal...

rdking commented 5 years ago

@ljharb I'm suggesting that it would be better to release a more complete solution than a less complete one. I'm suggesting that those who are waiting for class-fields is probably already using it via Babel. I'm suggesting that due to the grand amount of controversy over this proposal, it would be better to err on the side of caution than to rush out with something known to have issues.

Since there's apparently no way to derail this train, there's no harm in those wanting the proposal to land now using Babel. But for those of us who need those holes plugged, releasing now would be counter-productive, damaging, and utterly useless. So yes. That's exactly what I'm saying. It's not a suggestion, but a reality.

ljharb commented 5 years ago

@lifaon74 and you can already use decorators through transpiling, so I’m not clear on what the benefit of delaying to advance both together would be

rdking commented 5 years ago

@ljharb @littledan It's like this: I want data syntax in classes just as much, if not more, than the next developer. I'd be one of your strongest proponents if this proposal wasn't so riddled with issues. I think I made that clear with #144(comment). I'm just not willing to rush something out there when I know it has significant unsolvable problems.

rdking commented 5 years ago

@ljharb

@lifaon74 and you can already use decorators through transpiling, so I’m not clear on what the benefit of delaying to advance both together would be.

Once class-fields reaches Stage 4, there's a reasonable expectation of quality and stability from the community, right? Are you really willing to face the community and tell them that so many of the things in their production code that doesn't currently need Babel at all now requires Babel in order to solve the problems? By releasing class-fields to Stage 4, there will be those developers who try to use class-fields only to find that they now have a Babel dependency if they expect it to work the way they want. The benefit in delaying? Not unnecessarily damaging the language and TC39's credibility in the community.

ljharb commented 5 years ago

I don’t understand why anybody would be forced to use class fields, or why the absence of decorators in a non-babel-using codebase would impact them (they’d just not use class fields for a little while longer).

Note that I’m responding solely to the point about tying decorators and class fields together; if decorators were stage 3 today, and both proposals were as-is, that point would be resolved. What’s the benefit of delaying to wait for decorators, but with no changes to class fields?

rdking commented 5 years ago

Somehow, I guess I'm not using the right words for you to hear me.

Facts:


I know you're ok with everything so far, but the problem starts now.


Prediction


So what's the benefit of delaying? With class-fields being released along side decorators, adoption rates won't suffer. Developers won't waste time integrating them into production code only to find they don't fit the need. As much time and money won't be spent rolling back failed attempts to use class-fields and writing angry articles. Put succinctly, TC39 stands to loose more credibility than it stands to gain by releasing something it knows will cause issues not directly solvable within the language itself.

mbrowne commented 5 years ago

@rdking

Not quite. Mine isn't an argument about whether or not soft private should delay this proposal. Of course it shouldn't.

Just to clarify, my comment was in response to @lifaon74's comments. I realize that you have many different objections to this proposal advancing to stage 4.

ljharb commented 5 years ago

@rdking note that because of browser support, it’s highly unlikely that a codebase not using Babel will use new syntax features for many years; most sites still can’t use untranspiled arrow functions, as an example.

rdking commented 5 years ago

@ljharb Remember the last time someone fed a TC39 member that kind of argument? I believe the response was something to the effect of "the browser isn't the only target environment for ES".

ljharb commented 5 years ago

Very fair point.

mbrowne commented 5 years ago

Ok, let's talk about node.js as an example then. If some teams want to start using class fields without having to use Babel, and they are perfectly OK with the absence of a way to implement protected and soft private and are also fine with the [[Define]] behavior for public fields, why should they have to wait just because decorators haven't yet shipped?

rdking commented 5 years ago

As discussed before, there will be 2 classes of these developers, run into insurmountable issues and those who don't. I'm not worried about those who don't. They'll be just fine. I am worried for their code base, though. Among the ones who don't run into issues of their own, there are 2 groups: those writing in-house-only code, and those writing libraries/frameworks/modules. Again, I'm not worried for the 1st group.

For both of the 2nd cases, there is a major issue. They've all run into an insurmountable problem that cannot be solved without either including a transpiler or backing out all of their work. 1 group experienced it during their own development. The other had it reported to them from their users. In either case, it's not a good look for TC39, and sours developers on adopting something TC39 thinks of as a good proposal.

rdking commented 5 years ago

Let me add that in node, the shared code problem is especially egregious. Consider that a single node package can have dependencies, that have dependencies, that have dependencies, ad nauseam. This makes it significantly more daunting trying to find the cause of the errant code.

This may be just my imagination being facetious, but can you imagine node modules having to add

WARNING: Code contains Private Fields

near the top of the README.md? Not a good look.

ljharb commented 5 years ago

@rdking this is also why most node packages don't use new syntax, either - because they want to support older node versions. Typically it's only safe to rely on a top-level node app being able to use modern syntax.

They already, thus, have to have such a warning for any post-ES5 syntax - or alternatively, a warning about which node version is used. No warning would be needed for private fields alone, just like there's no warning needed for "this package isn't Proxy-safe".

rdking commented 5 years ago

@ljharb Like I said, I was being facetious. But did you notice you said "most" and not "all"? Isn't it one of your quotes that says "if it can be done, someone will do it"? My point remains. All it takes is 1 decent reusable package with private fields in it to pose a litany of issues that are difficult to track down and impossible to fix while still using private fields. Didn't you also say that "non-0 risk is unacceptable" when you made that other quote?

ljharb commented 5 years ago

@rdking sure. but it depends which "issue" you're talking about. Most of the things that decorators help with are problems for the class author - not problems for any of the consumers. Proxy-related stuff is already something you can't rely on without some cooperation from the author - if they choose to use private fields, they've chosen not to be "proxy safe" in some ways, and that's something you have to negotiate with them regardless of what language features they use.

rdking commented 5 years ago

@ljharb Not true about Proxy. I've already demonstrated that with user-land code included before any other library, I can fix the Proxy/WeakMap problem with less than 100 lines of trivial code (that's pretty-printed lines). I don't need the authors cooperation if I can directly interfere with how Proxy gets used.

Where private fields is concerned, it wouldn't likely be the case that they've willfully chosen to not be proxy safe. Just like now, it usually happens inadvertently, because use of the code with a proxy wasn't considered during the design and testing. That's very different from willfully chosing. But your point remains true nonetheless.

That being said, since Proxy-related issues were not on the list of issues TC39 pushed off onto the decorator proposal, your whole argument this time is kinda moot.

ljharb commented 5 years ago

What issues, to your mind, require decorators, but affect more than just the class author (who naturally wouldn't choose to use private fields without decorators in those situations)?

rdking commented 5 years ago

The biggest 2 are the [[Set]] vs [[Define]] problem that breaks upstream and downstream inheritance, and the lack of "protected" support. Both of these issues affect both the library developer and the library user concurrently. If a library user inappropriately overrides a developer's field, that could break the base class in some unexpected way. The same problem happens in reverse. If there is no "protected" support, there's no way to constrain monkey-patching, so a change by the developer can hurt the user, while a big enough user can constrain advancement of a developer. In other words, one of the biggest reasons for wanting full encapsulation and hard privacy would be left unsolved.

rdking commented 5 years ago

Just a side thought: I was just thinking about something. I don't think I want private fields at all now. If public fields, decorators, and pipelining released, without private fields, then there would already be a relatively easy syntax for creating and accessing private data.

ljharb commented 5 years ago

Thanks for the reply.

I don't see how "protected" support plays into that - if you mean hard privacy, then yes, i'm very aware of the dangers caused by delaying native hard-private instance properties.

As for Set vs Define, with the current choice of Define, decorators would be one way you could write a field that used Set, but you'd still have to remember to do that - lacking decorators, if you remembered, you'd choose not to use a field for that property and do the assignment in the constructor instead. I'm still not clear on why it would be harmful for "users who need Set" to do what they're already doing - assignment in the constructor - a bit longer, while "users do need Define" and "users who don't care which is used" could use fields?

rdking commented 5 years ago

It's harmful because it encourages usage.

" Hey world! TC39 has finally released a method of declaring instance properties in the class definition! They call them "fields".... blah, blah, blah."

After all the waiting we've been doing to get anything at all like that, how can you be so certain that the vast majority of developers won't forget about the gotcha until it get's them? Remember, if it can be done, someone will do it. So why is a non-0 risk suddenly ok?

ljharb commented 5 years ago

Because in this case, it's unavoidable - whether we choose Set or Define, with or without decorators, there's always the risk of someone accidentally using the wrong semantics.

As an aside, it's very off-putting when you try to weaponize my out-of-context quotes to make an unrelated argument.

rdking commented 5 years ago

@ljharb It's not weaponization, but rather just a kind reminder of the standpoint you claim to hold when you appear to be contradicting it. Feel free to do the same for me if I contradict myself.

It's not unavoidable. There are plenty of ways to avoid it. However, for various reasons that have been ground into atoms by discussions in these threads, TC39 refuses to avail themselves of those working, and based on pre-existing and proven concepts, methods. For instance, "only define properties on things you created". From this standpoint, it's fine for a constructor to write properties onto an instance of its class. That's the normal approach, right? It's also fine for a class definition to write things onto the constructor and prototype. That's also the normal approach, right?

I'm not trying to re-hash the relative merit of that approach vs what was chosen. So please don't. What I'm saying is that your claim of unavoidability is invalid.

bakkot commented 5 years ago

@rdking, I think @ljharb's point is that sometimes the thing someone wants is [[Define]] and sometimes it is [[Set]], and sometimes they will do the wrong one by accident, and that is not avoidable. Rules about how to write code which amount to "they should stop wanting that" aren't really a solution.

rdking commented 5 years ago

Allow me to paraphrase TC39's standpoint, but from the point of view of an outsider like myself:

"Let's hurry up and release this issue-riddled proposal. There's a small percentage of the target developer audience that would appreciate having it sooner rather than later. It doesn't matter that the vast majority of the target audience will not be able to benefit from it, or that it will block various use cases if that small percentage happens to be releasing shared code, or that it doesn't really solve 2 of the target goals of the proposal due to lack of sharing provisions."

@bakkot Believe me when I say that I realize this already. That's why when I broke down and included public fields in class members, I put in 2 different syntaxes. It makes no sense for there to only be 1, and the 1 chosen is the most technically damaging. I never once claimed that anyone should stop wanting a means to declare data properties in class. I want that too! I just want it in a way that fits the existing language and is good for the existing language. In this case, so much work was done to avoid a 1:4 footgun that it created a much bigger one. The 1:4 footgun is much easier to solve on it's own than the approach being taken. And all the peripheral wants surrounding the syntax chosen could be handled by much more effective, much less damaging means. So color me dumbfounded.

bakkot commented 5 years ago

It doesn't matter that the vast majority of the target audience will not be able to benefit from it

This seems like our core point of disagreement. I expect the overwhelming majority of the target audience will be able to benefit from this proposal.

bigopon commented 5 years ago

I expect the overwhelming majority of the target audience will be able to benefit from this proposal.

The target audience this proposal benefits will give a burden to non target audience who have to maintain later.

Also can we have a section in the readme stating clearly who would belong to target audience? TS users definitely are not. Lib authors, application developers ? etc...

ljharb commented 5 years ago

@bigopon why are TS users not? It seems to me like many TS users would prefer "actual" privacy: https://github.com/Microsoft/TypeScript/issues/564 https://medium.com/@pagalvin/private-members-in-typescript-are-not-always-private-962fd220d749 https://stackoverflow.com/a/12713869/632724 etc. I can't find the source, but I've been told that "actual privacy" is one of the most requested TS features (hopefully someone from the TS team can confirm/clarify)

Igmat commented 5 years ago

@ljharb, but the way you've taken isn't the only one to achieve hard-private.

mbrowne commented 5 years ago

FWIW, I'm a TS user (and also a JS user) and I look forward to being able to create private fields in TS that are actually private at run-time. (I also would like it if TS had an option for properties declared using private to transpile to symbols, so there would be a convenient syntax for soft private as well.)

bigopon commented 5 years ago

I can't find the source, but I've been told that "actual privacy" is one of the most requested TS features (hopefully someone from the TS team can confirm/clarify)

I do want to see it.

TS users code is fully of fields behaving in [[Set]] semantic, and this proposal does it differently. And TS chose to follow JS closely, how would existing code be handled once this proposal is released?

bakkot commented 5 years ago

@ljharb

I can't find the source, but I've been told that "actual privacy" is one of the most requested TS features (hopefully someone from the TS team can confirm/clarify)

You may be thinking of this comment. Though it's also pretty obvious if you follow the TS bug tracker, since there's a new issue about this once every couple months.

bigopon commented 5 years ago

@bakkot Thanks for the link. Then the next part to prove the claim "majority target audience" is to make sure who want private field aware that what they ask is [[Set]] private field, and what they get is [[Define]] private field, and that is acceptable for them.

ljharb commented 5 years ago

Since the difference only matters if you have a setter (edit: or a Proxy) higher up the prototype chain, which is not a common pattern, it seems apparent that the vast majority of cases won't care which is chosen for public fields.

bakkot commented 5 years ago

@bigopon I don't think it really makes sense to talk about Set vs Define semantics for private fields? And for public fields, most of the time (i.e. unless there are accessors or proxies involved, or someone has frozen a prototype somewhere) they will have precisely the same semantics.

bigopon commented 5 years ago

@bakkot That side effect on public field is really the issue though. But I think I'll just be repeating the arguments that have been made and probably won't be able to put it better so I'll stop at this

The target audience this proposal benefits will give a burden to non target audience who have to maintain later. Also can we have a section in the readme stating clearly who would belong to target audience?

Igmat commented 5 years ago

@ljharb, @bakkot it seems that you underestimate how many libraries are going to be created (and already) with Proxy as main building blocks of their capabilities. And [[Define]] will affect not only library authors but library consumers too. Surely it's very small issue comparing to problem granted by incompatibility between privates and proxies.

rdking commented 5 years ago

@bakkot

This seems like our core point of disagreement. I expect the overwhelming majority of the target audience will be able to benefit from this proposal.

Given how entrenched TC39 is on its highly subjective and speculative opinions, I sincerely hope I'm wrong.