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

Member on instance vs prototype #59

Open
thepian opened this issue Feb 13, 2017 · 34 comments
Open

Member on instance vs prototype #59

thepian opened this issue Feb 13, 2017 · 34 comments

Comments

@thepian
Copy link

thepian commented Feb 13, 2017

TL;DR Don't allow class values without qualifiers. The notion is ambiguous and counter to classes just being syntactic sugar on top of the prototype chain. Options static, let, const would make sense.

Does this proposal cover both ways to set a value?

It can be very useful to set configuration constants on the prototype. They can be accessed in class methods. A nice power is the ability to override these on sub-classes. It can also be used to set a default value more efficiently than in the constructor.

Obviously the way most people are used to is the equivalent of setting the value in the constructor.

given:

class X {
    x = 5
   
    getX() { return this.x; }
}

many people may expect it to be equivalent to,

function X() { this.x = 5; }
X.prototype.getX = function() { return this.x; } 

however that means that the definitions are inconsistent in where they are set, one is on instance, the other is on the prototype. I think a reasonable argument is that the constructor is the place to set instance variables, and the class space is the place for members of the prototype.

function X() { }
X.prototype.x = 5;
X.prototype.getX = function() { return this.x; } 

I think one idea to explore could be to have const denote values set on the prototype and let denote values set on the instance. That would remove the ambiguity of the current notation. It also hints that those on the prototype can be overridden but not changed.

class Foo {
    const  bar = 5
    let baz = 10

    getBar() { return this.bar; }
}
@bakkot
Copy link
Contributor

bakkot commented Feb 13, 2017

See #38 and #16.

That said, I think we're fairly set on properties being installed on instances. In simple cases they end up being equivalent, and I'd argue that putting the properties on prototypes gives the more surprising semantics in cases where they differ.

In particular, I think many, many people would get tripped up by this:

class Foo {
  a = { prop: 0};
}

(new Foo).a.prop = 1;
(new Foo).a.prop === 1; // true

Nor is it clear to me why you'd want this. "They can be accessed in class methods and overridden in subclasses" is true when properties are installed on instances, too.

@ljharb
Copy link
Member

ljharb commented Feb 13, 2017

Mutating the prototype after-the-fact seems like a very bad practice, and it feels like this would remove all the utility of the proposal.

If you want this, you can already do it with a getter shadowing the superclass property - ie, an instance method like get x() { return 5; }.

@thepian
Copy link
Author

thepian commented Feb 17, 2017

Mutating the prototype after-the-fact seems like a very bad practice, and it feels like this would remove all the utility of the proposal.

Hence the suggestion of not supporting members without const/let qualifier.

class Foo {
  const a = { prop: 0};
}

If you try to modify foo.a.prop you shouldn't be surprised about odd behaviour.

@bakkot
Copy link
Contributor

bakkot commented Feb 17, 2017

If you try to modify foo.a.prop you shouldn't be surprised about odd behaviour.

But there's no reason to have that behavior. If members are installed on instances, as in every other language, foo.a.prop will behave exactly as people expect.

@thepian
Copy link
Author

thepian commented Feb 19, 2017

In case I'm not making myself clear I'd like to clarify my stance: "Prototypical inheritance allows setting properties on the instance or the prototype. Since classes are just syntactic sugar on top, they should support both these options as well."

But there's no reason to have that behavior. If members are installed on instances, as in every other language, foo.a.prop will behave exactly as people expect.

So the prototype is just a nasty history proven to be a bad idea and classical inheritance has been proven to be the right way to do OOP? Sorry, but I beg to differ, however you will probably find ample support of turning JS into Java. I'd argue that what we have learned is that inheritance is highly problematic, and we probably haven't found the right approach yet.

@ljharb
Copy link
Member

ljharb commented Feb 19, 2017

Putting primitives (and functions, ofc) on the prototype would be fine, but putting mutable objects (including arrays) on the prototype is a massive footgun, because developers often do not realize that they will be shared across all instances. Regardless of how you think things "should" be, this is how things are.

Prototypical inheritance (which is what class is) is still not the same as classical inheritance - this has nothing to do with that. Putting objects on prototypes is simply a bad part of prototypical inheritance - one that new syntax shouldn't knowingly encourage.

@thepian
Copy link
Author

thepian commented Feb 20, 2017

@ljharb Fine, I agree. I've been trying to respond to the comments. I didn't know that JavaScript was supposed to be opinionated on classical inheritance, I understood it to be syntax not a change in what the language is, but no matter, I would rather use the following example

class Renderer {
  const textLimit = 40

  renderTitle(title) {
    render(shorten(title, this.textLimit));
  }
}

class SmallScreenRenderer {
  const textLimit = 25
}

My second example would be how class decorators might be implemented in libraries like React.

function addIn(updateStrategyFunction ,component) {
  return class extends component {
     const updateStrategy = updateStrategyFunction
  }
}

This is a function that really makes best sense on the prototype, but since it is passed by value rather than hard-coded, setting it on the prototype with the current spec requires twice as many lines and is sure not to be well received by clean code proponents.

I think there is a conceptual problem with the proposal as it implies a fundamental difference between functions and other types (I understand that all assignments are put on the instance). Since there is no qualifier to the instance variables; When you read a class declaration variables and methods seem to belong in the same bag, but methods go on the prototype, and everything else goes on the prototype. With static there isn't this problem as they all go on the constructor, all good. Hence I proposed that the let keyword could be used to hint putting the variable on the instance.

@ljharb
Copy link
Member

ljharb commented Feb 21, 2017

If you're using addIn as a class decorator, you could probably also use a method decorator like so:

return class extends component {
  @replaceWithMethod(updateStrategyFunction)
  updateStrategy() { /* will be thrown out */ }
};

@Jamesernator
Copy link

I definitely wouldn't want const/let being used for determining prototype/instance methods that would very unclear, personally I'm quite a fan of what's proposed in orthongonal classes.

@chyzwar
Copy link

chyzwar commented Mar 19, 2017

class MyClass{
    prop = 2;
}

is equivalent to:

class MyClass{}
MyClass.prototype.prop = 2;

or:

class MyClass{
   constructor(){
       this.prop = 2;
    }
}

Can you confirm?

@loganfsmyth
Copy link

It is roughly equivalent to your second example

class MyClass{
   constructor(){
       this.prop = 2;
    }
}

@chyzwar
Copy link

chyzwar commented Mar 19, 2017

Then this proposal is about having an another way of creating instance properties?

class MyClass{
    prop = 2 // instance property 

    constructor(){
        this.prop = 2 //another instance property
    }
    static prop = 2 //<- function property
    ['prop'] = 2 //instance computed property 

    static get prop(){ return 2;} //getter function property
    get prop() {return 2} //getter instance property

}
MyClass.prototype.prop = 2 // prototype data property

Imo this proposal then just add syntactic sugar and make things more confusing.
We already have 3 ways to create instance properties, do we really need forth?
This is no way user friendly and people will still need to access prototype object to add data properties....

@bakkot
Copy link
Contributor

bakkot commented Mar 19, 2017

@chyzwar, it's only roughly equivalent; its name is evaluated at a different time, it may have slightly different semantics, and it is expected to be easier for engines to optimize. And being able to omit the constructor in some cases is a significant gain, probably worthy of the sugar.

A lot of people are already using something similar via babel with no apparent confusion, so I'm not worried on that front.

There's very little use case for adding data properties to the prototype. I don't think we want to encourage it with first-class syntax; I expect that would to a great deal of unnecessary confusion.

@chyzwar
Copy link

chyzwar commented Mar 19, 2017

@chyzwar, it's only roughly equivalent; its name is evaluated at a different time, it may have slightly different semantics, and it is expected to be easier for engines to optimize. And being able to omit the constructor in some cases is a significant gain, probably worthy of the sugar.

I do not see how this is any different for engines to optimise. In both cases constructor and this proposal V8 will create hidden class. If anything it will be now more difficult because there could be now two transition paths for the same property.

A lot of people are already using something similar via babel with no apparent confusion, so I'm not worried on that front.

People use static, I am not aware of transformation for functionality this proposal describe. Also babel should not be reference point for language specification. We should now include JSX?

class Octopus {
    readonly name: string;
    readonly numberOfLegs: number = 8;
    constructor (theName: string) {
        this.name = theName;
    }
}

Basically this proposal is the same as readonly in TypeScript. But without keyword, and limitation on initialisation.

There's very little use case for adding data properties to the prototype. I don't think we want to encourage it with first-class syntax; I expect that would to a great deal of unnecessary confusion.

But this is possible using prototype. If you do not want people mixing classes and prototypes it should be possible to express in class the same things. Currently classes are less powerful and people will keep fiddling with prototype.

For example:
https://github.com/facebook/react/search?utf8=%E2%9C%93&q=prototype.
https://github.com/angular/angular.js/search?p=3&q=prototype.&utf8=%E2%9C%93

Both angular.js and react.js set data properties on prototype. This make sense where you want to have one object shared in all instances of given class. This is valid use case and optimisation technique.

@bakkot
Copy link
Contributor

bakkot commented Mar 20, 2017

I do not see how this is any different for engines to optimise. In both cases constructor and this proposal V8 will create hidden class. If anything it will be now more difficult because there could be now two transition paths for the same property.

The impression I got when implementing this feature in V8 is that having objects be created with a static shape requires fewer hidden-class transitions, which is a nontrivial benefit to instantiation. This is also the impression I've gotten from people working on other engines.

People use static, I am not aware of transformation for functionality this proposal describe.

The class properties transform implements approximately this proposal.

Also babel should not be reference point for language specification.

Of course. But we're not using it as a reference point; rather, it's a useful indication of how confusing users will find a feature. Implementation experience and developer feedback is something we consider strongly when advancing proposals.

Basically this proposal is the same as readonly in TypeScript. But without keyword, and limitation on initialisation.

I don't know what you mean by that. These properties aren't readonly.

But this is possible using prototype. If you do not want people mixing classes and prototypes it should be possible to express in class the same things. Currently classes are less powerful and people will keep fiddling with prototype.

It's always going to be possible to modify prototypes, and we're not seeking to prevent that. What we're trying to introduce is a nice syntax for the overwhelming common case, with a minimum of user confusion.

@chyzwar
Copy link

chyzwar commented Mar 20, 2017

The impression I got when implementing this feature in V8 is that having objects be created with a static shape requires fewer hidden-class transitions, which is a nontrivial benefit to instantiation. This is also the impression I've gotten from people working on other engines.

There will be hidden class for each transition. You can get clarification from someone on V8 team.
http://richardartoul.github.io/jekyll/update/2015/04/26/hidden-classes.html

The class properties transform implements approximately this proposal.

This mix two proposal static properties and this proposal. Well I am not surprised, this is babel.

I don't know what you mean by that. These properties aren't readonly.

Typescript readonly mean that property need to be initialised on declaration or in constructor. This will be similar to your proposal:

class MyClass{
    const a = 2;
}
class MyClass{
    readonly a = 2
}

It's always going to be possible to modify prototypes, and we're not seeking to prevent that. What we're trying to introduce is a nice syntax for the overwhelming common case, with a minimum of user confusion.

Overwhelming common sense is to initialise instance variables in constructor. By providing alternative syntax you will make this more confusing. In general dependencies should be injected in constructor, by adding instance variable outside constructor you create a unexpected side effects to constructor.

@bakkot
Copy link
Contributor

bakkot commented Mar 20, 2017

There will be hidden class for each transition.

Exactly: and declaring the properties in the class body minimizes the number of transitions. Writing to a property which already exists does not cause a transition. When properties are added in the constructor, there's a transition for each property added. The more static shape provided by this proposal requires fewer transitions in the common case.

For what it's worth, I was on the V8 team.

This mix two proposal static properties and this proposal.

It's only one proposal. This proposal includes static properties. I do recommend reading the proposal.

Overwhelming common sense is to initialise instance variables in constructor.

I don't think I agree. In many cases it's clearer to initialize them in the class body, as is common in other languages. This is especially true because the body reads as declarative, rather than imperative.

@wmertens
Copy link

wmertens commented Mar 21, 2017 via email

@chyzwar
Copy link

chyzwar commented Mar 21, 2017

I don't think I agree. In many cases it's clearer to initialize them in the class body, as is common in other languages. This is especially true because the body reads as declarative, rather than imperative.

I disagree. Dependency injection exists for a reason. You will now have multiple places for instance initialisation. Worse that that, when your declarative stuff get executed before or after constructor? How user will know that?

What if is some smart ass do this:

class MyClass{
    constructor(b){
      this.b = b;
    }
    initialise(){
       if(this.b) {
            this.c = b -2 
            return b + 2
       }
       throw Error('b is not there')
    }
    a  = this.initialise()
}

It is no longer declarative. There are now hidden side effects.

class MyClass{
    a = this.construct()
    async construct(){
       return await someAsyc(); 
    }
}

Will now constructor be blocking? Object will not be initialised until async action complete?

class MyClass extends C{
     b = this.mixinHell()

     mixinHell(){
          D.call(this)
          E.call(this)
          F.call(this)
     }
     constructor(d){
         super()
         this.d = d
      }
}

function D(){
  this.d = 2
}
new MyClass(10)

What is value of this.d? (10 or 2)

class MyClass{
     a = new MyClass()
}

What now? a is another instance of MyClass. This will be finite loop?

It will be make more difficult for engines to optimise. In languages like Java these declarations are added by compiler into constructor. JavaScript engines will now need dynamically create constructor or create execution plan for initialisation.

@bakkot
Copy link
Contributor

bakkot commented Mar 21, 2017

Yes, of course it will be possible to use this proposal to write confusing, side-effecting code. This is JavaScript; that's already possible and is always going to be possible. I think it also makes it much easier to write code which clearly and correctly expresses intent.

It will be make more difficult for engines to optimise.

I am telling you, having worked on this in V8 and having talked to other engine authors about this specific issue, that we don't expect this to the case. Asserting otherwise will not make it so.

In any case, we've gotten pretty far off the topic of this issue. You're welcome to open a new issue to continue this subthread, although I should warn you I doubt you'll find us convinced, but probably we shouldn't continue it here.

@chyzwar
Copy link

chyzwar commented Mar 21, 2017

I am telling you, having worked on this in V8 and having talked to other engine authors about this specific issue, that we don't expect this to the case. Asserting otherwise will not make it so.

Engine will first need to scan class for public properties and insert them into some kind of hidden secondary constructor. When class is then initialised then both constructors will get invoked in order or one if engine already have hidden constructor. I am not saying that this is huge cost, but this is not going to be free.

class MyClass{
     a = 2
     constructor(){
         this.a  = 3
      }
     a = 4
}

What is initialisation order in this case?

@normanzb
Copy link

normanzb commented Mar 23, 2017

Agree with original poster @thepian and I'd like to say it is wrong at the beginning to make prototype inheritance to be called with class keyword.

As a user of this proposal, I can confirm that I don't find it confusing in
the least, and very handy to avoid writing a constructor. If there is a = in the definition line, it goes on the instance

And here goes more confusion to that:

  class MyClass {
    a = (state) => {
    };
    b(state) {
    }
  }

So which method is going to be on the prototype? or both?

Yes, of course it will be possible to use this proposal to write confusing, side-effecting code. This is JavaScript; that's already possible and is always going to be possible.

We all know there are a lot of confusing code already, then why couldn't we stop making more? Why not just stop doing it declaratively and leave us assign those properties in constructor, what is the downside of that could be? Upset a few people coming from class based background?

@ljharb
Copy link
Member

ljharb commented Mar 23, 2017

Obviously b is on the prototype and a is on the instance - I'm not sure how you'd assume anything else. Can you elaborate on what else you'd assume, and why?

@chyzwar
Copy link

chyzwar commented Mar 23, 2017

Because function declaration and function statement hoist on the same scope/context.

cons a = function() {}
function a(){}

@normanzb showcase how this will get confused by new people coming to language. Public property declaration are inconsistent with how we think about scope in JS. In static case this is less an issue because keyword indicate different scope.

class MyClass{
   a = 2; // on instance
   a(){} // on propotype
   static a(){} //on MyClass 
   get a(){} // pseudo-property on instance
   static get a() //pseudo-property on MyClass
}

This will lead to really confusing code when some people will not understand differences. Classes will become more arcane for no reason.

Funny enough people will still access prototype because current class syntax is not as much powerful as prototype.

@ljharb
Copy link
Member

ljharb commented Mar 23, 2017

That has the function keyword, which is the sole cause of the behavior you're describing. Class bodies lack that, thus, no confusion.

@chyzwar
Copy link

chyzwar commented Mar 23, 2017

That has the function keyword, which is the sole cause of the behavior you're describing. Class bodies lack that, thus, no confusion.

Class is sugar for function. You are saying that we will use different keyword and create a different semantics and it will be ok?

class body is a function prototype, you want to introduce something that will break this contract. static keyword is already making this messy.

@ljharb
Copy link
Member

ljharb commented Mar 23, 2017

class is sugar for a constructor function AND a prototype, not just for functions. So yes, a different keyword obviously has different semantics, that's the point of it.

constructor() {} and statics are in the class body, and not on a prototype, so your mental model here is incorrect.

@normanzb
Copy link

normanzb commented Mar 23, 2017

Obviously b is on the prototype and a is on the instance - I'm not sure how you'd assume anything else. Can you elaborate on what else you'd assume, and why?

Because visually the 2 methods look similarly declarative and the keyword class is deceivingly confusion.

It says it is a class, it looks like very classic inheritance. Even this proposal is trying to make it "looks" more like a classic one, however with the new feature introduce by the proposal, when you defined a method in the wrong way it going to introduce side effects, couldn't that be called confusion?

To anyone who has never thoroughly read the document they may very easily miss the difference between those 2, and the toll for missing the difference can be huge in a lot of cases:

Imagine the case that a new JS developer coming from C# background, when he looks at those 2 methods under the disguise of curly braces and "class" keyword. Will they immediately realise method a will consume a lot more memory if they write it that way in a Vector base class of a graphic system?

Javascript community has done a lot to mimic the classic inheritance with essentially prototype inheritance, I thought the es6 class along with the other improvements like this one should be better at encouraging a safer, standard, easier to understand way to implement, rather than just another way to "mimic classic inheritance", perhaps my expectation was too high.

@ghost
Copy link

ghost commented May 23, 2017

just an ordinary javascript user here but when es6 was new i liked the class keyword but thought a big deficiency of it was that it only did methods and not properties, but let me tell you why i hate the "class instance fields" proposal

  • it steals the syntax i'd expect to do prototype properties

  • it's simply a gimped version of the constructor (no control over execution order, no using arguments)

  • the constructor should be used as the one place to initialize instances, not the class body which previously didn't contain arbitrary expressions that have possible side effects on every instantiation all over it

i dont think the slightly more concise syntax for very simple instance properties is worth these things

in my ideal version of javascript, declaring properties using the "class instance fiends" (heh) syntax would work just like methods - properties are evaluated just once when the class is declared, and then put in the prototype - and the constructor would actually be used for its intended purpose

i am rooting for "class static fields" though because they're actually useful and behave just as expected

i don't know any languages with classical oop so i don't know what that fuss is about

@wmertens
Copy link

wmertens commented May 23, 2017 via email

@ljharb
Copy link
Member

ljharb commented May 23, 2017

@wmertens heads up, foo = () => {} is not the same as foo() {} and this.foo = this.bind(this) in the constructor; you'll absolutely want the latter in a React component (with this proposal, you can do foo = foo.bind(this) however)

@wmertens
Copy link

wmertens commented May 23, 2017 via email

@ljharb
Copy link
Member

ljharb commented May 23, 2017

@wmertens when the meat of the function is in a prototype method, and only the bind proxy is per-instance, it can probably be optimized much more than when the entire functionality is per-instance. Using per-instance arrows can also make testing and spying harder. None of this is really relevant to this proposal, of course :-)

@wmertens
Copy link

wmertens commented May 24, 2017 via email

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

8 participants