proposal-class-fields icon indicating copy to clipboard operation
proposal-class-fields copied to clipboard

Summary: Objections to fields (as opposed to alternatives), especially the private field syntax

Open mbrowne opened this issue 5 years ago • 136 comments

Continuing from #100 and other threads, here's a summary of key objections to this proposal raised by the community:

@hax's top 3 concerns:

  1. TC39 underated the risk of community break which is very harmful to all of us.
  2. TC39 current process failed on such controversial problems.
  3. Some TC39 members use a vicious circle and/or double standards to dismiss the issues/alternatives other raised/offered.

Also, he believes another long wait for private state is acceptable if that's what it takes to get it right: the problems with the current proposal—including with public fields—are just too severe.

Regarding the substance of the proposal:

  • Duality of private vs. public fields will cause confusion: similar syntax but very different semantics
  • # syntax does not fit into the mental model of JS devs and will cause a community split, and TypeScript issues will further fracture the community
  • We don't need public property/field declarations and if we must have them, they should follow existing property semantics instead of fields. Fields cause pernicious bugs when overriding properties in subclasses or refactoring base classes.
  • Classes 1.1 proposal offers a better alternative in many ways, including instance variable semantics

@rdking:

  • Similar concerns about the process as @hax. Difficult to understand the committee's requirements and the reasons for them.
  • "Undetectability" should not be a requirement since it falls short of a fully protective security feature
  • Strongly believes that the best solution will integrate with existing features instead of crippling or disabling them, e.g. inconsistency of obj['property'] for public fields but no equivalent for private fields breaks existing feature by breaking expectations. Lack of dynamic access of private fields (e.g. this['#x']) is a deal-breaker.
  • # is not "well-defined"—has two meanings. Context is insufficient to disambiguate declaration and access.
  • Advocates property semantics instead of field semantics (but somewhat differently from @hax; see #144)

@bdistin:

  • This proposal lacks long-term planning and will irreversibly reduce the design space for important future proposals such as keywords for other access modifiers
  • Agrees with @rdking that lack of dynamic private properties is a deal-breaker; violates core strength of the JS language (dynamicism)

@mbrowne (me):

(NB: it's mbrowne, not mbrown)

  • This is a solid proposal as-is, but agree with @bdistin's point about long-term planning
  • Strongly agree with @hax that declarations should use property semantics, specifically the part about using [[Set]] instead of [[CreateDataPropertyOrThrow]], for the reasons described in https://github.com/tc39/proposal-class-fields/issues/151#issuecomment-431006824 and #144
  • private #x syntax should be strongly considered because # alone might make future native support of intermediate access levels impossible. But # alone might be OK if we are confident we could have another way of natively sharing private fields within a module.

@Igmat:

  1. agree with @hax about somewhat failed process for such controversial topic
  2. brand-checking shouldn't be added implicitly to all private reads/writes, since it breaks metaprogramming with proxies as described in #106 and #134
  3. strongly agree with @hax about that [[Set]] should be used instead of [[CreateDataPropertyOrThrow]]
  4. agree with that current proposal should allow future native expansion in some way (there are a lot of ways to achieve that)
  5. Symbol.private + some kind of shorthand syntax for making symbols more ergonomic, provides much cleaner mechanism for encapsulation

@shannon:

  1. # being part of the name leads to confusion because this.#x !== this['#x'], it's just too weird. The sigil is fine but the functionality is just too different.
  2. No dynamic access, no destructuring is a deal breaker for me
  3. I agree with @Igmat that "brand-checking shouldn't be added implicitly to all private reads/writes"
  4. I agree with @Igmat that "proposal should allow future native expansion in some way (there are a lot of ways to achieve that)"
  5. I agree with @rdking that "the best solution will integrate with existing features instead of crippling or disabling them."
  6. I believe a private symbol approach (as described in syntax 1 here #149) is by far the simplest addition to existing JS paradigms/existing JS class spec, and achieves most of what the current proposal wants with a few minor exceptions (possibility to leak private symbols for one, but I think this could actual be useful for things like protected to be implemented in userland).

@aimingoo:

  • Concept "Fields" is unclear and its necessity as an independent concept is questionable. The concept undermines the "Object is collection properties" core concept, and the impact will be extremely far-reaching, but nobody taken seriously. The Similar sound includes: Not Fields in Classes 1.1 proposal.
  • Using the lexical environment to implement "private field" is to recreate var and ctx's VariableEnvironment, which is an abuse of Environment components, and it is obviously too bulky.
  • The syntax is very unfriendly, especially with # as a prefix character for declaring fields and for name operations (such as the semantics of this["#x"] vs. this.#x). About grammar, I recommend enabling the private keyword to declare private properties (Note: that I don't support new concepts like "Fields or Private Fields"). And # as an operator, this is reasonable in both this#data and #data as unary or binary operators.

Reference: #148 , #100 (comment)


@RyanCavanaugh:

  • The choice of [[Define]] over [[Set]] for initializers is a bad trade-off that throws away six years of evidence from TypeScript and Babel developers' use of fields. The "natural experiment" has been performed, the results are clear, and the runtime breakage incurred here is very difficult to detect with tooling. The proposed upside of [[Define]] (const fields later) is purely speculative; the downside of not being able to install a deprecation setter is concrete and immediate.
  • Making initializers on fields optional, especially combined with the above, is unintuitive and dangerous. This syntactic expansion isn't strictly necessary and represents a substantial breaking change problem for TypeScript/Flow/Babel with no clear upside.
  • Honestly, the negative response to the # sigil is something we should just ignore and take the hit for, if built-in hard privacy is really needed. People will adapt and the runtime need for it is extremely clear; all counterproposals have been ignorant of the design constraints or runtime semantics.

@jamesernator

my main objection to the current private fields proposal (and more so private methods) is that it throws away the notion of prototypal inheritance in favour of an object branding mechanism inconsistent with how other properties work.


In addition, some members of the TC39 committee also have current objections to this proposal. For example:

@allenwb:

See the Classes 1.1 motivation document. In particular if you negate the descriptions in the Design Rules bullet list you have a good summary of my concerns with the major problems being:

Design Rules: Item 2:

Be as small as possible

Item 3:

Support a user conceptual model that is as simple as possible. It should be easy to teach and easy to understand.

Item 4:

Be internally consistent, with few, if any, special cases or unexpected feature interactions.


@zenparsing:

I believe that we are overpricing the benefit of this entire feature set while underpricing the risk.

Why do I think that we're overpricing the benefit?

  • In my experience, most of our constituency does not need hard-private in their day-to-day programming work. Hard private is important for a small number of use cases, such as platform library code and high-profile libraries which are especially subject to breaking consumers when implementation details are updated. Most application programmers I know don't write that kind of code. Sorry 🤷‍♀️
  • Hard privacy can already be achieved with decent ergonomics. See https://github.com/zenparsing/hidden-state#example, for example.
  • Folks that really want "fields" are mostly happy using TypeScript already. It doesn't seem implausible to me that TS could have an option for generating hard-private code for private x, perhaps with some optimization work applied to WeakMap on the engine side.

Why do I think that we're underpricing the risk?

  • We know that many devs don't like the syntax, and don't understand why the syntax that they enjoy using (private x from TypeScript) won't work. It doesn't "just work". We are taking on the risk that this feature will continue to create head-scratching across our community.
  • We're taking on the risk that we'll want # for something else and it won't be available.
  • We're taking on a small amount of ASI risk.
  • We're taking on a risk that by pushing something unpopular our community will lose some amount of faith in TC39 and the future of JS.

I probably missed something important, and obviously I haven't included a summary for everyone who has commented. If you feel something is missing, please provide a concise summary of your feedback as your first comment on this thread (saving longer arguments for subsequent comments). Also feel free to simply say, "I agree with [username]".

mbrowne avatar Oct 17 '18 14:10 mbrowne

@igmat (me):

  1. agree with @hax about somewhat failed process for such controversial topic
  2. brand-checking shouldn't be added implicitly to all private reads/writes, since it breaks metaprogramming with proxies as described in #106 and #134
  3. strongly agree with @hax about that [[Set]] should be used instead of [[CreateDataPropertyOrThrow]]
  4. agree with that current proposal should allow future native expansion in some way (there are a lot of ways to achieve that)
  5. Symbol.private + some kind of shorthand syntax for making symbols more ergonomic, provides much cleaner mechanism for encapsulation

Igmat avatar Oct 17 '18 14:10 Igmat

After talks with @ljharb, I concede that "undetectability" is a reasonable desire. Whether that reaches the level of a requirement depends on whether or not achieving it causes some existing functionality to fail or work in unexpected ways.

I don't remember wanting the ability to dynamically add a private field. I feel that would be counter-productive and dangerous.

I've also been working with the advocates of the classes-1.1 proposal on issues of functionality and implementation. You can count me as being in favor of that proposal.

rdking avatar Oct 17 '18 15:10 rdking

Sorry @rdking, that must have been a bad recollection on my part. I removed that from the original post.

mbrowne avatar Oct 17 '18 15:10 mbrowne

I don't remember wanting the ability to dynamically add a private field.

Someone want it. But I can't find the comment, too many 🤣

hax avatar Oct 17 '18 15:10 hax

  • strongly agree with @hax that [[Set]] is more consistent with current behavior than [[CreateDataPropertyOrThrow]].
  • believes that public properties should be getter/setter accessors backed by private properties
  • believes that the need for other accessor types will become significantly more apparent after the release of a private property solution.
  • strongly believes that the best solution will integrate with existing features instead of crippling or disabling them.

rdking avatar Oct 17 '18 15:10 rdking

@hax I remember that too, from when I debuted the idea behind proposal-object-members. Someone thought that would enable the possibility for dynamically adding private fields (and indeed it could), but I said that wasn't part of the plan.

rdking avatar Oct 17 '18 15:10 rdking

@mbrowne responded to your clarification request here

bdistin avatar Oct 17 '18 16:10 bdistin

@mbrowne re: "dynamic private properties", I assume you're thinking of @shannon here.

bakkot avatar Oct 17 '18 16:10 bakkot

I don't mind so much that they can't be created dynamically after construction. I just think dynamic access is a staple of JS and not having it is a huge oversight. Right now private symbols seems the best approach to achieve this in my opinion.

shannon avatar Oct 17 '18 16:10 shannon

Summary of my feelings of this proposal

  1. # being part of the name leads to confusion because this.#x !== this['#x'], it's just too weird. The sigil is fine but the functionality is just too different.
  2. No dynamic access, no destructuring is a deal breaker for me
  3. I agree with @Igmat that "brand-checking shouldn't be added implicitly to all private reads/writes"
  4. I agree with @Igmat that "proposal should allow future native expansion in some way (there are a lot of ways to achieve that)"
  5. I agree with @rdking that "the best solution will integrate with existing features instead of crippling or disabling them."
  6. I believe a private symbol approach (as described in syntax 1 here #149) is by far the simplest addition to existing JS paradigms/existing JS class spec, and achieves most of what the current proposal wants with a few minor exceptions (possibility to leak private symbols for one, but I think this could actual be useful for things like protected to be implemented in userland).

shannon avatar Oct 17 '18 16:10 shannon

@mbrowne Could you please add @shannon and @Igmat's feedback to the issue description? Just to avoid loosing them in thousand of comments like in the other issues.

nicolo-ribaudo avatar Oct 17 '18 19:10 nicolo-ribaudo

@nicolo-ribaudo Sure - updated. I also added @allenwb's feedback.

@rdking I am trying not to add too many bullet points of closely related items or second tier concerns, but this one that you added seems particularly significant:

strongly believes that the best solution will integrate with existing features instead of crippling or disabling them.

Can you expand on that slightly, while still keeping it a concise summary? I'd like to add it to the issue description but it doesn't say what you're specifically referring to.

Or if you prefer that I completely replace your summary, you can post something for me to copy and paste.

mbrowne avatar Oct 17 '18 20:10 mbrowne

CC @zenparsing @BrendanEich

mbrowne avatar Oct 17 '18 20:10 mbrowne

@mbrowne The things I listed were in addition to what you've already got.

strongly believes that the best solution will integrate with existing features instead of crippling or disabling them.

Where this is concerned, I mean that, using the concept of a "private property", since this is just another property of an object, it is reasonable for the uninitialized to assume that access capability will be no different for public than it is for private, especially since they both take the same form. So with the existing feature being

  • I can access a property with either obj.<property> or obj['property'] the existence of a private property should not break this expectation.

This is even more true in the case of Proxy where the completely undetectable private properties are not being stored in a separate object that uses the identity of the owner as a lookup reference, but somehow, Proxy still throws when dealing with a method accessing private fields. While the reason it throws is directly analogous to WeakMaps (the Proxy object itself is passed as the "target" to the handler functions), there's no reason for this to be the case, especially considering that developers implementing private fields in their libraries will be flooded with feedback from their users about use cases where whatever they used a Proxy for is now breaking merely due to the presence of the private fields.

Put shortly: If the proper introduction of a new feature into existing, working code bases that don't rely on the improper use of properties not intended for external use, causes the code to break, then the new feature has broken an existing feature.

rdking avatar Oct 17 '18 21:10 rdking

Since you asked...

I believe that we are overpricing the benefit of this entire feature set while underpricing the risk.

Why do I think that we're overpricing the benefit?

  • In my experience, most of our constituency does not need hard-private in their day-to-day programming work. Hard private is important for a small number of use cases, such as platform library code and high-profile libraries which are especially subject to breaking consumers when implementation details are updated. Most application programmers I know don't write that kind of code. Sorry 🤷‍♀️
  • Hard privacy can already be achieved with decent ergonomics. See https://github.com/zenparsing/hidden-state#example, for example.
  • Folks that really want "fields" are mostly happy using TypeScript already. It doesn't seem implausible to me that TS could have an option for generating hard-private code for private x, perhaps with some optimization work applied to WeakMap on the engine side.

Why do I think that we're underpricing the risk?

  • We know that many devs don't like the syntax, and don't understand why the syntax that they enjoy using (private x from TypeScript) won't work. It doesn't "just work". We are taking on the risk that this feature will continue to create head-scratching across our community.
  • We're taking on the risk that we'll want # for something else and it won't be available.
  • We're taking on a small amount of ASI risk.
  • We're taking on a risk that by pushing something unpopular our community will lose some amount of faith in TC39 and the future of JS.

Thanks for asking me to share my thoughts (I'm not particularly interested in debating though).

zenparsing avatar Oct 17 '18 21:10 zenparsing

@mbrowne Thanks for the summary! This helps a lot.

littledan avatar Oct 17 '18 21:10 littledan

Summary of my comment/issues:

  • Concept "Fields" is unclear and its necessity as an independent concept is questionable. The concept undermines the "Object is collection of properties" core concept, and the impact will be extremely far-reaching, but nobody taken seriously. The similar sound includes: Not Fields in Classes 1.1 proposal.

  • Using the lexical environment to implement "private field" is to recreate var and ctx's VariableEnvironment, which is an abuse of Environment components, and it is obviously too bulky.

  • The syntax is very unfriendly, especially with # as a prefix character for declaring fields and for name operations (such as the semantics of this["#x"] vs. this.#x). About grammar, I recommend enabling the private keyword to declare private properties (Note: that I don't support new concepts like "Fields or Private Fields"). And # as an operator, this is reasonable in both this#data and #data as unary or binary operators.

Reference: https://github.com/tc39/proposal-class-fields/issues/148 , https://github.com/tc39/proposal-class-fields/issues/100#issuecomment-429533532

aimingoo avatar Oct 17 '18 21:10 aimingoo

Updated. @zenparsing and @BrendanEich, I mentioned you not to encourage you to join the debate (unless you want to), but just to give you an opportunity to have your feedback included in the issue description. Thank you everyone for summarizing your feedback.

mbrowne avatar Oct 17 '18 22:10 mbrowne

I will summarize my own feedback as well:

  • The choice of [[Define]] over [[Set]] for initializers is a bad trade-off that throws away six years of evidence from TypeScript and Babel developers' use of fields. The "natural experiment" has been performed, the results are clear, and the runtime breakage incurred here is very difficult to detect with tooling. The proposed upside of [[Define]] (const fields later) is purely speculative; the downside of not being able to install a deprecation setter is concrete and immediate.
  • Making initializers on fields optional, especially combined with the above, is unintuitive and dangerous. This syntactic expansion isn't strictly necessary and represents a substantial breaking change problem for TypeScript/Flow/Babel with no clear upside.
  • Honestly, the negative response to the # sigil is something we should just ignore and take the hit for, if built-in hard privacy is really needed. People will adapt and the runtime need for it is extremely clear; all counterproposals have been ignorant of the design constraints or runtime semantics.

RyanCavanaugh avatar Oct 18 '18 17:10 RyanCavanaugh

@RyanCavanaugh

Making initializers on fields optional, especially combined with the above, is unintuitive and dangerous. This syntactic expansion isn't strictly necessary and represents a substantial breaking change problem for TypeScript/Flow/Babel with no clear upside.

How is it a breaking change or problem for Babel? Or do you just mean when using Babel and Flow together? I think undefined is a good default and don't see the advantage of explicitly initializing propertyName = undefined considering that this would be very common.

mbrowne avatar Oct 18 '18 18:10 mbrowne

@mbrowne Link

class Base {
  prop = 10;
}

class Derived extends Base {
  prop;
}

// Prints 10 in Babel 6.26, would print 'undefined'
console.log(new Derived().prop);

RyanCavanaugh avatar Oct 18 '18 18:10 RyanCavanaugh

This would be so much simpler and avoid many of the issues raised if redeclaring the same property in a subclass were simply illegal...you can already set different initial values if you want (or even call Object.defineProperty) in the constructor.

mbrowne avatar Oct 18 '18 18:10 mbrowne

@mbrowne Since properties are either data properties or accessor properties, it might be better if redeclaration of a data property that doesn't changing the property type from data to accessor is illegal. Likewise, changing from accessor to data should also be illegal. Redefining accessor properties as new accessor properties should be allowed as this might represent some useful change to the accessors.

rdking avatar Oct 18 '18 18:10 rdking

@RyanCavanaugh

The proposed upside of [[Define]] (const fields later) is purely speculative;

...and irrelevant since even with [[Set]], the action can be immediately followed by a change to the writability of the property, leaving all side effects of setting the value in tact.

rdking avatar Oct 18 '18 18:10 rdking

If possible, can I suggest we keep discussion of specific points to other threads? (Anything beyond clarifications, anyway.) I'd like to keep this thread as an accessible summary.

bakkot avatar Oct 18 '18 18:10 bakkot

@RyanCavanaugh

Honestly, the negative response to the # sigil is something we should just ignore and take the hit for, if built-in hard privacy is really needed. People will adapt and the runtime need for it is extremely clear; all counterproposals have been ignorant of the design constraints or runtime semantics.

  1. Kinda difficult to be aware of design constraints that are never revealed.
  2. The runtime semantics of the current proposal is crippling enough to break a sizable portion of the code in the wild should library authors decide to rewrite making use of this proposal's version of private fields.
  3. While the counterproposals do take into account different constraints, the general goal of adding private in such a way that if the only changes made are to make private those properties meant to be implementation details, then no code that did not make ill-advised use of such fields will be broken, is the one thing they all have in common, while differing with the current proposal.
  4. ES does not need built-in hard privacy. It's just an extremely strong want. Hard privacy is already well provided for via judicious manipulation of closures and WeakMap. I would much rather see the TC39 take this proposal back to stage 1 or 2 so it can be re-formulated than see it implemented, and watch as people are frustrated by unnecessary code-bloat (due to lack of [] support for private fields), broken libraries (due to incompatibility between Proxy and private fields), disabled functionality (due to field definition being a [[Define]] instead of a [[Set]] causing prototype properties to be ignored unexpectedly), and other such issues.

rdking avatar Oct 18 '18 19:10 rdking

@RyanCavanaugh

  • Honestly, the negative response to the # sigil is something we should just ignore and take the hit for, if built-in hard privacy is really needed. People will adapt and the runtime need for it is extremely clear; all counterproposals have been ignorant of the design constraints or runtime semantics.

Even hard privacy is must, both classes 1.1 and Symbol.private proposal meet this requirement. Basically instance variable in classes 1.1 keep the same semantic of current private field, but use different syntax which avoid controversial #. The other big difference between classes 1.1 with current proposal is classes 1.1 doesn't include public field, but it seems you agree current semantic of public field has many problems.

hax avatar Oct 19 '18 19:10 hax

Please, let's keep this thread topical and avoid back and forth technical discussion. The purpose was to summarize points of view to be accessible to people who don't have time to read very long threads.

littledan avatar Oct 19 '18 20:10 littledan

@littledan could you please clarify how this summary will affect proposal and / or process?

Igmat avatar Oct 19 '18 21:10 Igmat

If there's no legible summary, the whole discourse in the repository is likely to be ignored by the vast majority of observers, who have no time to read through all the back and forth.

I can't say that the summary will cause a change in the outcome because, as I've explained, TC39 has repeatedly reaffirmed consensus on the proposal in this repository, and implementations are moving ahead with it.

littledan avatar Oct 19 '18 23:10 littledan