-
Notifications
You must be signed in to change notification settings - Fork 25
What should this
be bound to in initializers?
#34
Comments
I prefer option 2 (undefined or TDZ). Rationale:
|
Important also to distinguish "instance" vs "static" fields: I believe it is both useful and important that 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 |
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 |
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 |
It is not possible to reason about the value of
I don't agree here. When users wish to have memoized/bound methods, the pattern that is already used is 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? |
cc @verwaest |
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. |
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 I don't see why we should blacklist method calls from initializers when we don't blacklist them from within constructors currently. |
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. |
Why is it a different amount of footgun than allowing method calls from within the constructor? |
@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. |
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. |
I think the only reasonable way to prohibit method calling is to prohibit access to |
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. |
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 |
yes. One invoked with no arguments, so On Thu, Jul 28, 2016 at 8:23 AM, Kevin Gibbons [email protected]
Cheers, |
Unfortunately I missed the rationale for that, but it seems unfortunate that if I move my constructor code using |
I do agree. Please reraise this specific issue. On Thu, Jul 28, 2016 at 5:32 PM, Jordan Harband [email protected]
Cheers, |
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. |
cc @allenwb who was in favor of initializer bodies having the semantic intuition of a method body |
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 ( 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 |
@ljharb: It's worth mentioning that your example does behave the same. It's only the [relatively rare] case where the user uses 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
Even today, any time you move an |
@jeffmo You're right that it's a specific case - but it's an easy one to fix. That's why i think 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: If we were to go back to early error, what would you say these expressions should do within direct eval? |
I'd assume a runtime error, although it'd be great if we could forbid direct eval in there too :-) |
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). |
I guess it's also worth pointing out that the current spec text (i.e. not updated since the latest committee discussion) bans let arguments = 42;
class C { p = arguments; } |
Is |
No, but classes can be used in scripts. (Or rather, it's a valid identifier, but you can't bind to it a la |
I tried in the console and it wasn't valid, but I guess that's probably due to the console being implicitly wrapped in a function. I think that's still a fine way to ban |
I'd expect instance properties to have access to things outside the scope of the class (e.g. other variables in the same module and globals) and to FWIW, if you paste this in Chrome's console today, you can see that constructors have access to class Thing { constructor() { console.log(arguments)}}
new Thing(1)
// [q] |
Just to clarify: The discussion from yesterday does not specify that Of course that decision is being called into question here -- which is fine -- but I don't think there are any options on the table that would make initializers share an |
Ahh - I misunderstood. Is there a reason you'd want to allow access to |
You might be inside the scope of a function: function f() {
class A {
x = arguments[0];
}
} It would be odd for it to resolve to |
I wrote spec text what we discussed at TC39 in #43 |
TypeScript has had this feature for quite a long time (for instance properties), and I think it works well. Basically, you get access to |
I am pretty new to TC39 specs style so can't get on my own if instance property initializers are executed before/after I think there is a clean thought: if they are executed before And finally, if property initializers are executing after |
@jakwuh: To clarify: Instance field initializers execute after the call to |
Options:
this
captured lexically, as in an arrow function, from where the class is instantiated -- the "default" optionAny thoughts? @michaelficarra @erights @domenic @jeffmo @zenparsing @allenwb others
We have discussed this a bit, but I didn't see an open bug.
The text was updated successfully, but these errors were encountered: