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

Inherited Static properties #43

Closed
jridgewell opened this issue Sep 10, 2017 · 70 comments
Closed

Inherited Static properties #43

jridgewell opened this issue Sep 10, 2017 · 70 comments

Comments

@jridgewell
Copy link
Member

jridgewell commented Sep 10, 2017

What is the output of:

class Base {
  static #field = 'hello';

  static get() {
    return this.#field;
  }
}

class Sub extends Base {}

// This one isn't controversial
Base.get() // => 'hello'

// But what does this do?
Sub.get()

I want it to equal 'hello', but I can't find where the behavior is defined. It's likely just because I'm not familiar enough with the spec text.

Maybe it's an error, though? Since I don't think Sub has #field in its [[PrivateFieldValues]] list, unless there's some super class iteration I'm not seeing.

@bakkot
Copy link
Contributor

bakkot commented Sep 10, 2017

It is indeed an error, for exactly the reason you give.

jridgewell added a commit to jridgewell/babel that referenced this issue Sep 10, 2017
@loganfsmyth
Copy link

Is this something that should be considered for a feature? It seems like a footgun that people could easily not think about, and it's a potentially unexpected difference between instance privates and static privates.

@bakkot
Copy link
Contributor

bakkot commented Sep 11, 2017

Mmmaybe? @littledan

I don't think of it as a difference, though. Private fields are not inherited through prototypes. That's true for static fields just as for instance fields.

@loganfsmyth
Copy link

Yeah I'm on the fence. I could kind of see it as expected since the constructor extends the parent, the same way the instance does, it's just that class instances have an established chained initialization process, so it's easy to support in there, where class constructors don't have that. On the other hand, I bet it'd be a pain to make this work and or support in Babel.

I don't feel super strongly either way, but I figured it was worth asking.

@jridgewell
Copy link
Member Author

I had a prototype crawler originally, and it worked fine. I do see it as a difference between instance and static privates that non-spec-writers will definitely trip over.

class Base {
  #instance;
  static #static;

  get() {
    return this.#instance;
  }

  static get() {
    return this.#static;
  }
}

class Sub extends Base {};

// This works
new Sub().get();

// This throws
Sub.get();

@jridgewell jridgewell reopened this Sep 11, 2017
@littledan
Copy link
Member

Well, I think this is the sort of question that led static private fields to be left out of the earlier private fields proposal by @zenparsing. @erights has brought up the question about whether subclasses should have a separate private field for subclass static fields. If the answer is "no" to that question (as we settled on), I don't see why users would use this.#static rather than Base.#static (which will work just fine).

Do you think we should have separate static private fields for each subclass? Or, do you think this.#static is much more intuitive than Base.#static to use within static methods? I would've thought that it would be more common to use the class name within static methods, generally.

I agree with @bakkot that, in a sort of spec-formal way, the current semantics are consistent. But if this would be significantly confusing/underpowered for users, maybe it should be reconsidered.

@jridgewell
Copy link
Member Author

I was trying to think of some example that would use @@species with a static field, but I can't seem to come up with a concrete example where I would use this.#static over Base.#static.

Maybe just early error on this.#static for static private fields?

@ljharb
Copy link
Member

ljharb commented Sep 12, 2017

It would be nice if there was a keyword that could be used to s;ways reference the current class, like static - then you could ergonomically do static#x for the common use case, and this#x for when you wanted a method that would throw when inherited.

@littledan
Copy link
Member

Maybe just early error on this.#static for static private fields?

It's hard for me to see how we can generalize that, in a few ways:

  • Should we early error for (this).#static? For anything besides Base.#static?
  • What should we do with decorators, when it may be not known until the decorators run whether #static is static or on instances?

@ljharb Why use that keyword when you can use the name of the class? Such a name exists for class expressions which want it, too.

@ljharb
Copy link
Member

ljharb commented Sep 12, 2017

@littledan to reduce the number of places I have to make a change when I want to rename the class.

@littledan
Copy link
Member

@ljharb That doesn't seem like a primary use case. I imagine it will be more work changing the scattered usage sites than what's inside the class definition. Also, you can use class expressions for an internally visible name (though this will leak through the class's .name...).

@ljharb
Copy link
Member

ljharb commented Sep 13, 2017

Yes, I understand the use case isn't highly compelling; but it's incredibly common in the airbnb codebase to call static methods from instance methods, or statics from other statics, and it's very messy having to repeat the class name identifier.

@littledan
Copy link
Member

I imagine that you do end up repeating the class name identifier when calling static methods from instance methods...

I've heard the opposite argument, that this is harmful because it is confusing, and should be avoided whenever possible.

Maybe we'd benefit from searching through code to see how many static methods use this. My guess would be, not very many of them.

@ljharb
Copy link
Member

ljharb commented Sep 13, 2017

I think the combination of "static methods using this" and "static or instance methods using the hardcoded class name" and "instance methods using this.constructor (which is unreliable)" would all indicate where this kind of a keyword would be potentially useful.

@littledan
Copy link
Member

Let's break this down:

  • static methods using this
    If that's common, well, this is really the target case that is affected, if used together with calling static methods on subclasses.
  • static or instance methods using the hardcoded class name
    This is the case that's already working without issue.
  • instance methods using this.constructor
    If you're using this.constructor to call a static method... well, that seems roundabout enough that it seems like you really want the genericity, that your subclass will do something different. If it's that sort of case, maybe you're really looking for different, individual mutable fields for each subclass (which this proposal doesn't provide either).

Overall, I see this thread as a reason to walk back on static private fields, but I don't see a clear way forward. I don't think going up the prototype chain makes sense with the rest of the private field design. Maybe we really should be adding "class instance" private static fields.

@littledan
Copy link
Member

How often do people even call static methods on subclasses? Outside of ES6 classes, do people even bother setting up the inheritance chain?

jridgewell added a commit to jridgewell/babel that referenced this issue Sep 15, 2017
@ljharb
Copy link
Member

ljharb commented Sep 15, 2017

Probably not; that's one of the benefits of ES6 classes - that you don't have to remember all the steps.

The case that's already working is not without issue - having to hardcore the class name is the issue.

I very much doubt people using this.constructor are doing it because they want the roundabout method - in my experience, people do it because they think it's the only way to reference the class name, and because repeating the class name is a very bad practice in other languages, so they assume it's one in JS.

@littledan
Copy link
Member

This "repeating the class name is bad practice" idea is new to me. Could you point me to a style guide where it's mentioned (in any language)?

@ljharb
Copy link
Member

ljharb commented Sep 19, 2017

@littledan
Copy link
Member

@ljharb In the linked case, a technique is described for not repeating the class name when defining static methods. In JS syntax, the class name is never repeated in that case. Do you have any other reference that talks about not repeating the class name for another case?

@ljharb
Copy link
Member

ljharb commented Sep 19, 2017

@littledan the overarching principle is DRY - repeating yourself is a bad practice. http://www.korenlc.com/ruby-classes/ mentions this (in the same context; of defining static methods).

In PHP 5.5+, this principle is satisfied by static::staticMethod() - the existence of this feature serves identical use cases as what I'm suggesting static.staticMethod() might do in JS.

@littledan
Copy link
Member

OK, that's another reference which is recommending the exact same thing--not repeating the class name when defining another static method.

It sounds like there's a reasonable class for adding this static. feature, but I don't think this proposal needs to block on it. The case in question is pretty narrow--it's calling a static method on a subclass, which might be based on using this or this.constructor as the receiver, and then referring to a private field within the method. I haven't seen any evidence that this is widespread currently--the only examples I can think of for calling static methods on subclasses don't refer to any properties. I don't think it's worth complicating this proposal for this edge case.

@ljharb
Copy link
Member

ljharb commented Sep 19, 2017

Oh I totally agree it's not worth complicating this proposal; I brought it up because the future possibility of a syntactic static.foo lookup could be used to address static private fields in the future, even if we made them early errors for this proposal.

@littledan
Copy link
Member

static.foo is already an early error. Do you think we should ban private static fields because of the runtime errors that they currently cause?

@ljharb
Copy link
Member

ljharb commented Sep 19, 2017

Sorry I'm not being clear :-)

I mean, if we decide to disallow static private fields for the time being, then we could later allow them via static#foo versus this#foo inside a static method, and then there'd be a cleaner way to use them.

jridgewell added a commit to jridgewell/babel that referenced this issue Sep 20, 2017
@erights
Copy link

erights commented Dec 7, 2017

Regarding the "freeze" behavior issue, if we go for "declared public fields are accessors, whether instance or static", then public fields remain mutable under freeze in ways that are consistent with both:
* private fields remaining mutable under freeze (which is essential of course)
* the interpretation of Object.freeze as a means to tamper proof an API surface.

Defensive classes could then freeze classes, prototypes, and instances without changing the meaning of their declared properties. A client of an instance could no longer change its behavior by adversarially freezing it, since it would already be frozen.

Note that I changed the phrasing from @zenparsing 's to say "encapsulated per-instance state" rather than private field. They are observationally equivalent, so the terminology should not suggest more specificity than needed. It is this rephrasing that enables us to see better how both public static fields choices are compatible with this way of doing public instance fields.

@littledan
Copy link
Member

@erights Thanks for this thoughtful explanation of alternatives and their implications. I take it that, in addition to these possibilities, you are also OK with the current proposal's treatment of public fields; is that right?

@erights
Copy link

erights commented Dec 7, 2017

Reluctantly ok, yes. My first choice is still to simply omit them. Always err towards the side of the smaller language.

@zenparsing
Copy link
Member

@littledan

(Regarding public-fields-as-accessors)

This differs significantly from current user practice, so it should increase transition costs

I don't find the transition cost argument particularly convincing when talking about language features under development. If we are in a position where transpilers and closely-related (but far less important) languages are significantly impinging upon feature development, then the process is broken.

such pervasive use of accessors could be slower on startup

My hope has always been that the implementation of private state/internal slots/etc. would encourage simpler fixed-shape object implementations in the long run.

it's another point of divergence with the semantics users currently have for public fields, which may be working OK as is for programmers

As with any cooperative and long-lasting endeavor we need to carefully balance path-dependence with our desired long-term future state.

@erights

My first choice is still to simply omit them

That was my first choice as well; public fields are pretty hard to justify over just putting the assignments directly into the constructor. By using syntactic field definitions, the programmer is trying to express a notion about the fixed shape of instances, and for classes, fixed shape should (IMO) be represented by one feature: private state.

An implementation of public fields as private state with accessors also leaves open the possibility of expressing, quite easily, something that we've talked about several times in the past: const classes. The idea of a const class is that it has a tamper-proof API.

The primary downside of using the accessor approach (as far as I can tell) is that it might be confusing for programmers that expect public fields to be enumerable own properties. If a programmer wants to use class syntax as a factory for creating object-as-dictionary style objects, then they would still need to use assignments within the constructor.

On the other hand, I've always been quite happy imagining a future where classes are primarily const, and class syntax is primarily used to express fixed-shape objects. Implementing a field syntax with own enumerable data properties has never really fit in with that vision.

Cheers!

@littledan
Copy link
Member

My hope has always been that the implementation of private state/internal slots/etc. would encourage simpler fixed-shape object implementations in the long run.

I don't actually see how these differ in how fixed the "shape" is. For private fields, committee members expected private fields to throw exceptions when they're not yet written to, and then be readable from later initializers. Somehow, we need to record that state transition, either in the hidden class of the object or in some other place off to the side. Either way, there are observable state transitions during initialization, so implementation-wise you don't get the biggest benefit of a static shape. Instead, the feature encourages fixed shape by making it so that, when things go well, you always get all the fields by the time the constructor returns. I don't really see how we can do any better, or how accessors would change anything.

@ljharb
Copy link
Member

ljharb commented Dec 8, 2017

By using syntactic field definitions, the programmer is trying to express a notion about the fixed shape of instances

This is decidedly not what I am ever trying to express with public fields, fwiw. I don't care about the fixed shape of instances as a developer; that's something that implementors care about. If I cared, I'd be using a type system.

CodingItWrong pushed a commit to CodingItWrong/babel that referenced this issue Mar 12, 2018
CodingItWrong pushed a commit to CodingItWrong/babel that referenced this issue Mar 12, 2018
CodingItWrong pushed a commit to CodingItWrong/babel that referenced this issue Mar 12, 2018
CodingItWrong pushed a commit to CodingItWrong/babel that referenced this issue Mar 13, 2018
CodingItWrong pushed a commit to CodingItWrong/babel that referenced this issue Mar 13, 2018
CodingItWrong pushed a commit to CodingItWrong/babel that referenced this issue Mar 13, 2018
CodingItWrong pushed a commit to CodingItWrong/babel that referenced this issue Mar 13, 2018
CodingItWrong pushed a commit to CodingItWrong/babel that referenced this issue Mar 13, 2018
CodingItWrong pushed a commit to CodingItWrong/babel that referenced this issue Mar 13, 2018
CodingItWrong pushed a commit to CodingItWrong/babel that referenced this issue Mar 13, 2018
CodingItWrong pushed a commit to CodingItWrong/babel that referenced this issue Mar 13, 2018
CodingItWrong pushed a commit to CodingItWrong/babel that referenced this issue Mar 13, 2018
CodingItWrong pushed a commit to CodingItWrong/babel that referenced this issue Mar 13, 2018
CodingItWrong pushed a commit to CodingItWrong/babel that referenced this issue Mar 15, 2018
CodingItWrong pushed a commit to CodingItWrong/babel that referenced this issue Mar 16, 2018
CodingItWrong pushed a commit to CodingItWrong/babel that referenced this issue Mar 20, 2018
@littledan
Copy link
Member

We've been continuing the discussion in https://github.com/tc39/proposal-static-class-features/ , while this proposal omits static fields.

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