Skip to content
This repository has been archived by the owner on Jan 25, 2022. It is now read-only.

A summary of feedback regarding the # sigil prefix #100

Closed
glen-84 opened this issue May 7, 2018 · 391 comments
Closed

A summary of feedback regarding the # sigil prefix #100

glen-84 opened this issue May 7, 2018 · 391 comments

Comments

@glen-84
Copy link

glen-84 commented May 7, 2018

Issue/comment 👍 👎 🎉 ❤️
Stop this proposal 196 31 20 45
Why not use the "private" keyword, like Java or C#? 132 7 12 5
Why not use the "private" keyword, like Java or C#? 💬 144 0 16 9
Why not use the "private" keyword, like Java or C#? 💬 51 0 0 0
Private Properties Syntax 33 4 0 0
Yet another approach to a more JS-like syntax 32 0 0 41
New, more JS-like syntax 32 4 0 0
Please do not use "#" 32 0 1 1
Proposal: keyword to replace `#` sigil 18 0 0 1
Why not use private keyword instead of #? 16 0 0 1
Explore a more JS-like syntax 7 0 0 0
The '#' looks awful, please change it 9 4 0 0
can we just use private keyword? 2 0 0 0
Do we still have a chance to stop private fields (with #)? 3 3 1 1
[Private] yet another alternative to `#`-based syntax 3 0 0 0
and yet another suggestion for "private but not closure" 0 0 0 0
Why don't use `private` instead of `#`? 0 3 0 0
Why "private" and "protected" future reserved keywords are not used? 0 0 0 0

Other feedback:

  • http://2ality.com/2017/07/class-fields.html
    • From the comments: "I'd rather shoot myself in the face than use hashtags for private fields."
      • 33 upvotes (comments closed)
  • ES Next Features That'll Make You Dance
    • "And beware, you may have an adverse reaction ..."
    • "Oh, I heard somebody groan!"
    • Some comments:
      • #method() is gross, at least I use typescript and don't have to look at it. 👍30

      • So ES is becoming the new PHP "ugly-wise"? So glad i use typescript. They should have delayed that private method feature some more and thought some more to really avoid having that absolutely terrible, horrible looking # syntax. Ew.. In code everything is possible. EVERYTHING (if your not into computer science). If they have to break backward compat then so be it. So very bad.

      • I am excited about getting new features and I like to see things continuing to grow, I am not liking that private property syntax in the function, it looks like a comment and makes me shudder... everyone should be aware of this "fact" - every time someone gets confused and thinks the # symbol is actually a comment when JavaScript should have used the word private to declare a private entity, a bag of kittens gets thrown into a wood chipper, very sad.

      • Fix the private keyword issue please! The proposed syntax is not only inconsistent with the static declaration but also looks like a comment!

The TypeScript team are not fond of the syntax either.

My suggestion to rename the proposal: tc39/proposal-private-fields#72
(the idea is mainly about making it a lower-level feature, and making room for a cleaner syntax at a later date)

@littledan @bakkot @ljharb – I apologize in advance, as I know that you must be thinking "Oh g*d, not again!", but I just wanted to put this all in one place, and allow the community to share their opinion in the form of reactions. It would be great if you could keep this open for a while (30 days?), or perhaps even indefinitely to prevent other users from submitting even more issues on the same topic.

To the non-committee members reading this: These proposals are now at stage 3, so it's unlikely that a syntax change will occur, but please feel free to up-vote this issue if you feel that the sigil (#) prefix is not a good fit for the language, or down-vote the issue if you believe that it's perfectly acceptable.

@bakkot
Copy link
Contributor

bakkot commented May 7, 2018

Um. Thanks for the effort, but... I don't really get what you intend this issue to accomplish. We've seen this feedback already.

@glen-84
Copy link
Author

glen-84 commented May 7, 2018

I don't really get what you intend this issue to accomplish

  1. Emphasize and centralize the large amount of negative feedback towards the sigil prefix
  2. Prevent users from submitting the same topic over and over again

We've seen this feedback already

Repeatedly. And how many unique threads does the commitee need to see before they consider it a real issue, as that clearly hasn't happened yet.

@bakkot
Copy link
Contributor

bakkot commented May 7, 2018

Prevent users from submitting the same topic over and over again

I was kinda hoping the FAQ would accomplish that, but no such luck so far. Still, might be worth a shot.

And how many unique threads does the commitee need to see before they consider it a real issue

We do and always have considered it a real issue. But as always it's one to be balanced against other issues, which we've done, and we concluded that this was the right path. There's no number of threads which will cause the constraints to change.

@stevenvachon
Copy link

The day that ECMAScript/JavaScript died.

@ljharb
Copy link
Member

ljharb commented May 7, 2018

@glen-84 That isn't unique threads nor unique counts; how many of those 99 or 111 etc are the same people? If you actually uniquify it to "how many unique accounts have indicated dislike of #", the numbers will be less impressive.

Either way, "popularity" is not nearly as relevant as the issues listed in the FAQ. There is no tenable syntax alternative, and no sigil that's any "better" in terms of the complaints you're describing.

@thysultan
Copy link

This documents a few others not listed here.

@ljharb There are tenable alternatives and community backlash is not the only motivating factor as there are concrete issues with the dot#sigil syntax that have mostly been ignored – #28 and #56.

@ljharb
Copy link
Member

ljharb commented May 7, 2018

@thysultan I wouldn't say ignored; more, considered and disagreed with.

@glen-84
Copy link
Author

glen-84 commented May 8, 2018

That isn't unique threads nor unique counts; how many of those 99 or 111 etc are the same people? If you actually uniquify it to "how many unique accounts have indicated dislike of #", the numbers will be less impressive.

I never said that they were unique counts. Even if the numbers aren't very high, the relative numbers are significant IMO.

Either way, "popularity" is not nearly as relevant as the issues listed in the FAQ. There is no tenable syntax alternative, and no sigil that's any "better" in terms of the complaints you're describing.

There is no tenable syntax alternative currently. That may change in 5-10 years. All I'm suggesting is to make room for a cleaner syntax in the future, by making this a lower-level (read: even uglier) feature and letting users transpile to that from better-looking code (code that actually looks like JavaScript, and not ASCII art).

@tasogare3710
Copy link

@stevenvachon

                   __________\
                  /          \\\
                 /    REST    \\\
                /      IN      \\\
               /     PEACE      \\\
              /                  \\\
              |       es         |\
              |                  |\
              |   killed by a    |\
              |  private-fields  |\
              |       2018       |\
             *|     *  *  *      | *\
     ________)/\\\\_//(\\/(/\\)/\\//\\/|_)_______

@tasogare3710
Copy link

People are not merely dislike sigil, they point out a many real issues. someone does not understand it well. However, this proposal is nothing that can be done as it has reached stage 3.

Everything happens for a reason.

@bakkot
Copy link
Contributor

bakkot commented May 8, 2018

There is no tenable syntax alternative currently. That may change in 5-10 years.

There are fairly strong reasons to believe that will not be the case. Few of the considerations in the FAQ are likely to change.

@littledan
Copy link
Member

Even before this compilation, it was clear to me that discomfort with # is widespread, and this discomfort has been visible to the committee for a couple years. However, I agree with @bakkot that it's unlikely that we will find a better syntax in the future.

Whenever I have discussed this issue with someone in person, explaining the argument in the FAQ, they have ultimately been accepting of the # syntax. It seems like many of the people who agree with the motivation for adding private fields and methods are accepting of the ergonomic cost of this syntax. It also seems like most of the concern comes from it not being exactly this.x--I don't have the feeling that any other syntax will be as popular.

@glen-84
Copy link
Author

glen-84 commented May 13, 2018

There are fairly strong reasons to believe that will not be the case. Few of the considerations in the FAQ are likely to change.

That's unfortunate. Long live TypeScript (and WebAssembly + C#?).

@ghost
Copy link

ghost commented May 14, 2018

It is pretty clear that the JS community looks at this proposal with disdain, so I must ask: who is this proposal for? The community clearly dislikes the syntax. Enterprise? TypeScript team clearly dislikes the syntax and they are really big in the enterprise.

Why do you even allow the community to contact the team, when you just disrespectfully dismiss everything, even valid points.

If you don't care about the community's voice a single bit then just straight up state the facts instead of sugar coating things and dancing around the issue. There are a ton of more appealing suggestions regarding the syntax and instead of talking about it with the community you just shut down the discussions.

This is ridiculous.

@jeffmo
Copy link
Member

jeffmo commented May 14, 2018

@ayylmar: I’m not participating in the committee much anymore, but having worked on public fields im still following all of the fields discussions.

I think there might be some context that you may have missed. I think it’s clear that several people in the community do dislike the syntax, but it’s not clear what a viable alternative is. If you put yourself in the position of those championing private fields they’re basically left with “keep it and annoy some people with syntax, or toss it altogether and annoy some people with a lack of private fields”.

It’s certainly unfortunate that many people dislike the syntax, but without a viable alternative it doesn’t seem like we’re left with anything but those 2 choices. Alternatives that many have proposed in the past tend not to make it very far down the path of exploration before an objectively blocking concern arises with them.

I get it though, on its surface this seems like a simple solution: Just fix the syntax. It also kinda sucks that there’s been so many issues about this that it’s a little tricky to find the exploration of all of the alternatives that ultimately just couldn’t work.

@doug-numetric
Copy link

I'm late to the discussion, but why can't we do something like this?

class Point {
  // instance private, just an intuitive closure
  let instance;
  constructor(_x, _y) {
    // class private, a private version of this to assign you class private stuff to
    private this = { x: _x, y: _y };
    instance = uuid();
  }

  equals(p) {
    { x: my_x, y: my_y } = private this;
    // this works only if p is an instance of Point
    { x: p_x, y: p_y } = private p;
    return p_x === x && p_y === y;
  }
}

javascript is inherently prototypal, not classical. The classical stuff should be 2nd class, not the other way around. Besides, this looks clean and intuitive.

@ljharb
Copy link
Member

ljharb commented May 19, 2018

@doug-numetric private isn't "classical", it's bringing closure-based privacy to instances. For your example - destructuring syntax allows computed property names, including symbols - and private this and private p would have to not be objects you can pass around or reflect over, but still looks like objects because you're destructuring out of them. That's a lot of complexity to add to the grammar, and more importantly, to the user's mental model.

@doug-numetric
Copy link

doug-numetric commented May 19, 2018

@ljharb Thanks for the response. I see the need for private this to not be an object. I also see the concern with this.#foo !== this['#foo'] which I think is unintuitive. I for one would opt for verbosity over using the # sigil in any case. With not adding complexity to the mental model as a goal, why pretend to add class privates to the this object when it's not really there. Javascript has private reserved already. If it's not used for this feature, when? Other than verbosity, what's wrong with:

class Point {
  // class private
  private z = 0;

  // instance private, just an intuitive closure
  let instance;
  constructor(_x, _y, _formatter) {
    // class private
    private this x = _x;
    private this y = _y;
    private this formatter = _formatter;
    instance = uuid();
  }

  equals(p) {
    // this works only if p is an instance of Point
    return private p x === private this x && private p y === private this y;
  }
  print() {
    return (private this formatter)(private this x, private this y, private this z);
  }
}

const p1 = new Point(2, 4, (x, y, z) => `<${x}, ${y}, ${z}>`);
p1.print(); // <2, 4, 0>

const p2 = new Point(2, 4, function(x, y) { this.x = 0; return `<${x}, ${y}>; });
// I suspect this should throw a TypeError "Cannot set property 'x' of undefined"
p2.print();

The destructuing shorthand is just really handy sugar, which I like as this can get verbose.

@bakkot
Copy link
Contributor

bakkot commented May 19, 2018

Other than verbosity, what's wrong with

See the FAQ.

// instance private, just an intuitive closure

Separately, no, that's not. In JavaScript as it stands, a given function doesn't see different values for a closed-over variable depending on how the function is invoked. (Keep in mind that class methods are shared; there's not a separate copy created for each instance.) As such the only consistent semantics for this syntax would be for it to have the same value for all instances of the class, just as if the declaration had occurred outside the class. Which I think is not what you want.

@jonmersan
Copy link

I've finally been motivated to sign up to github. I've been reading issues and such around several TC39 proposals because I'm in the process of helping my company figure out how we want to approach es going forward (a former employee enabled stage 0 plugins in Babel and no real discussion of risks ever happened).

See the FAQ.

Even with that, I disagree with the claim that # is significantly better than declaring a private field as private and then accessing it like any other field (just as in Java and other languages). From what I understand, it looks like you are avoiding implementation of privates using WeakMaps? Used properly, I have yet to see a case that breaks. As for the example in the FAQ, it has a flaw that's easily corrected:

let Person = (function(){
  const privates = new WeakMap;
  let ids = 0;
  
  function getPrivate(obj, name) {
    if(privates.has(obj)) {
      return privates.get(obj)[name];
    }

    //fall back to the public value, if it has one
    return obj[name];
  }

  return class Person {
    constructor(name, makeGreeting) {
      this.name = name;
      privates.set(this, {id: ids++, makeGreeting});
    }
    equals(otherPerson) {
      return privates.get(this).id === privates.get(otherPerson).id;
    }
    greet(otherPerson) {
      return getPrivate(this, 'makeGreeting')(otherPerson.name);
    }
  }
})();
let alice = new Person('Alice', name => `Hello, ${name}!`);
let bob = new Person('Bob', name => `Hi, ${name}.`);
alice.equals(bob); // false
alice.greet(bob); // === 'Hello, Bob!'
let mallory = new Person('Mallory', function(name) {this.id = 0; return `o/ ${name}`;});
mallory.greet(bob); // === 'o/ Bob'
mallory.equals(alice); // false. As expected!

I didn't bother refactoring the entire sample code to use getPrivate vs. directly accessing members of the privates object in every instance, but it's enough to demonstrate that the reasoning in the FAQ isn't entirely convincing. The important part is that you should never leak the references of any objects stored in the privates WeakMap.

Of course, I know you can't do getPrivate(this, 'foo') = 'bar', but you could just make a generic function to handle this case:

fieldAccess(scope, fieldName, setValue) {
  if(privates.has(scope)) {
    if(arguments.length === 3) privates.get(scope)[fieldName] = setValue;
    privates.get(scope)[fieldName];
  } else {
    if(arguments.length === 3) scope[fieldName] = setValue;
    return scope[fieldName];
  }
}

Basically, assignments to a private field go from this.id = 0 to fieldAccess(this, 'id', 0) while all other instances of this.id become fieldAccess(this, 'id'). If you don't want to pay the cost of calling fieldAccess for every field of every object within code declared within the class, you could make the substitution only wherever the member of any object you're accessing matches the names of any private members. For things like this['id'], you could run it through fieldAccess but I think it'd be better to leave it unchanged as it would simply fall back to a public id which would give developers a way to access a public member from within a class declaring a private member by the same name. For for...of (as I've seen it mentioned in some thread or another here) I don't think I've seen much of anyone giving a reason that it should need to iterate over private members.

As far as I can tell, the primary motivating factor behind #uglymess is performance considerations. I think it'd be a small price to pay to bring this in to the language in a way the people will be happy to write code with. After all, if coding was about performance above all else, why did anyone ever adopt C++ or C# over C or even Java to begin with?

Personally, I'd probably look into something like TypeScript before actually using # in any real code to declare/access private fields.

Primarily I ended up here because I had to just now refactor some code involving the use of static and went to look up where that proposal was at only to find it had merged with this. It almost seems like if this proposal goes to stage 4, it's because static carried it there.

@ljharb
Copy link
Member

ljharb commented May 24, 2018

Instances (public and private) have been stage 3 for awhile; statics only reached stage 3 this week.

Separately, typescript doesn’t have truly private fields - nothing ensures their privacy at runtime.

@jonmersan
Copy link

From what I've read, it looks like private/public, static, and decorators are kind of merging together due to the fact that decorators apparently need knowledge of private members.

Considering the demand for decorators, I wouldn't be surprised if the motivations for implementations that are spec compliant with private only happen because the contributors really wanted decorators but had to deal with this version of privates because they had to in order to be fully compliant.

I don't know much about typescript, but I wonder if the lack of true privacy has to do with the fact that WeakMap itself is fairly new and not everyone can work only with modern browsers.

@pumano
Copy link

pumano commented May 25, 2018

I'm really glad to see how @glen-84 grab issues together (I'm author who create one of that).

In my humble opinion, it's developer experience.

Community want to use private. That keyword is reserved. Why not to use it?

Committee should understand, that, you do it in your "own" way. It's truly egoistic way.
Think about other people. If many people don't want it, why you not listen?

All enterprise languages like Java, C#, TypeScript, also PHP use private.

It's a big problem to implement proper public / private access to field without using #? Come on!

You are talented engineers! You add many amazing features to JavaScript, and you can do it properly too (under the hood) without losing semantic. Some of people in issues provide possible examples for using private.

@fuchsia
Copy link

fuchsia commented May 25, 2018

General feedback:

I gave slots a serious test on Monday (Chrome 67). I found they caught dumb typos and it was great to have a list of (private) fields rather than having to squint at the constructor code; I even warmed to the # — it's a big warning these are different to and and can shadow conventional properties.

But, I've got into the habit of destructing the properties I need at the start of a method:

class {
   pos( dec )
   {
      const { latitude, HA } = this;

      const diff = cos( dec - latitude ),
            sum  =  cos( dec + latitude ),
            cosHA = cos( HA );

      /* etc... */
   }
};

It's ergonomic (one this plus a little boiler plate for every access), visually compact, and shows all the dependencies at a glance.

Unless, I've missed something, slots break that. Doing it manually is cumbersome and uses a heap of screen estate so I was slipping back into this.#x for single accesses. And as a class developed, variables were shifting between slots, properties, arguments and local variables so there was a lot of inserting and deleting of this.#. I am seriously thinking about a single this.#state slot and storing all my variables in it — after all, that's what you'd do with a WeakMap.

So my feedback is that slots have rolled back usability and manageability of the code. But that can be fixed with follow-on proposals: allowing access to slots as just #whatever without the this. prefix might be fine, or maybe it should be possible to destructure slots into local names (const {#latitude} = this; creates a variable called latitude — I can rename conflicts) .

Other than that, I found them far less horrible than they looked, and have been itching to use them in production code.

@bakkot
Copy link
Contributor

bakkot commented May 25, 2018

@fuchsia, thanks for the feedback. I think we likely will want to allow writing const { a: b, #y: z } = this; to mean const b = this.a; const z = this.#y;, at the very least - would that help? (Other things, including shorthand access, are on the table; this just seems the most likely, and unlikely to get in the way of other potential followups.)

@pumano, please see the FAQ. It turns out that, yes, this is the best solution for JavaScript, in the opinion of the committee. Our constraints are unusual, especially the lack of a type system. The other solutions people have suggested have all seemed unacceptably bad in one way or another.

@jridgewell
Copy link
Member

The renamed destructure seems like a great follow on. To avoid variable name confusion, it’ll probably be necessary to force the rename { #x: x } until we’re used to the pattern.

@rdking
Copy link

rdking commented May 31, 2018

@ljharb
At any point over the past few years of mulling this proposal over did the TC39 board ever consider that the reason they're receiving such viscerally negative reactions from commenters might be due to the nature of the requirements that went into this proposal? I'm curious as to what would happen if all of the non-logical ("I feel that...") made by the board and contributors were given as little consideration as those made by those of us who have levied equally valid opinions. While it cannot be said that there is anything logically wrong with this proposal, solutions which do less damage to the language's conventions have been offered and dismissed due only to such opinions.

@bakkot
I've been trying to figure this one out. class as it exists in ES today is just syntactic sugar. There is literally nothing that it does that cannot be done without the class keyword. Even inheriting from a native type like Array or HTMLElement can be done properly in ES5 with exactly the same results as what you'd achieve from class. However, with private fields, there will be no exactly equivalent means of creating such privacy. The closest you might come would be to implement the private fields using a Proxy object, but even that comes with 2 issues

  1. no equivalent access syntax
  2. no equivalent result object (since the outer object will always be a Proxy)

To both of you: Is having this implementation of private fields really worth promoting class from syntactic sugar status to its own unique entity when other solutions have been offered that do not break this well-known and heavily depended-upon symmetry simply because the opinions of a few members of the TC39 board outweigh the many dissenting opinions and logical issues that have been offered? Consider that even the original author of this proposal once tried to kill it.

@bakkot
Copy link
Contributor

bakkot commented May 31, 2018

However, with private fields, there will be no exactly equivalent means of creating such privacy.

This is not true. There is a fairly straightforward and precisely equivalent desugaring using WeakMaps, though it requires some non-obvious care to get calls right. See the babel transform for an implementation.

@ljharb
Copy link
Member

ljharb commented Jun 1, 2018

Class can definitely do things that can’t be done otherwise; extending builtins, in particular.

@rdking
Copy link

rdking commented Nov 19, 2018

Simply put, I'm arguing that the contents of the shipping container are more important than the container. You, seeing the container as more important feel it's perfectly fine to modify the contents.

@hax
Copy link
Member

hax commented Nov 20, 2018

@ljharb I don't care about how you read the spec, I just want TC39 guys tell me and the whole community, why all these issues are unimportant:

like class { name = 'hax'; greeting = 'hello ' + name } (error-prone syntax), like class { [x] = [y] } // destructuring?, like [[Set]] vs [[Define]], like complexity of initializer order, like proxy transparency issue, like hard private breaks framework (white box test, or reactive framework like Vue), like inherit method could access base class private, but inherit static method could not access base class static private (surprise!), and like missing protected which is obviously a very important requirements for OOP... Too many to elaborate.

@mbrowne
Copy link

mbrowne commented Nov 20, 2018

@hax Realizing that your question was for @ljharb, I still want to say that although I have made my position clear, I can see both sides of this debate. Can you? Opponents of this proposal are naturally very focused on its downsides, but classes 1.1 has a lot of downsides too, including some that are worse IMO than any downside of this proposal.

Since this thread is about syntax, I'll say that IMO the classes 1.1 syntax seems illogical and inconsistent compared with this proposal. With this proposal, you only have to learn the meaning of #; the rest is intuitive aside from some edge cases. With classes 1.1, you have to learn that var (or let/const, as more recently proposed) don't actually create a regular variable (not even a static class variable) but somehow create an instance-level variable. And BTW, the presence of a var/let/const keyword is insufficient to make class declarations perfectly consistent and clear as you and @rdking are aiming for when you advocate for everything else to go on the prototype. We have debated both here and in the class-members repo why shorthand syntax for access is potentially problematic, and in any case sooner or later people would need to know about this->x or this::x syntax which at first glance seems completely unrelated to a var/let/const declaration.

OTOH, I can see how classes 1.1—especially if public instance property declarations were added to it—could actually be a viable alternative to this proposal, and I can see why some people prefer its syntax... After the initial learning curve, the syntax does make sense and doesn't lead to false expectations like thinking you could access a private name dynamically (e.g. const myPropertyName = 'foo'; this[/* how to get myPropertyName? */]).

I'm sure there are others, especially including committee members, who could make plenty of other strong arguments about why this proposal is overall better than classes 1.1. I don't think anyone is saying that it's perfect or that its downsides are unimportant, just that on balance it's still better than the alternatives.

@rdking
Copy link

rdking commented Nov 21, 2018

@mbrowne

With classes 1.1, you have to learn that var (or let/const, as more recently proposed) don't actually create a regular variable (not even a static class variable) but somehow create an instance-level variable.

Wrong model. These are regular variables in every sense of the term. The only thing that's changed is that the lexical scope of the class declaration now provides a function-like container to hold them. In other words, "Where variable declarations are concerned, class {} is no different than function() {}, and the closure created by function() {} is just the instance created by new." Is that more complicated than the # explanation? Sure! But the benefits in terms of language flexibility and expressiveness certainly outweigh a slight increase in difficulty to explain.

And BTW, the presence of a var/let/const keyword is insufficient to make class declarations perfectly consistent and clear as you and @rdking are aiming for when you advocate for everything else to go on the prototype.

Please explain. Everyone already understands that let/const is only valid between the nearest wrapping {}. So where's the lack of clarity?

in any case sooner or later people would need to know about this->x or this::x syntax which at first glance seems completely unrelated to a var/let/const declaration.

It's a new operator. Of course it would need explaining. But it's worth the gain in expressiveness. Also I thought of a solution to the shorthand syntax problem that would put both instance properties and instance variables on the same playing field by omitting the need for this(::/.). That would make it feel more similar to the shorthand syntax allowed by class-based languages. I'm still not sure I want that though.

I want it to be clear that while class-members is a fork of classes-1.1, it diverges from it in significant ways. The fact that it is still open for modification means that any such concerns TC39 has for its design and/or limitations can still be addressed. It's main benefits are that:

  • It's more syntacticly palatable.
  • It's mental model doesn't set up false expectations (as you pointed out)
  • It doesn't require the awkward trade-offs required by private-fields
  • It makes room for more expressive class-related syntax in the future

Beyond this, it has the same basic capabilities as the existing proposal. Despite my desire to avoid it, even public-fields can be added to the proposal without difficulty.

@mbrowne
Copy link

mbrowne commented Nov 21, 2018

@rdking

Please explain

You have repeatedly insisted that top-level, non-static declarations inside a class should only define things that go on the prototype. And yet you make an exception for the instance variable declarations in classes 1.1 and class-members (var/let/const). I realize that instance variables are in no way properties, but at least regarding the syntax your argument is inconsistent. In this (class-fields) proposal, myField = ... looks different from myMethod() {...}; in yours, let/const declarations look different from methods. Different behavior for different syntax is entirely reasonable and desirable and your proposal takes advantage of this, and yet you insist that myField = ... looks like something that should go on the prototype (and BTW, most developers have the opposite expectation). While still subjective, your argument that the prototype is where properties belong is at least a consistent argument, but your argument regarding the syntax doesn't follow.

@hax
Copy link
Member

hax commented Nov 21, 2018

@mbrowne I agree there are still problems in classes 1.1 or @rdking 's fork. As I said before, how could you expect a proposal only have several months (or days for @rdking 's case) to discuss can just be perfect? Actually I think classes 1.1, @rdking 's fork and private symbol proposal are all better than this proposal even in current state using the same logic of "balance". And consider current proposal has been discussed several years and still so fuzzy, I also believe there is no way to fix current proposal, but alternatives still have many possibility to get better. For example, a criticism to private symbol is it miss the brand check. Actually it could be easily added if we have time and space to investigate.

Of coz many could just disagree. Many time, I feel they disagree arbitrary. Anyway, the problem is how could we decide which opinion is much "correct balance"? This is a very hard question and I don't have the answer.

So I think @Igmat collecting feedbacks from community is the only way we can do now, and I decide to follow his way, though I was very unwilling to do that because it could cause break between the community and the committee.

@hax
Copy link
Member

hax commented Nov 21, 2018

you only have to learn the meaning of #; the rest is intuitive aside from some edge cases.

Actually public field is also new syntax, and you eventually have to learn that the meaning of = in class body is totally different. How could it be intuitive?

And you also need to learn this.#x actually is not property and have to learn all consequence of it (eg. no way to iterate private fields) though it looks very like pubic field and both are named "field". How could it be intuitive?

The worst part is, the slogan "# is new _" is too attractive and the superficial usage of it in React make the programmers lose the alertness of such a new feature which have many consequences for real world engineering (especially of OOP).

@ljharb
Copy link
Member

ljharb commented Nov 21, 2018

@hax the = is intuitive in that most people who see it immediately understand that it's applied per-instance, without explanation, and without needing to learn anything. imo the common path of learning it won't be "these are public fields, here's how they work" - it'll be by finding it in code and trying to understand what it does. Learning the term "field" likely comes later. Most people will never need to know about Set vs Define (no matter the default) for public fields, and I personally think that the majority will never even attempt to dynamically iterate an instance inside the class body such that they'll run into being unable to do so on private fields.

I'm still not sure what "OOP consequences" you mean. The ability to encapsulate functionality such that it can't be tapped into, reflected on, or overridden is a very important part of OOP.

@rdking
Copy link

rdking commented Nov 21, 2018

@mbrowne

You have repeatedly insisted that top-level, non-static declarations inside a class should only define things that go on the prototype. And yet you make an exception for the instance variable declarations in classes 1.1 and class-members (var/let/const). I realize that instance variables are in no way properties, but at least regarding the syntax your argument is inconsistent.

Ok. I get your point here. Let me make my argument more clear. Any declaration in a class that begins in the same way as a method declaration ((get/set/static) [[IdentifierName]] ...) in the existing class definition belongs on the prototype. To add a definition that doesn't belong on the prototype, it should begin with a keyword that

  1. doesn't just move it to the constructor, and
  2. doesn't resemble property descriptor properties or their inverse

It's following this approach that the let/const in class members is allowed. The semantic of these terms already expresses exactly what will be created, a per-instance variable within the lexical scope of the class. Hopefully this makes my argument more clear.

@mbrowne
Copy link

mbrowne commented Nov 22, 2018

@rdking You're nitpicking, and other people will see it the opposite way. The field syntax includes the = sign (at least when there's an initializer, and most of the time at least one field in a class will probably have an initializer), so it's already distinct from method syntax. Yes, the presence of a keyword makes the difference a bit more obvious but your argument is weak, especially considering that people with experience in other OOP languages (and of course Babel/TypeScript) probably already expect x = ... notation to add a property to the instance (not to the prototype or the constructor function).

The semantic of these terms already expresses exactly what will be created, a per-instance variable within the lexical scope of the class.

There's nothing that makes it immediately clear that it's per-instance. In fact when I first saw it, I thought it looked like a private class-scoped (static) variable.

@rdking
Copy link

rdking commented Nov 22, 2018

@mbrowne Saying I'm nitpicking is a meaningless jab. The devil is in the details. If you're not willing to scrutinize the details, you'll miss opportunities. The = makes it clear that what's being defined is a data property as opposed to a function property. That distinction is important all by itself and = is well suited to that purpose. However, what in the declaration makes it clear that this property isn't going where all other declarations are going?

Yes, the presence of a keyword makes the difference a bit more obvious but your argument is weak, especially considering that people with experience in other OOP languages (and of course Babel/TypeScript) probably already expect x = ... notation to add a property to the instance (not to the prototype or the constructor function).

Strawman argument again. Other OOP languages are class-based, not prototype based. They have metadata for the class template. All class properties are automatically on the instance because there's no other place for them to be! In prototype-based languages, the prototype is the template.

My argument isn't as weak as you try to make it out to be. Look at how class currently works:

  • static makes it clear this declaration is going on the constructor instead of the prototype.
  • get/set makes it clear this declaration is going into a property definition to be applied to a prototype.
  • all declarations without a keyword prefix go on the prototype

Notice that declarations use keywords when the declaration doesn't get applied directly to the prototype? That's the precedent for my argument. There's no way that matching pattern in the existing language constitutes a "weak argument".

There's nothing that makes it immediately clear that it's per-instance. In fact when I first saw it, I thought it looked like a private class-scoped (static) variable.

I might have to relent on that one, but I'm not so sure. Currently no definition in the class declaration exists without being instantiated unless it uses the static keyword. My proposal continues that pattern. It's not static if it doesn't use static. In my mind, this makes it fairly easy to reason about what is an instance member and what is not. How will others see it? I can't say, but I would hope a brief explanation would be more than sufficient to bring anyone up to speed.

@hax
Copy link
Member

hax commented Nov 22, 2018

@ljharb

the = is intuitive in that most people who see it immediately understand that it's applied per-instance, without explanation, and without needing to learn anything.

Really "without explanation"? Really "without needing to learn anything"?

I hope you could do test in the wild. Find someone who never heard class field before then throw this code to them:

let a = 1, b = 2;
class Test {
  a = 3;
  b = a + 1;
}
const test = new Test();
console.log(a, b, test.a, test.b)

Let them tell you what result they expect.

Do be ostrich, do it yourself!!!

it'll be by finding it in code and trying to understand what it does.

Yes let the developers who never see class field before read the code I provide and try to understand it and tell you the truth.

For those already know or used class field (especially the react users), throw this one to them:

let a, b;

a = [1,2,3];
[b] = a;

class Test {
  a = [1,2,3];
  [b] = a;
}

const test = new Test();
console.log(a, b, test.a, test.b)

Let them tell you what result they expect.

If you don't like do the real world test, no problem. I will do it in Jan 6th, 2019. There will be 500+ china js programmers in the D2 conference, and I will report the result to you.

Learning the term "field" likely comes later.

They'd better never learn the tragic term which convey the wrong information of similarity of public/private.

Most people will never need to know about Set vs Define (no matter the default) for public fields,

Yes, why they need to know? So let them only know it when they shot by footguns.

@hax
Copy link
Member

hax commented Nov 22, 2018

@ljharb

Here is the only useful use case discussion part.

I personally think that the majority will never even attempt to dynamically iterate an instance inside the class body such that they'll run into being unable to do so on private fields.

So what you mean is your use case is iterating data properties only outside of class?

Could give a real world code sample for us to understand the real concept model of using it? What data properties (actually Object.keys not return all data properties but only enumerable string keys.) really mean for them in these cases?


Note, we also heavily use object destructuring. You should notice that it have different semantic with object spread, Object.assign, Object.keys, etc.

  • Object destructuring use [[Get]] and respect all properties (both own and prototypes')
  • Object.keys/values/entries only respect own enumerable string property
  • Object spread and Object.assign respect own enumerable properties (both string key and symbol)

There is also Reflect.ownKeys, Object.getOwnPropertyNames, Object.getOwnPropertySymbols..., don't forget the old for...in! Though I guess they are not used very frequently. But who knows whether someone rely on a special API and think it's a important use case even for classes object ?

So I very doubt we can inspect an object in a easy way and solely rely on the usage of special API.

We'd better check the real world use case and find what the real concept model they are using, instead of saying we should follow any inconsistent object reflection functionality in classes arbitrary.

@hax
Copy link
Member

hax commented Nov 22, 2018

@ljharb And I suspect the usage of reflection of own props of class object (not plain object) may essentially collide the encapsulation idea which is why private field exists.

Consider an exist class which (have to) use data properties to store internal state now. With private field available, the author could decide to hide some data properties, use private field instead. And your assumption of iterating of own props is must to have is just conflict with it. If anyone rely on iterating all data properties outside of the class, such class could never use private field.

We should realize there are two different usage of classes:

  1. Use class for encapsulation, aka. the OO way (or, let's say the way which much close to Java/C# or other OOP language usage). Even we rely on data properties which everyone can iterate from outside, it's just because we don't have real encapsulation, it's not the design contract provided by the author of the class, just the inevitable companion because of the language limitation.

  2. Use class for object factory.

Private field is based on 1, and public field may based on 2. These two may have essential conflict. And current proposal just bundle them together and the duality of syntax and name (both 'field') just make the situation even worse.

@mbrowne
Copy link

mbrowne commented Nov 23, 2018

@hax

Find someone who never heard class field before then throw this code to them:
...
Let them tell you what result they expect.

(Referring to your first code sample) that would not be a fair survey. For people who have never seen or used class fields before, the first step should be to show them working code, not invalid code that doesn't even run successfully. At the very least, show an example with basic, correct syntax (e.g. foo = 1) before showing broken syntax or you will bias the results (I hope you can see my point without further explanation but let me know if you'd like me to elaborate on why this could bias people's expectations). Additionally, you could show a valid usage of this in a field initializer and see if anyone finds it confusing.

@hax
Copy link
Member

hax commented Nov 23, 2018

@mbrowne

(Referring to your first code sample) that would not be a fair survey.

How could a code sample be absolute fair?

For people who have never seen or used class fields before, the first step should be to show them working code, not invalid code that doesn't even run successfully.

The sample code is valid and runnable in latest chrome (with flag enabled) and babel 7.

But I guess you mean we should first introduce a "good example" to teach them what's the syntax designed to behave. I don't necessarily disagree this way. That's why I also provide the second code sample for those who think they already know public field well.

At the very least, show an example with basic, correct syntax (e.g. foo = 1) before showing broken syntax

The code examples are totally "syntax correct" as public field proposal. If you think it's a "broken syntax", you should agree current public field have a "broken syntax". 😜

or you will bias the results (I hope you can see my point without further explanation but let me know if you'd like me to elaborate on why this could bias people's expectations).

I guess I know what you mean. But for the same reason, if you introduce public field motivation/syntax/semantic first, you also bias people's expectations --- Many people will become blind to the gotchas until they was caught in real world programming (I just found this is why they are called GOTCHAS 😜 ).

As my observation, when you tell a programmer what intend to work, they will try to decode the underlying logic (for example, transpile the code in their mind) and follow your design by default, even your design was broken.

This is what is happening in dealing with public fields --- auto transpile a = 1; b = a + 1 to this.a = 1; this.b = a + 1 in our mind, even it's very weird and extremely error-prone to make two a refer to different thing (I never see such awful thing in any other mainstream languages).

Even worse, most programmers have trained themself always follow the rule: if the result is not you want, that's your fault, and you should RTFM. Such rule is good for practice, because you can never change the language, you eventually have to swallow it. But it also means the programmers are very tolerant and insensitive to the design flaws of the language. And someone even refuse to admit it's a flaw, and argue that "it's by design".

/* Here I give you an example of PHP which maybe a little bit off-topic: In PHP, you can write const a = [1, 2, 3]; but never const f = function () {};. Someone ask why on stackoverflow, most answers are just "function is not constant, you should RTFM". Actually, though PHP const means compile-time constant (different to JS const), function literal is definitely a compile-time structure. The real reason why PHP can't support function constant, is the historical design flaw of the language. See my full explanation. Not supporting function constant in PHP only add a small surprise and inconvenience, not important in real world programming, so we can ignore it. But we should agree public field in JS is very different. */

That's why I provide the first code sample, let the programmers use their original intuition to investigate the syntax.

Additionally, you could show a valid usage of this in a field initializer and see if anyone finds it confusing.

Yesterday I gave a speech about class fields in my company, and something interesting happened. A talented programmer in our React team see my slide of the first code sample and told me the result have to follow class field semantic or it doesn't make sense (this is just the reaction of "it's by design" I mentioned before). And my next slide is:

class HelloWorld {

	name
	greeting = `Hello ${name || 'world'}!`

	constructor(name) {
		this.name = name
	}

	run() {
		console.log(this.greeting)
	}
}

new HelloWorld().run()
new HelloWorld('hax').run()

This buggy code have the result of two time Hello world output in Browser. When I asked why, the guy I mentioned before said: "Because the execution order of field initialization is after super() and if no base class then just before all statements of the constructor, and field without initialization will be undefined. So the name in Hello ${name || 'world'}! is always undefined, so output Hello world twice."

See? He is a very good JS programmer (in the top 10% I believe) and obviously know many details about the semantic of class field proposal. But the explanation is wrong. When I pointed out name in Hello ${name || 'world'}! should be this.name as his explanation, he just shocked: "Shit! I'm stuck!"

We also discussed, in real world what will happen if you want to fix the code but with wrong understanding about the reason of the bug, but I have written too much again, so leave this part to you.

Hope you see my point.

Yes, a = this.b + 1 is very ok. But the questions are:

  • Are you sure you will never write a = b + 1 when you really want a = this.b + 1?
  • Are you sure your team members never write a = b + 1 when they really want a = this.b + 1?
  • Are you sure when you do code review and see a = b + 1, you can easily confirm whether it's correct or a mistake of a = this.b + 1?

All programmers attending the discussion in my company agreed the answers are three NO.

@mbrowne
Copy link

mbrowne commented Nov 23, 2018

@hax Could we continue this discussion in a new thread? This thread has become so long that it's difficult to navigate...348 "hidden items", and you can't even expand them all at once making it difficult to look up comments from a few weeks ago. I created a new issue here:

#175

If that works for you then I will respond to your latest comment there. We could CC everyone who has been recently participating in this thread.

@hax
Copy link
Member

hax commented Nov 26, 2018

@mbrowne That will be great!

@trusktr
Copy link

trusktr commented Nov 10, 2021

@douglascrockford Have you taken a look at this proposal and the high amount of votes against it? It might be a worthy contender for the bad parts.

Besides syntax dislike that some people have with the private portion of the proposal (which is of no consequence to a pragmatic engineer), the proposal actually has fairly bad technical issues waiting to chop someone's foot off:

  1. Private members break proxies #106
    • Implication: Library author decides to add "implementation detail" to library by using private fields within the internal library code. Subsequently consumers depending on the library and using Proxy to wrap private-field containing objects break experience a breaking change that the library author had absolutely no idea about.
  2. Public fields "Define" semantics and subclasses #176, Insufficient time allotted for acceptance testing [[Define]] semantics #273, and many issues
    • Implication: x = 123 in a class definition is not sugar for this.x = 123 inside a class constructor, but sugar for Object.defineProperty(this, 'x', {value: 123}) inside a constructor. This means that class fields do not trigger inherited setters, and subclass authors can not intercept base class properties with accessors, etc. A library base class author might like to change a property to an accessor, and it breaks. Or for example, a developer desires to switch a property to class field "sugar" syntax and the app totally breaks. Etc.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests