proposal-class-public-fields icon indicating copy to clipboard operation
proposal-class-public-fields copied to clipboard

What should `this` be bound to in initializers?

Open littledan opened this issue 9 years ago • 39 comments

Options:

  • The new object under construction -- what this proposal currently states
  • undefined or TDZ -- what the private state proposal states
  • The outer this captured lexically, as in an arrow function, from where the class is instantiated -- the "default" option

Any thoughts? @michaelficarra @erights @domenic @jeffmo @zenparsing @allenwb others

We have discussed this a bit, but I didn't see an open bug.

littledan avatar Apr 04 '16 17:04 littledan

I prefer option 2 (undefined or TDZ). Rationale:

  • Binding this to the instance would necessitate a novel scope, where this is not lexically bound to the containing scope, but arguments is. In general, I would prefer not to introduce any additional new scoping rules for this.
  • Binding this will strongly encourage developers to write methods as arrow functions installed per-instance. I don't think we want to pave that path.

zenparsing avatar Apr 04 '16 17:04 zenparsing

Important also to distinguish "instance" vs "static" fields:

I believe it is both useful and important that this in instance fields should refer to the instance, and this in static fields should refer to the class object.

For instance fields: It is often useful to use methods on the prototype chain to calculate the initialization value of a field, so having access to this is important in these cases.

jeffmo avatar Apr 04 '16 17:04 jeffmo

so having access to this is important in these cases.

My point of view, from the private field side of things, is that the initializers are only there as sugar for when you are initializing to some obvious initial value. For instance:

class C {
    #list = [];
    constructor() { }
}

I've always just imaged that anything more complicated (including dereferencing this properties) should just go in the constructor, where all of the other complex initialization logic resides.

zenparsing avatar Apr 04 '16 17:04 zenparsing

Option 1. Option 3 is unintuitive and confusing, and Option 2 cripples the usefulness of the proposal.

One concrete use case: a React "class" component, with a getInitialState prototype method, with state = this.getInitialState() (that way anything else that resets the state can re-call the getInitialState method to get a fresh state object).

ljharb avatar Apr 04 '16 17:04 ljharb

Binding this to the instance would necessitate a novel scope, where this is not lexically bound to the containing scope, but arguments is. In general, I would prefer not to introduce any additional new scoping rules for this.

It is not possible to reason about the value of this anywhere in JS without contextual information. It may be a lexical binding, but it is expectedly parameterized on context. With the context of a property initializer, it's clear to most that I've polled that it is intuitive to expect this to refer to the instance in an instance field property.

Binding this will strongly encourage developers to write methods as arrow functions installed per-instance. I don't think we want to pave that path.

I don't agree here. When users wish to have memoized/bound methods, the pattern that is already used is this.foo = this.foo.bind(this);. I don't see any harm in this as long as it is done with consideration. The truth of property initializers is, of course, that the expression is evaluated on each instantiation.

All that said, there is a clear pattern that benefits from creating per-instance function properties (the most common use-case for this is setting method-listeners as event handlers). I think if we want to omit this capability on the grounds that it isn't useful or reasonable, we need to show that this use-case isn't useful or reasonable.

Additionally I would be interested to hear how difficult it would be for VMs to optimize the un-mutated form of a function-property? @littledan, thoughts here?

jeffmo avatar Apr 04 '16 17:04 jeffmo

cc @verwaest

littledan avatar Apr 04 '16 18:04 littledan

I've always just imaged that anything more complicated (including dereferencing this properties) should just go in the constructor, where all of the other complex initialization logic resides.

My only concern with this perspective is that it downplays the value of a pattern that many users do find useful. I think it's a reasonable perspective as a style for some cohort of users, but I don't think we have shown sufficient grounds to ban it at the language level when it is clearly useful for some valid patterns.

jeffmo avatar Apr 04 '16 18:04 jeffmo

If you're not careful with the first option, you end up with this sharp edge:

class Thing {
  _something = this.makeSomething();

  makeSomething() {
    // constructor hasn't run here yet
  }
}

There's definitely value in allowing a property's value to be the result of an expression (for instance, it allows you to call other object's constructors rather than only primitives). However, an author may reasonably assume that a class's methods will be run after the constructor. If we're not thoughtful here, we may break that assumption. (The Babel implementation currently does.)

Would it be possible to parse property definitions in source order, blacklisting method calls?

Being able to say


class Thing {
  _thing = new Rx.Subject();
  _otherThing = this._thing.map(…);
}

is really nice, but


class Thing {
  _thing = this.makeThing();
}

seems like it shouldn't work.

appsforartists avatar Jul 25 '16 20:07 appsforartists

@appsforartists I don't see why we should blacklist method calls from initializers when we don't blacklist them from within constructors currently.

littledan avatar Jul 25 '16 21:07 littledan

Maybe my concerns are off-base. Allowing methods in property initializers feels like a foot-gun.

To be fair, this isn't a bug I've personally encountered, just one I got curious about exploring.

appsforartists avatar Jul 25 '16 21:07 appsforartists

Why is it a different amount of footgun than allowing method calls from within the constructor?

littledan avatar Jul 25 '16 22:07 littledan

@appsforartists, I feel like it would be more surprising if other properties were visible but methods were not. Since this issue only applies within a single class (i.e. since the super constructor will have run by the time you could call a method from the superclass), I don't think allowing method access is too risky.

bakkot avatar Jul 25 '16 23:07 bakkot

That case wasn't one that came to mind when I raised the concern.

This is potentially more foot-gunny, since it expands the scope of places where a method may be called before the constructor has completed (and indeed is the only place where a method may be called before the constructor has even been called). Still, I understand the objection to my concern.

appsforartists avatar Jul 26 '16 01:07 appsforartists

I think the only reasonable way to prohibit method calling is to prohibit access to this entirely.

littledan avatar Jul 26 '16 01:07 littledan

Some methods don't need the constructor to have been called; some methods can be called as long as some things have been initialized on the instance. The same hazard exists with:

constructor(x) {
  this.foo(x);
  this.bar = {};
}
foo(x) {
  this.bar.baz = x;
}

If your own implementation has ordering dependencies with the constructor, than your instance properties will require the same ordering dependencies. This doesn't seem to me to be a footgun as much as a natural consequence of writing code that is brittle in that way.

ljharb avatar Jul 26 '16 04:07 ljharb

As I understand it, discussion today seemed to settle on treating initializers as the body of an anonymous method (static method for static fields) for the purposes of this, super, new.target, and arguments.

bakkot avatar Jul 28 '16 06:07 bakkot

yes. One invoked with no arguments, so arguments would have a zero length.

On Thu, Jul 28, 2016 at 8:23 AM, Kevin Gibbons [email protected] wrote:

As I understand it, discussion today seemed to settle on treating initializers as the body of an anonymous method (static method for static fields) for the purposes of this, super, new.target, and arguments.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/jeffmo/es-class-public-fields/issues/34#issuecomment-235809579, or mute the thread https://github.com/notifications/unsubscribe-auth/AAQtzFJAyiifygdmSk4U3cCpyYK4XSB0ks5qaEragaJpZM4H_UlZ .

Cheers, --MarkM

erights avatar Jul 28 '16 15:07 erights

Unfortunately I missed the rationale for that, but it seems unfortunate that if I move my constructor code using arguments to an initializer, it would silently work differently - I feel like the early error would be much more useful.

ljharb avatar Jul 28 '16 15:07 ljharb

I do agree. Please reraise this specific issue.

On Thu, Jul 28, 2016 at 5:32 PM, Jordan Harband [email protected] wrote:

Unfortunately I missed the rationale for that, but it seems unfortunate that if I move my constructor code using arguments to an initializer, it would silently work differently - I feel like the early error would be much more useful.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/jeffmo/es-class-public-fields/issues/34#issuecomment-235931984, or mute the thread https://github.com/notifications/unsubscribe-auth/AAQtzE3tlwE15xeUDhsf_rB5_2ZtkRdqks5qaMuTgaJpZM4H_UlZ .

Cheers, --MarkM

erights avatar Jul 28 '16 15:07 erights

We have a choice to make: Should we have special handling of arguments, as in the current spec text, or should we make it analogous to something pre-existing, which would have an empty arguments? Either is technically feasible, both from a spec text and implementation perspective. How should we weigh these alternatives?

Overall, I think of arguments as an "old, bad style" feature, and I think users are recognizing this too, using rest parameters instead. Maybe they'll switch to ES2015 features in refactorings before they switch to more advanced class features that come in later versions of ES, and this question won't become an issue for them.

littledan avatar Jul 28 '16 17:07 littledan

cc @allenwb who was in favor of initializer bodies having the semantic intuition of a method body

jeffmo avatar Jul 28 '16 17:07 jeffmo

I think that it is critical to match most users' mental model so that in the following example:

class Foo {
  foo = bar; // 1a
  [baz] = quux; // 2a

  constructor() {
    this.foo = bar; // 1b
    this[baz] = quux; // 2b
  }
}

that - separate from named constructor arguments, which we've all agreed multiple times only make sense when lexically available - that lines 1a/1b, and 2a/2b, for any given LHS value (foo or baz), and any given RHS value (bar or quux), either behave exactly the same in both positions, or, have an error (early or otherwise) in one or both positions. The same should be true even if we come up with an alternative syntax for = because of a different mental model we want to respect (declarative vs imperative, for example).

We can separately decide what semantics we want (set vs define, for example) and what syntax we want, but I think it's a massive refactoring hazard if I can move an initializing line out of a constructor, remove the "this" or "this.", and have it silently behave differently. The same should hold if I move an instance property initializer line into a constructor. I think that allowing arguments inside the class body to mean anything other than an error, or "the constructor's arguments object" (which we don't want, for a number of reasons), would be a huge mistake.

ljharb avatar Jul 28 '16 17:07 ljharb

@ljharb: It's worth mentioning that your example does behave the same. It's only the [relatively rare] case where the user uses arguments (and not the less-deprecated rest-arg) that it would not. I realize you used bar and quux as higher-level placeholders, but I want to point out the importance of considering relevancy of the specific case.

As much as it's important to consider the holistic/general composability of stuff like this, it's also important to consider the likelihood of encountering various incongruencies. Additionally, as mentioned in the discussion yesterday: There are many views of "consistency" that can often conflict. In this case, one view of consistency is to say that initializer bodies are consistent with method bodies. Another view of consistency is to say that initializer bodies are consistent with behavior of the same logic in a constructor.

As for where I stand: I don't feel super strongly here, but at the moment I find the "consistency across class members even in rare cases" perspective a little more compelling than the case for "consistency with constructor initializers even in rare cases" (where "rare cases" is referring to the use of new.target/arguments in initializers).

The same should hold if I move an instance property initializer line into a constructor.

Even today, any time you move an arguments or new.target expression from one position to another you risk heavy contextual mis-match -- so I don't think it would be too surprising to expect this (even if done by mistake). There's really no good reason to use arguments or new.target inside an initializer expression though, so again I think it's reasonable to expect the initializer -> constructor refactoring of arguments/new.target to be extremely rare. The opposite direction seems more plausible (but also flawed WRT the expectation of the behavior of the expressions after factoring out of a contextual function body).

jeffmo avatar Jul 28 '16 17:07 jeffmo

@jeffmo You're right that it's a specific case - but it's an easy one to fix. That's why i think arguments MUST be an early error in a class body, which we all agreed to in May.

That there's no good reason to use either of those in initializer position is exactly why they should be forbidden by the grammar.

ljharb avatar Jul 28 '16 17:07 ljharb

@ljharb: If we were to go back to early error, what would you say these expressions should do within direct eval?

jeffmo avatar Jul 28 '16 17:07 jeffmo

I'd assume a runtime error, although it'd be great if we could forbid direct eval in there too :-)

ljharb avatar Jul 28 '16 17:07 ljharb

Unfortunately detecting direct eval statically is undecidable, so there's not much we can do there.

We could make it a runtime error only for direct eval, but we do have some precedent for removing static errors when there's a chance that static analysis can't entirely cover the validation (duplicate object properties in strict mode comes to mind).

jeffmo avatar Jul 28 '16 17:07 jeffmo

I guess it's also worth pointing out that the current spec text (i.e. not updated since the latest committee discussion) bans arguments in a not-so-precise way -- which is that it bans any identifiers named arguments. The false positive here (albeit potentially rare) is:

let arguments = 42;
class C { p = arguments; }

jeffmo avatar Jul 28 '16 18:07 jeffmo

Is arguments a valid identifier at the top level of modules?

ljharb avatar Jul 28 '16 18:07 ljharb

No, but classes can be used in scripts. (Or rather, it's a valid identifier, but you can't bind to it a la let arguments.)

bakkot avatar Jul 28 '16 18:07 bakkot