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

Fields not applied to prototype #123

Closed
rdking opened this issue Aug 16, 2018 · 50 comments
Closed

Fields not applied to prototype #123

rdking opened this issue Aug 16, 2018 · 50 comments

Comments

@rdking
Copy link

rdking commented Aug 16, 2018

I just finished looking over proposal-decorators. It reminds me of Java Annotations, which I have mixed feelings about, but I can see it as a very useful way of solving the mixin issue. What I noticed, however, was that in the example for @prototypeProp, by default, fields will be applied directly to instances. This makes no sense. Not only does this immediately conflict with the point of having a constructor function, it also conflicts with the fact that class is meant to build a prototype out of the description it is given. There is very literally nothing to gain by sidestepping the prototype inheritance mechanism to inject values directly on an instance without the assistance of the constructor.

The class keyword is meant to produce a prototype and, if the prototype doesn't contain a constructor function, a default constructor. I would assume this is being done this way because the current design directly places private fields in slots of an instance just before calling the constructor. This too is not a good idea. The design and management of private fields should be congruent with the design and management of public fields. This will reduce the risk of developers encountering an unexpected outcome.

@bakkot
Copy link
Contributor

bakkot commented Aug 16, 2018

See tc39/proposal-class-public-fields#59 and tc39/proposal-class-public-fields#38.


Summarizing:

class Example {
  field = [];
}
let a = new Example();
a.field.push('foo');

let b = new Example();
console.log(b.field.length); // 0 or 1?

In my experience, people are very very surprised if the last line is 1.

Compare Java:

class Example {
  LinkedList<String> field = new LinkedList<>();
}
Example a = new Example();
a.field.add("foo");

Example b = new Example();
System.out.println(b.field.size()); // 0

or C++:

class Example {
  public:
    list<string> field;
};
Example a;
a.field.push_back("foo");

Example b;
cout << b.field.size(); // 0

or for that matter the behavior of TypeScript and Babel for several years now.

@jridgewell
Copy link
Member

Strongly agree with @bakkot. The behavior you're asking for here was the source of countless bugs in Backbone.

@rdking
Copy link
Author

rdking commented Aug 16, 2018

Well, I certainly cannot disagree with that reasoning. Is it possible to put the original on the prototype, but copy it onto the instance at instantiation? Hoping for something like this:

class Example {
  list =[];
}

Example.prototype.list; // returns [];
let a = new Example();
let b = new Example();

a.list.push("foo");
a.list; // returns ["foo"];
b.list; // returns [];

@ljharb
Copy link
Member

ljharb commented Aug 16, 2018

That would run the initializer an extra time for the prototype, which imo would be very surprising.

@rdking
Copy link
Author

rdking commented Aug 16, 2018

At the same time, it's surprising if any public member of the class is not present on the prototype.

@ljharb
Copy link
Member

ljharb commented Aug 16, 2018

I don’t see why; prototypes are typically for functions, own properties are for everything else.

@rdking
Copy link
Author

rdking commented Aug 16, 2018

The typical use case is not the only use case. Otherwise, people wouldn't be writing code like

class Example {...}
Example.prototype.foo = "something";

which is an unfortunate reality I've seen too many times.

@ljharb
Copy link
Member

ljharb commented Aug 16, 2018

Then it’s a good thing this new and attractive syntax feature won’t make it easier for people to write unfortunate and surprising code.

@rdking
Copy link
Author

rdking commented Aug 16, 2018

True. However, even if the value is left "undefined", it should still exist as a member of the prototype. The value can continue to be applied to the instance as presently defined.

@ljharb
Copy link
Member

ljharb commented Aug 16, 2018

Can you explain “should”, which is not an intuition i share?

@rdking
Copy link
Author

rdking commented Aug 16, 2018

@ljharb Here's an argument:

Do you remember how you felt about the behavior of "protected" under my proposal? You found it surprising for properties to appear in an instance for which the class definition contained no declaration. Likewise, for constructor functions (doesn't need to be a class), the members that appear are either explicitly placed there by the constructor function or inherited from the prototype. Anything else would be "surprising". class is thought of by the community as a constructor factory keyword that produces both a constructor function and a prototype. The constructor is known to be either a default (does nothing but call super() if necessary) or the one written by the developer. So if it's not in the developer-provided constructor function and not on the prototype chain, it's not a public field on an instance.

@rdking
Copy link
Author

rdking commented Aug 16, 2018

There are also those who will have the perfectly reasonable (and true since the beginning) expectation that for any obj.x, deleting the own instance property x will cause obj.x to return the value from the prototype, not always undefined as is currently the case under this proposal.

@ljharb
Copy link
Member

ljharb commented Aug 16, 2018

Once you learn that "class fields go on the instance", do you think it would still be surprising?

deleting the own instance property x will cause obj.x to return the value from the prototype, not always undefined as is currently the case under this proposal.

Based on your own suggestion:

even if the value is left "undefined", it should still exist as a member of the prototype.

then "always undefined" would indeed be the value from the prototype.

@rdking
Copy link
Author

rdking commented Aug 16, 2018

I'm not saying that developers wouldn't learn it. However, I'm saying it will be in the following year's JS WTFs. The "suggestion" was purely meant to get you to ignore the value for a minute and focus on the existence of the property. I'm not suggesting that public property values ever be undefined unless set so by the developer. I'm just saying that the developer has a reasonable expectation that the default value of any public field will be present on the prototype if it isn't being set by a developer-defined constructor function. I'm also saying that the user has a right to expect that even after the class has been defined, altering the value of a public member not defined by the developer-defined constructor can be done by altering the prototype, and that change will affect all new instances as well as existing instances that have deleted the own property.

This is just how the language has always worked for public member fields. Changing this just for this proposal illogical given that it is possible to avoid the "surprise" simply by keeping the original value on the prototype.

@ljharb
Copy link
Member

ljharb commented Aug 16, 2018

I don't think most developers would be likely to ever inspect the prototype directly or care what's defined on it.

Conceptually, to me, class fields are part of the constructor - and so the thing you're worrying about isn't being changed.

@bakkot
Copy link
Contributor

bakkot commented Aug 16, 2018

I'm not saying that developers wouldn't learn it. However, I'm saying it will be in the following year's JS WTFs.

I believe you are mistaken. People have been making widespread use of public fields through TypeScript and babel for several years without apparent issue.

@rdking
Copy link
Author

rdking commented Aug 16, 2018

@bakkot Don't the Typescript and babel transpilers simply rewrite the constructor (or write a new one) to include the necessary assignments? That's not nearly the same as what the proposal is doing. The only thing that's similar is the end result. Strawman argument.

@ljharb Sadly, once again there's a slight disconnect between your view of the situation and objective reality. True, most developers won't care. However, most developers tend to use the libraries provided by the few who will care. Therefore, even if most developers don't care, they'll still be affected by it.

Here's a parallel argument to what you just said: I don't think most chefs would be likely to ever inspect the calibration of the thermometer of their walk in freezer directly or care if its 20F too cold. Even when plates start being returned to the kitchen due to bad taste caused by unexpected instances of freezer burn, they won't think to inspect it. They'll just complain to the freezer manufacturer who'll send out a technician that will inspect it. Either way, the result is the same. It is important.

@rdking
Copy link
Author

rdking commented Aug 16, 2018

@ljharb Conceptually to me, class fields are part of the "class", i.e. the template used to create instances, a.k.a. the prototype. Hence the reason for my worry.

@bakkot
Copy link
Contributor

bakkot commented Aug 16, 2018

@rdking

Don't the Typescript and babel transpilers simply rewrite the constructor (or write a new one) to include the necessary assignments? That's not nearly the same as what the proposal is doing. The only thing that's similar is the end result. Strawman argument.

Are you just referring to the mechanism that compilers happen to use to implement this feature? I don't understand the relevance.

From the point of view of the users, they get the same syntax and same observable semantics as this proposal, modulo minor differences like Define vs Set. The fact that they are not generally surprised by this proposal's semantics for this proposal's syntax seems like a fairly strong signal to me.

@rdking
Copy link
Author

rdking commented Aug 17, 2018

@bakkot I get where you're coming from, but it still rubs me as a bad implementation approach. At least tell me this: What happens if someone alters the prototype of a class and adds a different value for a declared public field? Under this proposal, it fails to do anything noticeable since the value in the class declaration is written directly to the instance as an own property.

I can almost hear a rebuttal coming containing "shouldn't" and/or "once they get used to" or something of that nature. And again, I find myself inclined to agree with that kind of assessment. However, I still don't find it logical or prudent to unnecessarily limit an existing and used capability of the language like this when not limiting has such a low cost.

Let me raise a question. Is it true that only the functions present on the prototype(non-static) or on the constructor(static) that were in the class declaration will have access to private fields of a class instance? Note: I'm not worried about nested functions and nested classes since they will inherit access according to the function scope they're create in.

@ljharb
Copy link
Member

ljharb commented Aug 17, 2018

@rdking since public fields are analogous to things assigned in the constructor, it'd be the same thing that happens there - the own instance property would shadow the prototype one, and nothing would break. I would argue that that is exactly what most people will expect, since that's already how it works.

Only code written lexically within a class body will have access to the private fields declared in that class body.

@rdking
Copy link
Author

rdking commented Aug 17, 2018

BTW, the relevance of the mechanism is visibility. When the prototype owns the public members of a factory, the initial conditions of the factory can be mutated and affect all past and future instances. When the constructor creates the public members on the instance, it's effectively a static initialization on that instance with no possible preemptive mutation. If the initial conditions are not visible in either location, naive developers and/or existing logic may incorrectly presume that a particular field will not be part of an object.

@rdking
Copy link
Author

rdking commented Aug 17, 2018

@ljharb So even if someone does this, it should be fine?

Update: caught a few silly mistakes.
@ljharb caught another one.

class BadExample {
  #counter = 0;
  Utility = class Utility {
    #inst;
    constructor(obj) {
      this.#inst = obj;
      ++obj.#counter;
    } 
    get totalInstances() {
      return this.#inst.#counter;
    }
  };
}

var a = new BadExample();
var b = new a.Utility(a);
b.totalInstances; // expect to return 1

@ljharb
Copy link
Member

ljharb commented Aug 17, 2018

@rdking the prototype never fully owns the public members of an instance - that's not how JS works. The instance always gets to override it, and public fields are intended to override it.

If someone writes code like that - that to me looks entirely inscrutable - then I don't think there should be a reasonable intuition about how it will work. That said, I would expect the Utility constructor to throw an exception, because you constructed a.Utility with undefined, and undefined doesn't have a #counter field. If you intended to do new a.Utility(a), then I would indeed expect b.totalInstances to return 1.

@loganfsmyth
Copy link

As another viewpoint here, in my time helping people using class fields via Babel, there's only one prototype-related case that comes to mind that I've seen trip people up, in several instances.

class Base {
  someMethod = () => {
    return 4;
  };
}
class Child extends Base {
  someMethod = () => {
    return super.someMethod() + 1;
  };
}

where super.someMethod in this context is always undefined because someMethod is not on Base.prototype.

That said, I've found that once people stop to remember that class fields are not on the prototype, they pretty much understand. It also doesn't seem like there's any useful way for someMethod could be on the prototype in this context, because there'd be no way to bind someMethod to the correct instance.

@rdking
Copy link
Author

rdking commented Aug 17, 2018

@ljharb You're right, I didn't write clearly enough. What I meant to say is that the "...prototype owns the public template of a factory...." meaning that the prototype contains the fields and values that can be reliably expected to be found on any constructed object, even for the constructor. In fact, this is the only way to guarantee that a particular field and value are present in an instance before super() is called on a constructed instance when there is a need to alter a default expected by a base constructor but not passed via arguments.

Edge case? Yes. Useful? Still Yes.

As for the example: yet another mistake.... Rough week... I've corrected it above.

Anyway, I'm having trouble making sense of how that works. Utility is a function, but BadExample.prototype.Utility doesn't exist. If all public fields are written to instances during construction, then this happens:

new BadExample.prototype.Utility(new BadExample()); //TypeError: Utility is not a constructor
var c = new BadExample();
c.Utility === a.Utility; // false

I think this will be supremely counterintuitive for most developers.

@nicolo-ribaudo
Copy link
Member

nicolo-ribaudo commented Aug 17, 2018

Would you find it more surprising if the first example logs ["foo"] or if the second logs false? We can't have the first log [] and the second log true, because they are opposite behaviors.

class Example {
  list = [];
}

var a = new Example;
var b = new Example;

a.list.push("foo");
console.log(b.list); // Should log []?
class Example {
  Utility = class {};
}

var a = new Example;
var b = new Example;

console.log(a.Utility === b.Utility); // Should log true?

@rdking
Copy link
Author

rdking commented Aug 17, 2018

@loganfsmyth For the example you wrote, you're right. There's no way to bind someMethod to the correct instance... or for that matter, any instance. I would expect that those arrow functions, even though they're in the lexical scope of their respective classes, would have the same this as the 2 class declarations. So super.someMethod() should throw unless these declarations occurred inside a method of a descendant class. That would be the same whether or not someMethod appeared on the prototype.

Try this:

a = { test: () => { console.log(`test exists? ${this.hasOwnProperty("test")}`); } }

That's kinda just how it is.

@loganfsmyth
Copy link

@rdking

There's no way to bind someMethod to the correct instance... or for that matter, any instance.

That is not the current semantics of arrow functions within class fields. The same way class fields are initialized during construction, their this and super bindings access the current instance e.g.

class Foo {
  prop = this;
  constructor(){
    console.log(this.prop === this);
  }
}
new Foo();

would log true.

This feature is used extremely heavily.

@rdking
Copy link
Author

rdking commented Aug 17, 2018

@nicolo-ribaudo I'd find both to be extremely unpleasant. That's why I present the solution of copying the non-function, non-primitive, public items off of the prototype and onto the new instance during construction.

@loganfsmyth So are you saying that the constructor in your example is an arrow function? If so, then you've lost me since arrow functions have different semantics. If you're saying arrow function semantics are being altered for the class-fields proposal, then I am both relieved and highly disturbed. If the arrow function semantics are being altered to match that of method declarations, then your previous post is incorrect, and applying arrow functions to the prototype will be no different than they are for method declarations.

So what did I miss?

@loganfsmyth
Copy link

loganfsmyth commented Aug 17, 2018

@rdking

So are you saying that the constructor in your example is an arrow function? If so, then you've lost me since arrow functions have different semantics

I'll clarify. My point there is that, with an example like

class Foo {
  prop = this;
  constructor(){
    console.log(this.prop === this);
  }
}

in the current spec it is equivalent to

class Foo {
  constructor(){
    this.prop = this;
    console.log(this.prop === this);
  }
}

and following that same example

class Foo {
  method = () => this;
  constructor(){
    console.log(this.method() === this);
  }
}

is equivalent to

class Foo {
  constructor(){
    this.method = () => this;
    console.log(this.method() === this);
  }
}

Note how the this inside of the arrow function accesses the current instance.

If you're saying arrow function semantics are being altered for the class-fields proposal

It's not an alteration of how arrow function semantics, but it is the addition of a scope around the contents of class field initializers. Everything after the prop = or method = is executed in a scope where this is the current instance.

@rdking
Copy link
Author

rdking commented Aug 17, 2018

@loganfsmyth Thanks for the clarification. However, as I expected, that leads to the non-intuitive nested class declaration problem I pointed out above. I'm offering that

class Foo {
  prop = { fu: "bar" };
  constructor(){
    console.log(this.prop.fu === "bar");
  }
}

be equivalent to this instead.

class Foo {
  constructor() {
    if (Foo.prototype && (typeof (Foo.prototype) == "object") && 
      Foo.prototype.hasOwnProperty(Symbol.ownFields)) {
      let proto = Foo.prototype;
      let fields = proto[Symbol.ownFields]();
      for (let field of fields) {
        let def = Object.getOwnPropertyDescriptor(proto, field);
        if ("value" in def) {
          def.value = deep_clone(def.value);
          Object.defineProperty(this, field, def);
      }
    }
    console.log(this.prop === this);
  }
}
Foo.prototype.prop = { fu: "bar" };
Foo.prototype[Symbol.ownFields] = () => [ prop ]; 

With this approach functions and primitives would never be in the ownFields array. There's no need to copy them. Also, it's completely non-intuitive to see the "this" keyword inside a declaration. That particular construct should be illegal simply for the fact that "this" is not available at the time of construction. Or rather, that should run conflict with the "this" value of the current context.

There's another problem. Doesn't moving the execution of non-function field declarations into the constructor prevent class declarations from making use of values in the containing scope when they are shadowed in the constructor?

var someval = 49152;

class Example extends SomeOtherIrrelevantClass {
  publicField = someval; //What does this get set to?
  constructor() {
    var someval = -16384;
    SomeOtherIrrelevantClass.mustDoThisBeforeInstanceInit(someval);
    super();
    /* main init logic here */
  }
}

So will (new Example).publicField === 49152 or -16384?

@ljharb
Copy link
Member

ljharb commented Aug 17, 2018

It would of course be 49152, because of lexical scoping. this inside the class body is the class instance, and many people have found that perfectly intuitive after years of using the syntax in babel.

@nicolo-ribaudo
Copy link
Member

@nicolo-ribaudo I'd find both to be extremely unpleasant. That's why I present the solution of copying the non-function, non-primitive, public items off of the prototype and onto the new instance during construction.

How would you clone them? Finding a deep_clone function which works in every case is probably impossible in JS (https://twitter.com/awbjs/status/1018166939598770177)

@rdking
Copy link
Author

rdking commented Aug 19, 2018

@ljharb People using future features in babel should be aware that they're using experimental features of a non-finalized spec. So I don't consider things done in babel to be any point of reference. Anyone will get used to almost anything if they have to do it long enough, so that's not a good argument. As for your answer, that answer is good and is what I would hope would happen but makes no sense: a definite "WTF!?". Think about it. The equivalent code is this:

var someval = 49152;

class Example extends SomeOtherIrrelevantClass {
  constructor() {
    var someval = -16384;
    SomeOtherIrrelevantClass.mustDoThisBeforeInstanceInit(someval);
    super();
    this.publicField = someval; //global value assigned here?
    /* main init logic here */
  }
}

At that point, there's a local someval shadowing the global one. Logically, the value should be -16384, but that's not what the developer would expect. You probably don't yet see why this is bad. Since you said that the value from the containing scope would be used, what would happen if we used @loganfsmyth's Foo?

class Example  {
  confuse() {
    return new (class Foo {
      prop = this; //Which "this" is this?
      constructor(){
        console.log(this.prop === this);
      }
    });
  }
}

(new Example).confuse(); //true or false?

See the problem? If you're right and the outer scope is respected, then we get false. Probably not what the developer expected. If, however, the inner scope is respected, then we get true, but that makes for a contradiction. If the inner scope is respected, then outer values can get clobbered, leaving the developer with a bug. If the outer scope is respected, then a non-nested Foo would not be able to assign this to prop since class is strict by nature and this would not be defined. Like @nicolo-ribaudo stated before, "these are opposite behaviors."

@nicolo-ribaudo How to clone them? Yeah, that's a tough problem, even from inside the engine, so I'm giving up on that one. If after so many years of JS, developers still don't understand that actions on objects on a prototype affect the prototype object without copying it, then the language is already in trouble. Catering to ignorance of how the language works is not a good thing. If something must be done, then what I'd rather have in its place is disallowing explicit object initialization for non-static fields. I just can't bring myself to get behind magic transportation of code.

If I define a field in a class and apply an object to it, I should be aware of the fact that any manipulation of that object affects all instance of the class. If that's not what I want, then I need to initialize the field in the constructor. That's what the constructor is for! We already have to be aware of that fact when using things like Object.assign, the lodash library, etc. So either leave the definitions in place where they are and let the user suffer for not being aware that they're manipulating the prototype, or simply disallow non-function, non-null object initialization on class fields. I'd prefer the former, because that's more consistent with the behavior I've come to expect from ES.

@bakkot
Copy link
Contributor

bakkot commented Aug 19, 2018

Stepping back a little: The committee considers the experience of people using Babel to be fairly strong evidence that developers will not be unduly surprised by a given design, and separately that the current design is well-motivated and ergonomic. I'm sorry your intuition differs, but inevitably someone's will; I don't know that there's that that much value in rehashing these cases.

@rdking
Copy link
Author

rdking commented Aug 20, 2018

If the committee's opinion cannot be swayed, then there's no point in this proposal remaining in stage 3. It seems it's a done deal despite all its short-comings and outright flaws. What I've pointed out above is an outright flaw. I admit that my original suggestion was untenable, but the problem itself is equally so.

That being said, if I were to implement my proposal in babel as another proof of concept, would that be of any merit or a waste of time?

@jhpratt
Copy link

jhpratt commented Aug 20, 2018

@rdking Per TC39's process document, stage 3 indicates that the spec is

Complete: all semantics, syntax and API are completed described

and post-acceptance changes are

Limited: only those deemed critical based on implementation experience

I'm not sure why you think the committee's opinion can't be swayed, as they have pulled a stage 3 spec before. You say that what you've pointed out is an "outright flaw". Many people, including myself, disagree. Regardless, the process document clearly indicates what's acceptable for changes to a stage 3 spec.

@bakkot
Copy link
Contributor

bakkot commented Aug 20, 2018

If the committee's opinion cannot be swayed

It can, of course, even at this late stage, although at this point we only expect changes which are quote "those deemed critical based on implementation experience". But it's always possible we missed something and would unexpectedly have to revisit things.

But we are extremely unlikely to change our minds on the basis of arguments we've already considered. The decision to provide blessed syntax for installing fields on instances in addition to the existing syntax for installing methods on prototypes, and crucially not provide blessed syntax for installing fields on prototypes, was not made on a whim. It was a deliberate decision made for reasons outlined above, after consideration of situations including those you've raised. The fact that at least one people finds it surprising or unintuitive is not new information to us. Nor is anything else in this thread, as far as I can tell.

I'm sorry you can't get behind the design of this proposal, sincerely. I wish we could find something which would satisfy everyone. But I don't think we can, and I continue to believe this design is the best we are likely to have.

That being said, if I were to implement my proposal in babel as another proof of concept, would that be of any merit or a waste of time?

If you implemented it in babel and it saw significant adoption with most users finding your design more useful or less surprising than the current one, that would be valuable information. But I don't expect that to happen - even if your design were objectively better, it's hard to get adoption for new things.

I don't think it would be useful as merely a demonstration that it could be done, though, no. Very rarely is "would this be possible to implement" the critical constraint on a design.

@rdking
Copy link
Author

rdking commented Aug 20, 2018

Well, I'm not worried about the "if it can be done" as I've already proven the concept to myself through the POC I created in my proposal. As for whether or not it would be adopted, somehow I feel I would have had a fighting chance if I had done so around the same time as it was done for the current proposal.

This late in the game is very likely indeed to be effectively too late. This is a sad reality to face. It's not so much that I think everyone would get behind my design, but rather that my design poses less overall disruption to the existing capabilities of ES while at the same time offering 100% of the same features the current proposal has along side additional features that are natural extensions of the design.

That's really the gist for me. The committee is plodding forward on a design that has not yet been fully accepted, yet isn't really open to reviewing alternate approaches. I get that there's a point where you need to cut your losses, but despite the overall adoption via babel, the approach of the current proposal does more harm than is necessary to the language as a whole.

I think I just might write that babel plugin, even if I don't think anyone will adopt it. A year ago in a conversation with @littledan, he said something to the effect of "the viable solution space for this problem is so small that it is highly unlikely that a better solution will come along". He was right. However, I can't help but wonder how or even if a better solution would be recognized should it come along late in the game.

@rdking rdking closed this as completed Aug 20, 2018
@rdking
Copy link
Author

rdking commented Aug 20, 2018

@jhpratt I'm no longer arguing the point. However, I'm still curious. If what I pointed out is not a logical flaw, then what was the answer to the question? Here it is again:

class Example  {
  confuse() {
    return new (class Foo {
      prop = this; //Which "this" is this?
      constructor(){
        console.log(this.prop === this);
      }
    });
  }
}

(new Example).confuse(); //true or false?

@rdking rdking reopened this Aug 20, 2018
@ljharb
Copy link
Member

ljharb commented Aug 20, 2018

It’s true, full stop. The Foo class fields have the same receiver as Foo’s constructor.

@rdking
Copy link
Author

rdking commented Aug 20, 2018

So then the correct answer above is the non-intuitive -16384? If that's not the case then there is an inconsistency.

@rdking
Copy link
Author

rdking commented Aug 20, 2018

Maybe I should ask it this way:
Will it always be that the public fields are evaluated as if they had been written in after the super() call? If not, then what are the rules for switching?

@ljharb
Copy link
Member

ljharb commented Aug 20, 2018

Yes, except as it relates to the lexical scoping of constructor arguments and variables defined in the constructor (it would be exceedingly weird if a public field, written outside the constructor, had access to any of the variables defined by the constructor - this not being one of them)

@rdking
Copy link
Author

rdking commented Aug 20, 2018

Agreed, and yet that's precisely what's odd here. The field declarations have been transported inside the constructor. If the assignment part is resolved first (which you've just verified is the case), then the "this" that may exist in those assignments should not be the this of the constructor simply for the fact that the value was resolved before the assignment was transported.

Oh well. It's not like I don't understand the reason for this intentional inconsistency, but it is an inconsistency, a logical flaw. Maybe it won't end up in the WTFs because people will understand why it was done.

@rdking rdking closed this as completed Aug 20, 2018
@rdking rdking reopened this Aug 20, 2018
@rdking
Copy link
Author

rdking commented Aug 20, 2018

Wait, now I'm confused again. You just verified that the assignment part of the property declaration is, with the exception of things involving this, resolved before being transported. Is that true for all declarations? If the property is assigned an object, then that would have to be resolved after transporting to ensure that instances don't share a single copy of the declared object. So then, is the logic something like this pseudocode?

  let assignment = declaration.assignment;
  if ((assignment.type != "Identifier") || (assignment.identifier.name != "this"))
    assignment.value = resolve(assignment.value);
  
  transport(declaration);

@nicolo-ribaudo
Copy link
Member

The analogy of moving class fileds to the constructor, as you pointed out, doesn't always work because of variables scoping. You can thing at them like this:

var myVar = 3;

class A {
  foo = {};
  bar = this;
  baz = myVar;

  constructor() {
    var myVar = 4;
  }
}

// Becomes
var myVar = 3;

class A {
  constructor() {
    this.#initializeInstanceFields(); // <- In derived classes this is after super()
    var myVar = 4;
  }

  #initializeInstanceFields() {
    this.foo = {};
    this.bar = this;
    this.baz = myVar;
  }
}

this is scoped to the class, so everything inside class { ... } can access it. Constructor variables are scoped inside the constructor, so they can only be accessed by code inside constructor() { ... }. This is how scoping has always worked: you can access variables only from the same block of from outer blocks.

@rdking
Copy link
Author

rdking commented Aug 20, 2018

Ok. That's far clearer, and very similar to one of the field implementations I created. Just saying "initialized in the constructor" without the "by a separate function" is what was causing the unacceptable misunderstanding. If this is the case for all non-static private and public fields equally, then I can handle that. I'll just need to remember that the field declarations are essentially an automatic function, and that class is essentially generating 2 functions now.

@rdking rdking closed this as completed Aug 20, 2018
@rdking rdking reopened this Aug 20, 2018
@rdking rdking closed this as completed Aug 20, 2018
@bakkot
Copy link
Contributor

bakkot commented Aug 20, 2018

If this is the case for all non-static private and public fields equally, then I can handle that.

It is, yeah.

In the interests of total transparency, there's one other potentially confusing caveat here, which is that referring to arguments in a class field initializer is a syntax error. This cannot be understood by analogy to anything else. But I think it will avoid confusion; it means that you'll never have to read code that refers to arguments in an initializer and figure out what the author meant, and even if you try to refer to it it will just throw right away rather than working in a way you might not have expected.

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

7 participants