tc39 / proposal-class-public-fields

Stage 2 proposal for public class fields in ECMAScript
https://tc39.github.io/proposal-class-public-fields/
488 stars 25 forks source link

Usage with arrow functions #18

Closed AsaAyers closed 8 years ago

AsaAyers commented 8 years ago

What is the intended behavior here? Based on the location where all of these arrow functions are defined, I think they should all return "outer"

function contextWrapper() {
  const outer = () => this.name

  class Example {
    name = 'instance'
    instanceMethod = () => this.name
    static staticMethod = () => this.name
  }

  const tmp = new Example()

  console.log('outer()', outer())  
  console.log('Example.staticMethod()', Example.staticMethod())
  console.log('tmp.instanceMethod()', tmp.instanceMethod())
}

contextWrapper.call({name: 'outer'})

Using babeljs.io/repl the output is actually:

outer() outer
Example.staticMethod() outer
tmp.instanceMethod() instance
jeffmo commented 8 years ago

Babel's output is correct. Instance field initializer expressions operate in the context of the instance itself, and so their this value is a pointer to the instance object that was allocated from the class.

This is important for multiple reasons: It allows field initializers to use helper methods on the instance, it allows field initializers to refer to each other -- even when one field is at a higher level of class hierarchy than the other, it allows for instance arrow properties to close over the instance this similar to how a method works (except there's no need to .bind such a property when passing it off to, say, an event handler), etc.

AsaAyers commented 8 years ago

Is this explicitly stated in the proposal? I think an argument could be made either way.

All other arrow functions inherit the this of exactly where they physically are in the code. Making instanceMethod and staticMethod here have two different this is not what I would expect.

If instanceMethod binds this to the instance, why doesn't staticMethod bind this to the class? That would seem like a more consistent behavior. If they did then you would get the same behavior as you do by default with an ordinary function, it would just be permanently bound to the same thing.

Here is an expanded example

function contextWrapper() {
  const outer = () => this.name

  class Example {
    name = 'instance'
    static staticMethod = () => this.name
    instanceMethod = () => this.name

    static staticFn = function() { return this.name } // Example
    instanceFn = function() { return this.name } // instance
  }

  const tmp = new Example()

  console.log('outer()', outer())  
  console.log('Example.staticMethod()', Example.staticMethod())
  console.log('tmp.instanceMethod()', tmp.instanceMethod())

  console.log('Example.staticFn()', Example.staticFn())
  console.log('tmp.instanceFn()', tmp.instanceFn())
}

contextWrapper.call({name: 'outer'})

This is the current output.

outer() outer
Example.staticMethod() outer
tmp.instanceMethod() instance
Example.staticFn() Example
tmp.instanceFn() instance
jeffmo commented 8 years ago

Is this explicitly stated in the proposal?

Yea, it's mentioned in step 4 here: https://github.com/jeffmo/es-class-static-properties-and-fields#instance-field-initialization-process

All other arrow functions inherit the this of exactly where they physically are in the code

This is true, but also all other expressions in the language execute exactly where they are physically in the code as well. Here we don't execute the expression until instantiation time. So I think the concern you're raising generalizes further to the fact that this expression is executed in a deferred manner and isn't really specific to how this dereferences, right?

If instanceMethod binds this to the instance, why doesn't staticMethod bind this to the class?

Yea, I've thought a little about this and it makes sense to me that this could refer to the class itself. I'd like to stew on this a little more, but you're probably right here.

AlicanC commented 8 years ago
function wrapper() {
   class Parent {
    static foo = 'Parent.foo';
    static sayFoo = () => this.foo;
    // static sayChildFoo = () => ???.foo;

    foo = 'parent.foo';
    sayFoo = () => this.foo;
    // sayChildFoo = () => ???.foo;
  }

  class Child extends Parent {
    static foo = 'Child.foo';
    static sayFoo = () => this.foo;
    // static sayParentFoo = () => ???.foo;

    foo = 'child.foo';
    sayFoo = () => this.foo;
    // sayParentFoo = () => ???.foo;
  }

  const parent = new Parent();
  const child = new Child();

  console.log('parent.sayFoo()', parent.sayFoo());
  console.log('child.sayFoo()', child.sayFoo());

  // 1
  console.log('Parent.sayFoo()', Parent.sayFoo());
  console.log('Child.sayFoo()', Child.sayFoo());

  // 2
  console.log('Child.sayParentFoo()', '???');
  console.log('child.sayParentFoo()', '???');

  // 3
  console.log('Parent.sayChildFoo()', '???');
  console.log('parent.sayChildFoo()', '???');
}

wrapper.call({ foo: 'outer.foo' });

/*
parent.sayFoo() parent.foo
child.sayFoo() child.foo
Parent.sayFoo() outer.foo
Child.sayFoo() outer.foo
Child.sayParentFoo() ???
child.sayParentFoo() ???
Parent.sayChildFoo() ???
parent.sayChildFoo() ???
*/

My thoughts:

  1. Parent.sayFoo() and Child.sayFoo() should log Parent.foo and Child.foo.
  2. super should work for Child.sayParentFoo() and child.sayParentFoo().
  3. Something should be introduced to make Parent.sayChildFoo() and parent.sayChildFoo() work.

PHP has self, parent and static to make all of these work. We have this for self, super for parent, but we don't have anything for static which is required to make number 3 work.

jeffmo commented 8 years ago

Either way we go here we're going to give a behavior that may not be suitable for some use cases. Fortunately in the scenario outlined above, it's possible to create a self variable in the wrapper class and refer to that directly. If we were to default this to resolve to the enclosing environment, there wouldn't be a good workaround.

However the strongest arguments in favor of this referring to the class at hand are:

  1. Allows field initializers to refer to each other during initialization.
  2. Allows field initializers to use helper methods defined on the class (or parent classes) during initialization.
  3. Allows for a simple conceptual mental model shows field initialization moving in/out of the constructor minimal common pitfalls.

Per the last meeting, we've opted to think of initializer expressions as having the semantics of individual methods with no arguments being run on the class.