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

Instance and prototype fields #265

Closed a-ejs closed 4 years ago

a-ejs commented 5 years ago

This is an updated summary of the problems I see with new syntax. I might update this with more points later.

It is unintuitive

Given that this:

class foo {
  bar() {}
}

is practically identical to:

function foo() {}
foo.prototype.bar = function() {};

it is not entirely unreasonable to expect

class foo {
  bar = null;
}

to be the equivalent of

function foo() {}
foo.prototype.bar = null;

but it isn't.

Likewise, in this class definition:

class B extends A {
  foo() {}
}

method foo of B.prototype expectedly takes priority over inherited method foo from A.prototype:

class A {
  foo() {}
}

But if A were defined like this:

class A {
  foo = 1;
}

behavior would be radically different, which is not apparent.

It is inconsistent

It is also inconsistent with similar constructs in the language or this very proposal. For example, if you add the keyword static, this:

class foo {
  static bar = function() {};
}

expectedly does become identical to

class foo {
  static bar() {}
}

Object literals use somewhat similar syntax, so this:

{
  foo() {}
}

is also practically same as

a = {
  foo: function() {}
}

Proposed class fields on instances is not consistent with any of this behavior.

It does not add any clarity

Properties on class instances are initialized in the constructor, like so:

class {
  constructor() {
    this.listOfThings = [];

    // do other work
  }

  /* ... */
}

Everything about this example is clear. It defines a constructor function that, when called, sets the property listOfThings of this to a new array. There's also zero ambiguity to when this assignment is done and how it works with inheritance.

Proposed syntax simplifies it to:

class {
  constructor() {
    // do the work
  }

  // The following line can go anywhere in the class body
  listOfThings = [];
}

This is, supposedly, somehow an improvement. But where it saves the programmer the need to type this. in front of the definition, it makes the behavior much more implicit and less apparent.

It changes the meaning of class syntax

Classes in JavaScript is just an abstraction, syntactic sugar, whatever you want to call it, over its prototype-based inheritance. A "class" is defined by a constructor function and its prototype. So, class syntax is just an easier way to define that function and the prototype.

Body of class definition describes properties of the prototype or the function itself, only the constructor property being somewhat special since it describes the function itself. But it still exists as a method property (syntactically) and even goes into the prototype (as A.prototype.constructor) - that's also pretty neat. (Although that behavior is unrelated to class syntax, it's just a nice touch. And it's consistent!)

class A {
  // Everything here describes the properties of A and A.prototype
  constructor() {
    // This defines the body of A
  }
}

New syntax invalidates all of that. Now, the class body describes both what the constructor is and what it does, in one place. And since it doesn't replace the constructor() {} syntax, what it does is now defined both inside and outside the function body, something extremely unusual and not achievable with other syntax.

It mixes function code with normal code

As far as I know, this behavior is not present anywhere else in the language.

A function's body is a sequence of statements that's executed when the function is called, as opposed to non-function object literals that are evaluated right away.

Class body is not a function body. It describes the function and its prototype as an object. With instance fields, class body becomes mixed code, where some parts are evaluated right away, and some are evaluated with the constructor:

class {
  // definition
  constructor() {}

  // definition
  method() {}

  // function code
  a = 1;

  // definiton
  static a = 1;
}

This is very unusual for JS. Just because similar behavior exists in other languages like C++ doesn't necessarily mean it would work perfectly fine in JS, especially considering how C++ classes and JS prototype inheritance have very little in common besides general purpose.

Imagine if function properties could be defined within its body:

function foo() {
  doAThing();
  bar: baz();
  doAnotherThing();
  return 123;
}
// as sugar for
function foo() {
  doAThing();
  doAnotherThing();
  return 123;
}
foo.bar = baz();

This isn't much different from class fields.

Summary

This syntax provides very little, if any, benefit for all the downsides that it has.

I don't see how current way of initializing instance properties is so bad that it needs to be simplified or changed in the first place.

New syntax is deceiving, confusing, does not promote good practice. Someone who doesn't know better could easily create more instances than needed of the same thing without realizing, as an example. It can easily become another "ugly" part of the language, for the lack of a better word.

It's very strange to me that apparently very few people seem to see an issue with all this.


original issue text below


The following code:

class foo {
  bar() {}
  baz() {}
}

defines a "class" foo with "methods" bar and baz - "class" and "methods" being quoted because that's just syntactic sugar over defining a constructor function and a prototype, like so:

function foo() {}
foo.prototype.bar = function() {};
foo.prototype.baz = function() {};

So if I were to swap out a method definition for another value, I would expect something like this:

class foo {
  bar = 1;
  baz() {}
}

to become (identical to):

function foo() {}
foo.prototype.bar = 1;
foo.prototype.baz = function() {};

but instead it becomes:

function foo() { this.bar = 1; }
foo.prototype.baz = function() {};

which is counter-intuitive, and frankly not that useful (saves typing the this. part in constructor, whereas adding properties to the prototype remains a pain in the ass), but I think the former should be the focus.

So why does it work that way? Seems much more confusing than it is useful.

Likewise, the whole concept of "private fields" brings in too much complications while not being a very useful addition in general to say the least, and I don't think it belongs in the language - but that's just my opinion (and isn't the subject of this issue).

bakkot commented 5 years ago

Suppose it worked like that. Then

class foo {
  bar = [];
  baz() {
    this.bar.push(10);
    console.log(this.bar);
  }
}

let a = new foo;
a.baz(); // [10], as you expect

let b = new foo;
b.baz(); // [10, 10], as almost no one expects
a-ejs commented 5 years ago

Yes, which is why you would want to initialize that member in the constructor. That way there's zero confusion on how things work, whereas this syntax is not nearly as intuitive.

bakkot commented 5 years ago

In practice, very few people seem to find the specified behavior surprising.

a-ejs commented 5 years ago

And yet it is inconsistent. And I don't see how "in this scenario some people find otherwise normal behavior surprising" is a valid argument in favor of introducing such inconsistency.

Speaking of being practical, current behavior saves the programmer the need to type this. in constructor body for every member initialization, and maybe the constructor() { } bits if no other work is done in constructor. On the other hand, properly extending the prototype with non-function values remains ugly (in which case why use the nicer syntax in the first place).

I don't like that now methods (functions) get this special treatment of going into the prototype whereas everything else implicitly becomes a member that is initialized on every object. You provided an example with an object (array), but functions are also objects:

class foo {
  bar() {}
}

const a = new foo;
const b = new foo;
a.bar.baz = 1;
b.bar.baz // 1

Is this confusing behavior? Not really, because normally you don't use method functions to store data or manipulate them in that fashion. But it doesn't mean that every other type of object necessarily gets the opposite treatment, there are countless others situations where you don't modify the state of member object, only read it.

Here's a counter-example involving an array:

class foo {
  bar = [1, 2, 3];
  setBar(bar) { this.bar = bar; }
  printBar() { for(const item of this.bar) console.log(item); }
}

In this case, the array [1, 2, 3] serves as the default value for bar, and it doesn't have to be initialized for every instance of foo. If my code involved manipulating the contents of said array, I would have to explicitly define the bar property for every new instance in the constructor, or use a different approach entirely.

A different example...

class TheoreticalEventTarget {
  // Default value for `eventHandler`.
  // There's no need to create a new `EventHandler` object for every `TheoreticalEventTarget` object, especially since it might get overwritten right away.
  eventHandler = new EventHandler();
  setEventHandler(handler) { this.eventHandler = handler; }
  runEvent() { this.eventHandler.run(); }
}

Do you see my point?

To sum it up:

ljharb commented 5 years ago

Storing non functions on prototypes is incredibly rare, and far more often a bad practice.

bakkot commented 5 years ago

Current behavior of assigning values to members in class body is inconsistent and confusing,

Again, in practice very few people seem to find it so. "Methods go on the prototype, fields go on instances" is a fairly easy rule to remember, in those rare cases where one needs to. The syntax for methods and fields is not the same; it does not need to have identical behavior.

The opposite (consistent) behavior also has its use cases, and they're not rare either.

I do not think the use cases of this outweigh the risk of accidentally sharing state between instances.

For your example, it is not much really that much more typing to write

let defaultHandler = new EventHandler();
class TheoreticalEventTarget {
  #eventHandler;
  get eventHandler() { return this.#eventHandler ?? defaultHandler; }
  setEventHandler(handler) { this.#eventHandler = handler; }
  runEvent() { this.eventHandler.run(); }
}

and I think this is, in addition, less likely to mislead readers.

a-ejs commented 5 years ago

How is it a bad practice exactly?

And what kind of statistic are the claims about "most people" or "incredible rareness" based on? I don't think that's a very strong argument.

ljharb commented 5 years ago

It’s the same class of argument as your “they’re not rare either”, i suppose.

bakkot commented 5 years ago

And what kind of statistic are the claims about "most people" [...] based on?

Babel has implemented the per-instance behavior in this plugin and its predecessor (and in 5.0, for that matter) for years. Typescript does the same (and has for even longer, if I recall), as did traceur. People very rarely seem to get tripped up by this.

(Edit: I should also add that there is very little chance of this changing. We were aware of this tradeoff when we chose this design, and as far as I'm aware, no member of TC39 has ever expressed the slightest inclination to change it, going back to the earliest iteration of this proposal.)

a-ejs commented 5 years ago

@bakkot It is very similar to the new object syntax:

{
  a() {}, // instead of a: function() {}
  get b() {} // instead of having to do Object.defineProperty(...)
}

and it's not a very bold assumption to make for someone not familiar with specifics of the language that everything that goes into class body defines the prototype (in a language where inheritance is prototype-based!)

Like here:

class foo {
  bar = 123;
  constructor() {
    // do stuff
  }
  baz() {}
}

Do you think it's obvious that bar is an instance property initialized on every new object?

And what if I do this?

class foo {
  constructor() {
    this.a = 1;
    // do other stuff
  }
  baz() {}
  bar = 123;
}

For your example, it is not much really that much more typing to write...

It is much more than just one this. per member with the current behavior.

As in, I could say the same thing about

class foo {
  constructor() {
    this.bar = [];
  }
}

instead of

class foo {
  bar = [];
}

(and that's just the best case scenario where constructor is otherwise empty, because if it wasn't it'd save even less typing)

And please note that it is completely unambiguous what bar is when it's initialized in the constructor.

Point is, both use cases can be done without new syntax, but current behavior saves much less time/work, does not make the code much prettier, but makes the language more complicated and (to some, I guess) less intuitive.


@ljharb That was hardly even an argument, more like a side note. Either way, it doesn't affect other points.

bakkot commented 5 years ago

it's not a very bold assumption to make for someone not familiar with specifics of the language that everything that goes into class body defines the prototype

Such an assumption is wrong even today - class A { static m(){} }. Different syntactic forms end up in different places. (If someone is unsurprised by this because they come from a language which uses static similarly, well, such languages also tend to run field initializers for each instance.)

does not make the code much prettier,

It makes the code much clearer, by making the shape of instances declarative (assuming someone is using the feature consistently, at least, which we cannot force them to). Since the point of classes is to be declarative, that's a large win. Clarity is important. "Pretty" is not so important, though I think it is also prettier.

a-ejs commented 5 years ago

As I said, method syntax in class body is very similar to that in object literals, so if

{
  a() { return 123; }
}

is identical to

{
  a: function() { return 123; }
}

then it's not very obvious that

class {
  a() {}
}

and

class {
  a = "some value";
}

have nothing in common. You can't say the same about the static keyword.

As for "other languages", let's not forget that javascript "classes" have very little to do with how classes work, or what "class" even is in, say, C++. So I don't see why we should try to imitate that syntax.

And as for clarity... How is defining properties in the constructor not clear? I can't believe that proposed syntax is an improvement in that regard. From code prettiness standpoint - sure, it's subjective (I personally don't think so), but definitely not clarity, if we're making that differentiation.

Are you saying that this:

class {
  foo = [];
  bar = 123;
  baz = new Thing;

  constructor() {
    // do some work
  }

  // ...
}

is clearer than this?

class {
  constructor() {
    this.foo = [];
    this.bar = 123;
    this.baz = new Thing;
    // do some work
  }

  // ...
}

What's unclear about the latter? If anything, it's a downgrade, because it's not obvious whether or not these properties are initialized before or after constructor call, if the order matters (relative to constructor and each other), etc.

And here's how these properties must be added to the prototype with the current behavior, for example.

const a = class { // notice how we must assign it to an identifier to the be able to access its prototype property which makes it impossible to define this in a single expression without function wrap
  constructor() {
    // do some work
  }
  // ...
}
a.prototype.foo = [];
a.prototype.bar = 123;
a.prototype.baz = new Thing;
bakkot commented 5 years ago

is identical to

Those are not identical; they have different semantics. They do have the same placement, but there is only one placement possible.

have nothing in common

They have a lot in common: they are both accessible as this.a. They have different placement, but classes create multiple objects where an object literal creates only one, so this shouldn't be (and, again, to most people isn't) too surprising.

Are you saying that this [...] is clearer than this?

In that very simple case, no. If you have more logic in the constructor, yes.

And here's how these properties must be added to the prototype with the current behavior

Again, we think adding properties to the prototype is generally dangerous. We are not going to add syntax to encourage it.

rdking commented 5 years ago

Notes:

Those are not identical; they have different semantics. They do have the same placement, but there is only one placement possible.

{
  a() { return 123; }
}

is closer to

{
  a: () => { return 123; }
}

but even then there are still semantic differences. However, I believe @a-ejs is arguing on the basis of placement, not on semantics regarding the setting of internal properties.

As for the argument about class already supporting differing placements, well, even that has a structure that's flatly being violated by this proposal. For every property of the class, that structure looks like this:

[[Modifiers]] [[Identifier]] (( = <<expression>>) || ([[ArgumentList]] { <<body>> }))

and it's the [[Modifiers]] that determine this placement. So question, where is the modifier that says fields don't go on the prototype? If there isn't one, then congratulations. You've caused an inconsistency in the structure of property definitions in a class statement.

bakkot commented 5 years ago

For every property of the class, that structure looks like this:

[[Modifiers]] [[Identifier]] (( = <<expression>>) || (([[ArgumentList]]) { <<body>> }))

The = <<expression>> part is not currently a part of the syntax of classes. It is a new syntactic form introduced by this proposal. It has different semantics than other syntactic forms, as new syntax usually does. We are comfortable with this.

a-ejs commented 5 years ago

In that very simple case, no. If you have more logic in the constructor, yes.

All the logic still goes below the definition, especially since there's not much (any) variability to the values of these members. Or am I missing something?

they are both accessible as this.a

"nothing in common" was probably an overstatement, but there's still a considerable difference between prototype property and instance property. Constructors create objects, and class syntax defines constructors.

When new syntax is used, it's not just about the placement of the value, it's that the value isn't even evaluated with the class expression, but instead each time the constructor is called. Is it really 100% straightforward and obvious?

You're talking about the dangers of extending the prototype and accidentally introducing a shared value, but isn't such deceiving syntax also dangerous?

I would say that the opposite thing, i. e. unnecessarily spawning objects/other values for every class instance, is also bad practice and can also lead to some obscure bugs (although mostly performance issues).

Class syntax initially was (is) a prettier way to define constructors, it did it without introducing any ambiguity. This:

function A() {
  // constructor
}
A.prototype.b = function() { /* method */ };

is a series of declarations/assignments, so is this:

class A {
  constructor() {
    // constructor
  }
  b() { /* method */ }
}

Everything is perfectly clear, both snippets do the same thing, the only difference is syntax used. But...

class A {
  // class body is no longer a once-evaluated
  // definition of the class. Instead, some fields
  // here effectively modify the constructor body.

  a = getValue(); // evaluated each time constructor is called.

  b() {} // but this is a method, constructed once and right away

  constructor() {
    // this is a function body, so it's obvious that
    // any code here will be run when called
  }
}

Now some parts of the class body are evaluated right away, and some only when the constructor is called. How is it "clear"?

I don't think there are any other examples of similar behavior in the language as of now, might be wrong though. It compares to adding the ability to define function properties in function definitions like this:

a = function() {
  statement;
  doAThing();
  &foo: 1; // this part of function's body sets its `foo` property at definition, doesn't do anything when it's called
  doAnotherThing();
};
a.foo; // 1

instead of

a = Object.assign(function() { /* ... */ }, {foo: 1});
// or just
a = function() { };
a.foo = 1;

This example, while being somewhat ridiculous, illustrates the point pretty well.

rdking commented 5 years ago

The = <> part is not currently a part of the syntax of classes. It is a new syntactic form introduced by this proposal. It has different semantics than other syntactic forms, as new syntax usually does. We are comfortable with this.

Yeah, my mistake there. However, whether including or ignoring that piece, the point remains, no [[Modifier]] means no expectation of placement other than on the prototype. The fact that TC39 is comfortable with doing something that conflicts so obviously with existing structure (and therefore mental model) without a broadly compelling reason is a bit disturbing.

a-ejs commented 5 years ago

Also: another source of confusion is that this

class {
  static a = function() {};
}

is, to my understanding, identical to this (minus minor differences like toString return value)

class {
  static a() {}
}

but this

class {
  a = function() {};
}

is not the same as

class {
  a() {}
}

So when static is used, values defined with both method syntax and new syntax are evaluated right away and go into the same place, but remove that keyword and meanings become radically different.

bakkot commented 5 years ago

Is it really 100% straightforward and obvious?

Most people seem to find it so, yes.

Everything is perfectly clear, both snippets do the same thing, the only difference is syntax used.

No, they don't. They do similar but different things. This becomes especially obvious when you start using some of the additional features afforded by class, like extends and super.

Now some parts of the class body are evaluated right away, and some only when the constructor is called. How is it "clear"?

Again, most people seem to find it clear. It is impossible to make an objective argument for what is clear and what is not, but again I point you to the behavior of Babel and TypeScript and Traceur, all of which have implemented the per-instance behavior for years and seen broad adoption with little confusion. I am sorry your intuition differs. I don't think there's much more that can be said here.


The fact that TC39 is comfortable with doing something that conflicts so obviously with existing structure (and therefore mental model) without a broadly compelling reason is a bit disturbing.

We disagree that there is a conflict. It is new syntax. It has new semantics. New syntax usually has new semantics.

ljharb commented 5 years ago

For what it’s worth, that can be avoided by not putting functions in fields at all.

a-ejs commented 5 years ago

No, they don't. They do similar but different things. This becomes especially obvious when you start using some of the additional features afforded by class, like extends and super.

I provided a simple example, the differences there are minimal and practically non-existent (same scale as different toString output for functions). As far as I'm aware, no part of class syntax allows doing something that was previously impossible or too complicated - extends and super included.

The new syntax, however, extends what constructor function does from outside its body - not something achievable with other syntax. And the main issue is how "function code" (statements evaluated on call) and method definitions (evaluated right away) go into the same scope, behavior that isn't seen anywhere else in the language. And it's just one of the ways in which it is, yes, confusing and deceiving.

Am I really the only one who sees it as an issue?

The only thing gained from this addition is not having to type this. in front of these initial definitions. It does not add any clarity. Is it really worth complicating the language this much for something this simple? Or am I missing something?

bakkot commented 5 years ago

As far as I'm aware, no part of class syntax allows doing something that was previously impossible or too complicated - extends and super included.

Depends on what you mean by "too complicated", I guess. Getting the home object right for super isn't trivial.

The new syntax, however, extends what constructor function does from outside its body - not something achievable with other syntax.

"extends what the constructor function does from outside its body" is pretty much equivalent to "wraps the constructor function", which is something which has always been possible. Though I would not generally encourage you to think of it that way; rather, I would encourage thinking of the entire class as defining how to construct objects.

Am I really the only one who sees it as an issue?

No, but you are in a very small minority. I am not aware of any member of TC39 who shares your opinion, or who has shared it since the beginning of this proposal.

The only thing gained from this addition is not having to type this. in front of these initial definitions.

The benefit is that it allows class fields to be written declaratively in the class body, rather than imperatively in the constructor. Since the point of classes is to be declarative, that's a large win. Saving keystrokes is not the point.

a-ejs commented 5 years ago

"extends what the constructor function does from outside its body" is pretty much equivalent to "wraps the constructor function", which is something which has always been possible.

The constructor body was in the constructor definition constructor() {}, like any other method definition. Now it extends beyond that definition, and the class body is a mix of method definitions and parts of constructor body.

The benefit is that it allows class fields to be written declaratively in the class body, rather than imperatively in the constructor. Since the point of classes is to be declarative, that's a large win. Saving keystrokes is not the point.

I don't see how it's more declarative, but that's not even the point. This syntax introduces ambiguity to what class body even means. It used to describe the constructor and the prototype. Now it's a mix of that and description of individual objects, but not the properties that class instances have through the prototype, rather their own properties that are assigned when constructing the object. Many important details regarding how and when these values are assigned, how they work with inheritance etc. are also not apparent at all. It's like taking a step forward then multiple steps back.

I feel like a huge part of the idea that this syntax is somehow better comes from that it's similar to class syntax in languages like C++. But javascript's inheritance is just completely different and ignoring that in favor of introducing behavior similar to other languages' isn't a good thing in my eyes.

a-ejs commented 5 years ago

Another thing: you can already imitate the non-method prototype properties using getters, but that adds performance overhead which isn't very nice.

(I do understand the position on prototype properties, it's just a side note.)

rdking commented 5 years ago

Again, most people seem to find it clear.

Do they find it clear before explaining? Or does there need to be a bit of explanation before the difference is understood? Just claiming that most people get it doesn't necessarily account for what it took to help them get it.

We disagree that there is a conflict. It is new syntax. It has new semantics. New syntax usually has new semantics.

Ignoring private fields, it's not so much a new syntax as it is an obvious extension to a prior syntax. The conflict comes because the natural expectation (one that does not require additional explanation) has not been met. In its place, new semantics were added to avoid a well-known, well-handled with mere knowledge, and yet somehow still useful, foot gun. What did we get in trade? Something that can cause unexpected issues and requires yet more explaining and working around.

Look. It's as simple as this: You had to make "trade-offs" because there were "conflicts". That's the nature of the type of decisions TC39 was faced with in this proposal. If you admit that there were trade-offs, then you are likewise admitting that there are conflicts. If there weren't, no trade-offs would have been necessary.

bakkot commented 5 years ago

Do they find it clear before explaining?

Yes.

It's as simple as this: You had to make "trade-offs" because there were "conflicts". That's the nature of the type of decisions TC39 was faced with in this proposal. If you admit that there were trade-offs, then you are likewise admitting that there are conflicts. If there weren't, no trade-offs would have been necessary.

Of course. If there were no trade-offs, design would not be necessary.

rdking commented 5 years ago

Of course. If there were no trade-offs, design would not be necessary.

Thank you for finally accepting that you've created a conflicted design.

Do they find it clear before explaining?

Yes.

Did they come from a TypeScript background where they've already encountered this kind of thing?

You see, a lot of @a-ejs's argument about the clarity of the placement of these "fields" seemed to be trying to account for developers new to the language. If it wouldn't be clear to them without explanation that "methods go on the prototype object in a prototype-oriented language, but you should never put data properties on the prototype", then what you've done here simply isn't clear.

I wouldn't expect that to be enough to get you to change your direction with this unfortunate proposal, but I would expect that you could at least understand and accept the perspective being presented.

bakkot commented 5 years ago

Thank you for finally accepting that you've created a conflicted design.

All design work involves tradeoffs. Otherwise it would not be necessary. I don't know what it means to describe a particular design as conflicted.

Did they come from a TypeScript background where they've already encountered this kind of thing?

Not universally, no.

rdking commented 5 years ago

All design work involves tradeoffs. Otherwise it would not be necessary.

This is true. As someone who's designed everything from OS's to enterprise applications, I'd only be making a fool of myself if I tried to deny this. However, there's something you're missing.

I don't know what it means to describe a particular design as conflicted.

What you're missing is that when designing something, you must never trade away the core principle of the existing thing or the new thing you're trying to create. A conflicted design is one where you've done either or both.

Where class-fields is concerned:

These are just 3 of the ways in which class-fields is conflicted. I haven't even touched on private-fields. The long and short is that while, as you said, there are always trade-offs, you've got to recognize the line where, once crossed, you've destroyed something important that you may not be able to get back.

The sad part about all of this is that there are still so many other approaches that can be taken with significantly less trade-offs, and no "fields". There's simply no need in ES for a delayed imperative statement like that, masquerading around as a declarative one.

ljharb commented 5 years ago

The prototype remains the shape of inherited data; fields make instance properties just like constructor code does. Nothing is violated here.

The shape of an instance has never been defined by solely the prototype - but by the combined semantics of the prototype and the constructor. Fields add to this. No conflict.

Constructor code can always use Object.defineProperty to avoid triggering setters in the base class. Nothing is violated here either.

The issue is that your concept of core principles is incorrect/incomplete.

bakkot commented 5 years ago

Constructor code can always use Object.defineProperty to avoid triggering setters in the base class. Nothing is violated here either.

(Not just that, but the existing pattern of

class Base {
  constructor() {
    this.foo = 0;
  }
}
class Derived extends Base {
  foo() {}
}

has exactly the same super-overrides-sub behavior.)

a-ejs commented 4 years ago

The prototype remains the shape of inherited data; fields make instance properties just like constructor code does. Nothing is violated here.

Which is why I say it's confusing when this code goes not into constructor body (where it belongs), but instead together with everything else that goes into the prototype (or constructor object itself with static).

The shape of an instance has never been defined by solely the prototype - but by the combined semantics of the prototype and the constructor. Fields add to this. No conflict.

With the class syntax, constructor body is (was) defined by constructor() { /* ... */ }. Now this syntax defines a function that does more than its body says.

And I can't stress enough how unsettling and just plain weird it is to me that the same lexical entity can contain definitions that are evaluated right away (method and static values definitions) and what is basically function code (class fields).

the existing pattern of ... has exactly the same super-overrides-sub behavior

Except here it is 100% clear and obvious, while new syntax leaves a lot of unanswered questions.

Class syntax without this addition is clear and concise. It's a function definition. Every method in class body is a property of that function or its prototype field. constructor field defines the function body. With class fields, it becomes a mess. Function code should not leave the function body.

More declarative doesn't necessarily mean better when a lot of important implementation details become unintuitive and contradicting. And in the end, what's gained? Not having to type definitions in the constructor body (even though that's where they end up anyway).

ljharb commented 4 years ago

@a-ejs currently, it's not just the constructor body in a derived class - calling super delegates authority to the superclass. Installing fields is the last step of calling super.

a-ejs commented 4 years ago

super is still called from the constructor, implicitly if constructor isn't defined.

ljharb commented 4 years ago

Just like class instance fields are implicitly initialized as part of super, whether you have an explicit super call or constructor, or not.

a-ejs commented 4 years ago

Then why not just do that in the constructor?

ljharb commented 4 years ago

https://github.com/tc39/proposal-class-fields/issues/265#issuecomment-524981552

It makes the code much clearer, by making the shape of instances declarative (assuming someone is using the feature consistently, at least, which we cannot force them to). Since the point of classes is to be declarative, that's a large win. Clarity is important.

a-ejs commented 4 years ago

I already responded to that. I explained many ways in which this syntax is confusing and inconsistent, and I don't see how it (or anything, really) can be clearer than initializing properties in constructor.

a-ejs commented 4 years ago

I updated the issue text with a summary of the problems brought up.

rdking commented 4 years ago

@ljharb

The prototype remains the shape of inherited data; fields make instance properties just like constructor code does. Nothing is violated here.

Methods are not "data" per se, yet they exist on the prototype. This goes against your notion that a prototype is for "inherited data". Also, if your notion is true, aren't instances supposed to inherit the default data from the class structure? That means that instead of "fields", prototype properties are more inline with your notion. That's a conceptual conflict.

If you go with the notion that they are "data", then I won't deny you. I'll simply point out that it makes no sense to segregate the location of data based on type in a weakly typed language. When you do so, you create a conceptual conflict. Turns out, you seem to be conflicted in your conceptual understanding either way.

The shape of an instance has never been defined by solely the prototype - but by the combined semantics of the prototype and the constructor. Fields add to this. No conflict.

There is truth in this. Due to the fact that when class was released, it came with no means of providing declarative data, the factory approach was all that existed for lexically defining the data in a class. Short of that, it had to be done externally to the lexical scope of the class definition. So in all, it's actually the combined semantics of the prototype, constructor, and the execution environment in which the class was declared. Given that the prototype is just another modifiable object, other execution contexts can get in on the act as well.

I agree that fields do not conflict with this. This wasn't my claim either. My claim is that "fields" conflict with both the conceptual nature of a class, in which the entirety of its shape is defined by the class' template, and the prototypal nature of ES, in which the prototype and the copy-on-write semantic perform a lazy "field" semantic by creating new instance properties from the prototype properties on write attempts. "Fields" are thus in conceptual conflict.

Constructor code can always use Object.defineProperty to avoid triggering setters in the base class. Nothing is violated here either.

Again true, but again misses the point. If a developer uses Object.defineProperty, that developer understands that they are intentionally suppressing the base class behavior. If the engine does it, the developer might get blindsided by an unintended suppressed behavior. That's both a conceptual and a usability conflict.

The issue is that your concept of core principles is incorrect/incomplete.

We may have to agree to disagree here. From where I sit, it is your understanding of core principles that is underdeveloped and insufficiently nuanced.

ljharb commented 4 years ago

Fields are in the class body, thus they’re part of the class’s template. That they’re a new templating form doesn’t mean your claim no longer holds.

I was using your “inherited data” terminology; i don’t consider functions data, so pretend i said “inherited values” or something equally generic.

rdking commented 4 years ago

Fields are in the class body, thus they’re part of the class’s template.

"Fields are in the class body": true. "thus they're part of the class's template": false. The problem is that class is a declarative statement. It's a description of something yet to be created, in this case, the class template. The only 2 products of a class are the constructor and prototype. The constructor is an instance initializer for the object that inherited the prototype. Neither the prototype nor the constructor contain any "fields". In fact, neither does the instance object after construction! The field declarations in the class declaration get translated into [[Define]] calls against the instance object. So they're not something that exists as part of a template, but rather an initialization step ala object factories that produce extra properties on the object. As I've said before, this is not in keeping with the concept of a class. Is it workable for ES? Of course it is (though the semantics could be better), but it cannot be said to be consistent with the concept of a class or the nature of prototypes. The nuances are important.

That they’re a new templating form doesn’t mean your claim no longer holds.

Agreed. It "doesn't mean (my) claim no longer holds". In fact, it reinforces it. This "new templating form" with its myriad trade-offs is simply not needed. The existing templating form (prototypes) is more than sufficient for the same purpose. On top of exacerbating the risk of unintentional structural damage, it increases memory usage in scenarios that would otherwise only require the memory occupied by the prototype. The up side of fields (automatically avoiding a well known foot gun) isn't worth the additional costs, IMO. But maybe that's just me.

I was using your “inherited data” terminology; i don’t consider functions data, so pretend i said “inherited values” or something equally generic.

Regardless of the terminology used, it boils down to this: a reified "field" is a property. Prototypes can hold properties. Instance objects use copy-on-write semantics to modify properties of their prototypes. So the trade-offs induced by trying to avoid a scenario that few non-novice developers fall into without intention, and excessive amounts of advice on how to avoid exists for, simply aren't worth it. Instead, a new kind of problem is being created that even experienced developers will end up falling into. Isn't the inconsistency caused by creating such new issues one of the reasons why this proposal won't fix the Proxy issue? Using your argument, instead of trying to fix the problem for this one-off case, wouldn't you rather see a generic fix to the object-on-prototype foot gun?

rdking commented 4 years ago

@bakkot

Constructor code can always use Object.defineProperty to avoid triggering setters in the base class. Nothing is violated here either.

(Not just that, but the existing pattern of

class Base {
  constructor() {
    this.foo = 0;
  }
}
class Derived extends Base {
  foo() {}
}

has exactly the same super-overrides-sub behavior.)

The difference here is that the developer is responsible for doing so directly. The developer knows that they just set something that may interfere with derived classes. This is one of the reasons I'm fully against class having anything inheritable not exist on the prototype. The class metaphor doesn't work properly unless inheritable content is contained in a hierarchal template. Instance properties disturb that.

littledan commented 4 years ago

It's pretty clear from experience with various object systems in JavaScript that properties on the prototype cause bugs, and properties on the instance is the actually desired behavior. @bakkot and @ljharb describe this well above. The proposal will not change here, so I'm closing this thread.

a-ejs commented 4 years ago

Properties unexpectedly going onto instances can also cause bugs, and I don't understand what you mean by 'desired behavior'. Desired by whom? There are scenarios where extending the prototype with non-function members would be beneficial, so it really depends on the situation, both options have their use cases. But the point I am (was) making is that instance properties can already be defined with nice and concise syntax in the constructor, whereas prototype properties have to be added the old-fashioned way, at which point why even use the new class syntax at all (looks ugly either way).

But I get it. Because some people might not understand how objects in JS work, we're not gonna see syntax that would allow such things.

However, this doesn't cancel the numerous downsides that current instance-based implementation has. It can also cause bugs, and/or what's more likely performance issues. It can also be confusing, and is just inconsistent in multiple ways with other things in the language. (which is pretty funny considering this comment on a different issue:)

I think consistency with the rest of the language is pretty valuable.

If the syntax has the downside of being unintuitive regardless of its meaning (instance or prototype fields), maybe it isn't needed in the language at all. Because after all, the cost paid is huge for little, if any at all, benefit that it provides in its current shape.

Should I create a separate issue about that?

rdking commented 4 years ago

@littledan

The proposal will not change here, so I'm closing this thread.

Given that this same issue has been raised several times both before the proposal reached stage 3 and after its implementations, but the response from TC39 has been so soundly resolute, this is not surprising.

It's pretty clear from experience with various object systems in JavaScript that properties on the prototype cause bugs, and properties on the instance is the actually desired behavior.

It's pretty clear from experience that most developers already know several ways to not encounter this issue unless they want it to happen. It's also pretty clear from all the times this exact same issue has been raised (in many different incarnations) that "fields" cause bugs. In both cases, it comes down to the expectations of the developer. In truth, there isn't that high of a risk that developer will make this particular bug unless they are completely green. The knowledge required to avoid the problem is already in existence and widely known by the vast majority of developers.

On the other hand, the problems with "fields" is still not widely known yet. Compared to the 1 issue for data on the prototype, there are multiple issues for "fields":

So honestly, this argument isn't compelling at all. That's why people keep trying to sway your opinions. Please recognize that if your intent is to have the community readily accept this proposal and not resent TC39 for essentially shoving something unwanted onto us, you're going to need a better justification.

littledan commented 4 years ago

Note, fields on the instance rather than the prototype matches what people are using with TypeScript and Babel. We have tons of real-world experience with the fact that this is a sensible pattern that works well. By contrast, we also have experience with various systems putting fields on the prototype, and the bugs they cause. There are other aspects of this proposal which are somehow debatable/value judgements, but I feel very strongly that non-static fields must be on the instance, not the prototype.

rdking commented 4 years ago

Note, fields on the instance rather than the prototype matches what people are using with TypeScript and Babel. We have tons of real-world experience with the fact that this is a sensible pattern that works well.

I never argued that it wasn't. My only argument is that it also causes bugs, especially in scenarios involving moderate to deep inheritance hierarchies. I would wager that this is far less well known than the prototype problem because people haven't been doing it for nearly as long, and are probably keeping inheritance hierarchies shallow (if they have any at all). When this proposal hits stage 4, that will likely change relatively quickly.

By contrast, we also have experience with various systems putting fields on the prototype, and the bugs they cause.

"Fields on the prototype" don't cause bugs. Initializing prototype property values to an object without re-initializing in the constructor, and then modifying a sub-property of that property is what causes the bugs. That's a very distinct, well known path. Any deviation from that path and no bug is ever caused.

I feel very strongly that non-static fields must be on the instance, not the prototype.

I understand this, which is why I don't generally try to argue with the goal of convincing you to change your stance, only the arguments you use to support your stance. At every turn, the severity of the issue you've sought to avoid has been misrepresented towards the extreme, while the severity of the approach you advocate has been misrepresented towards the mundane. Both approaches have their dangers, and those dangers can be ameliorated via a combination of a reasonable understanding of the problems involved, and appropriate adjustments to the logic.

Note: The reason I feel very strongly that non-static fields must be on the prototype is about what it takes to solve the issue in either case:

I would much rather move 1 line of code than give up 2 useful language features.

littledan commented 4 years ago

I think classes with instance fields are fairly popular in TypeScript and Babel. This proposal goes from Set to Define, but being on the instance rather than the prototype is common between them all.

rdking commented 4 years ago

I understand the popularity of instance fields as well as where it comes from. Given that ES6 class didn't provide a means of defining data elements, there was really only 1 approach that seemed easy: set it in the constructor. That line of thinking was preserved during the early implementations of this proposal in Babel as well as in TypeScript. Given that this approach has been around for some time, there's no wondering about its popularity.

However popular it has become, there's still no changing the fact that it causes problems. The reason so few people have seen these problems is that around the same time, FP started gaining popularity, and the notion that composition should be preferred over inheritance predated it. Between those 2 concepts was the perfect environment in which to grow "fields". Using composition instead of inheritance means the 2 biggest issues with "fields" becomes moot.

That being said, just because an approach has become popular doesn't imply that it's the best approach. Popularity can be a fleeting thing. Think about the popularity of OOP first in the past, and now in the presence of the resurgence of FP. But which approach is better? Doesn't that depend on what you're trying to accomplish?

I said all of that to say this. The purpose of a class in general is to create a rubber stamp with which to create objects, especially ones that benefit from the existence of other rubber stamps. Barring the set/define issue, if any feature of a class makes it difficult for new rubber stamps to be created that extend old ones, then that feature is a problem for classes in general. Where the correction to the prototype problem is just a matter of when to initialize, the problem with fields actually puts hard limits on what you can do with the language that didn't previously exist.


I know you won't be convinced by my argument, but I am willing to use this style of arguing to help you find one that may be more acceptable than what you've been relying on thus far.