-
Notifications
You must be signed in to change notification settings - Fork 113
Allow to mark class fields as readonly #83
Comments
I was thinking that decorators could be used for this, but the latest decorators proposal doesn't allow for this (details). It might be possible to implement in a future add-on to the decorators proposal, but I am now thinking it would be better to just add a
I think readonly properties are an important use case, and it's also important in many cases to be able to set them in the constructor but not allow them to be writable after that. Yes, we've gotten by without it all this time, but that doesn't mean the solutions we've been using are ideal. Alternatively, the decorators proposal could go ahead and implement a |
A matter of taste, and I don't have a strong opinion about the keyword. A decorator would have been fine too. However, I do believe certain things should be first class concepts of a language, and since there are non-reassignable variables, readonly fields are a natural conclusion. Immutability is an important concept in JavaScript, and deserves first class support in my opinion. |
I think this could be implemented as a decorator, and doesn't need to be built-in. It's pretty difficult to come up with built-in semantics that really work the way you'd expect them to, unfortunately. It's not possible to use a finisher to turn a field non-writable (since finishers run once per class) but decorators could be used to make a constant field by turning it into a getter/setter pair which permits exactly one set (either from the initializer or, if there is no initializer, from the first thing that writes to it). |
Can a field decorator add a finisher? |
@ljharb Yes. But this finisher runs once for the whole class. |
Yeah, in order to use a decorator finisher to implement read-only fields, the decorators proposal would need to include instance-level finishers, which @littledan said (in a different thread) probably won't be included in the first official decorators spec (but could be added in v2). A getter/setter pair that permits only one set might be OK in the meantime (and perhaps even a good thing in the case of something like an I can actually see both kinds of decorators being useful in different situations. If going with @littledan's suggestion, I would probably call the decorator |
@littledan thanks for your input, this kind of would work. Relatively easy to implement even, there is a similar great example in the decorator proposal for observable fields. After thinking a bit about it, I'd say, this stands and falls with anonymity of private fields. At the moment, decorators can create anonymous, thus "hidden", private slots. If this changes, or can be bypassed, immutability is gone too. However, nothing except time is lost if we do not add a keyword now. The time it takes for a new proposal to add it, and some time for missed JIT optimizations. Baked in immutability would be a huge plus, IMHO. |
I'd prefer a decorator and to respect the field descriptors semantic, ie |
I think it would be best to limit |
@mbrowne But why have a |
Because it's a useful pattern and that's how read-only properties are implemented in many other languages, so it's more likely to be the programmer's expectation than any of the other alternatives. But I don't know if naming it this way is the best; perhaps it should be more explicit like |
For keyword, const x;
// ...
x = 1; // initialize later Note, when class fields land, it will be very hard to add readonly x;
readonly // <- a field named "readonly"
y; About the name for decorator, |
I still don't really know what semantics you're proposing here. Could you be more specific, in terms of what this would mean in terms of the object model, when you can write to the field, etc? I'm still not sure exactly what benefit we're expecting to get by making this built-in as opposed to based on a decorator, possibly based on the addition of more decorator capabilities. Would it be OK to pursue this feature in a follow-on proposal? The class fields proposal is already at Stage 3, which means that we hope to be done with the "language design" part and are getting tweaks from actual implementations. |
I think it depends on the overall strategy. If in the future there will be other modifier keywords for class fields and methods (e.g. ...but now that I think about it, this introduces a new issue to discuss, which is that someone could legitimately want a property named any of these things, or might already have properties with these names in code they wrote using the current class-properties Babel plugin or TypeScript. I just tried it in TypeScript, and actually they don't have any restrictions on this: e.g. you can have a private string property named Anyway, I also wanted to offer another alternative to consider, which is that the decorator syntax could be used for all of these things, even for features that might be native in the future. There could be a special namespace or syntax to indicate that, e.g.
|
Yes TS guys decide to limit the keywords |
@littledan semantics I had in mind is more or less analogous to Java and others: Readonly fields must either have an initializer, or be initialized in the constructor (but not both). After once assigned a value, it cannot be re-assigned. Inherited private properties are not relevant, since completely invisible to a subclass. Public fields can not be shadowed or overridden by child classes, re-defining a readonly property should throw an error. (Same actually goes for method definitions, readonly would be useful there too. Where would be the right place to discuss this?) Today I would have to use Object.createProperty in the constructor to do this, but that's too cumbersome to me and therefore I don't do it. For static class constants it is even less obvious, as this has to be done after the class is defined. To communicate intent, allow for some JIT optimizations and prevent mistakes I would like to use it in some simple fashion. Why this is helpful: Immutable value objects, immutable services etc. Often a field is initialized in the constructor, and not intended to ever be overwritten. Most fields in our code are not intended to be re-assigned after IOC has wired up everything. @hax you are right, was not thinking about ASI. At the very least reserving potential modifiers would not hurt. I think we might could learn a lesson from PHP here, where they got class fields entirely wrong at first. As I said earlier, I think some concepts should be first class. Immutable variables are, so immutable fields should be too. Decorators can help with many things, but this I'd rather have one single first class facility. |
Should we allow this? class A {
readonly x
constructor() {
this.x = 1
...
this.x = 2
}
} Sometimes I think allow mutate readonly fields in the constructor is convenient. And should we allow this? class A {
readonly x
constructor() {
this.init() // call other methods
}
init() {
this.x = 1 // init() should only be called by constructor, or it will throw runtime error here
}
} It's common to do initializations in other methods, especially when there are many states need to initialize, for example, deal with complex options object. And should we allow this? class A {
readonly x
}
const a = new A // `x` not initialized, throw TypeError here?
a.x // `undefined` or throw TypeError?
a.x = 1 // can we ? Note, it |
@hax first example, where would this be useful? I've never encountered it, and I'm suspicious.. I'd probably even rate it as a code smell in a review. Thus, no, I would specifically think this should not be allowed. Your second example looks useful sometimes, but also seems very confusing. Id rather not allow it. In the third example, the field should just be undefined, and not throw an error. Trying to set it "from the outside" should always throw, even if not assigned (is undefined). |
@bschaepper class A {
readonly x
constructor() {
this.x = this.initX()
}
initX() {
let x = 1
...
if (...) x = 2
return x
}
} But that means many old codes need to refactor first before add |
@hax I think your second example should be allowed, but not the others. It often improves code readability to have helper methods called by the constructor that initialize some of the fields. And I think it would be easy to implement with decorators if a future version of decorators allowed instance-level finisher functions. In that case you could just set all the I think the bigger question to be answered here is whether or not some possible future modifier keywords (including |
This semantic discussion is great, but we have to think about how it will be represented in the JavaScript object model as well as within the lifecycle here of superclass and subclass constructors. We can't apply the Java model here, as I believe that is based on a static analysis of how many times the instance variable is written to. |
Perhaps an example will help to discuss what the specification should be. Let's suppose this is the code we'd like to write:
Here is how I am thinking it could work under the hood - demonstrated in ES6:
(Side-note: In a real app I would probably have some sort of convenience mechanism to set the initial state of the object [the above example is a little verbose/repetitive], but not just |
P.S. I realize that my proposed design as shown in the ES6 example doesn't prevent @hax's first example, which I said should be illegal:
As I said above, that could be prevented with a linting rule. However, if readonly properties are natively supported rather than implemented with a userland decorator, it would be ideal if the JS engine made the property read-only immediately after the first write in the constructor (i.e. you can't set it multiple times in the constructor, only once). |
@mbrowne Very nice illustration, similar to what I was thinking. However, I'd set fields to Part of the goal for An assignment to readonly properties thus always comes down to this:
This greatly simplifies semantics, IMHO, making readonly a very simple thing. @littledan does this illustrate what is supposed to happen for you, or shall I try to spec this out, in a PR maybe? |
Note: if you make fields non-writable and non-configurable immediately,
writing a userland decorator for this would no longer be an option, even
with the addition of instance-level finishers (unless decorators would have
a special ability to recreate a non-configurable field), because by the
time the user's code in the constructor ran, it would be too late to set a
value - the field would already be non-writable.
But we're still discussing whether or not decorators are a good solution
for this, so maybe that's fine. And maybe it would make sense to allow
decorators to recreate a non-configurable field in the finisher.
|
You are right, @mbrowne. I'm very sorry but I have to ask: From the "Evaluation Order Proposal" references in decorators-proposal my understanding is, 1. constructor is executed 2. field decorators are executed, 3. initializers are executed. (leaving out irrelevant details). Is this correct? If so, I am not certain this is the best way. First of all, I can not reference field values with an initialzer, they are undefined while the constructor is evaluated, right? Furthermore, they are not yet decorated, or are they? If I'm not mistaken, this is a problem, or very unexpected at least. Probable bugs whenever I set a value in the constructor, since decorations are not ready, or use a field in the constructor, since values are not initialized yet. Isn't that extremely confusing? Up until re-reading the evaluation order I was under the impression, initializers and decorators should run before one gets a chance to use them in the constructor. Would make a lot more sense from a user perspective IMHO. |
@bschaepper It looks like that "Class Evaluation Order" PowerPoint presentation (assuming that's what you're referring to) was written a while ago, without class properties in mind. I think the better place to look to understand the evaluation order would be this proposal (which says it includes everything that was previously in the class evaluation order proposal). My understanding is that declared fields are created before the constructor runs, at least if we're talking about just one class. If there is a superclass, then its constructor won't know anything about instance-level fields in subclasses because they don't exist yet (I was actually a bit concerned about that as I explained in #36, but there are other ways to enable reflection on subclass properties, such as decorators or alternatively flow types made available at runtime). Also note that initializers aren't just for decorators; it's just reusing the initializers that are already part of this proposal. If you have:
then the initializer is setting |
About ASI (automatic semicolon insertion)...Suppose that a future version of JS has
I think it might be better to just enforce that the modifier and the field name are always together on the same line, similarly to the requirement for
It would also be one less thing to add to this proposal, helping it get out the door (not that reserving keywords is a big effort, but still...). Read-only properties could be a later add-on to this proposal. Or alternatively, included in a standard decorators library with a future version of decorators. |
@mbrowne totaly right about ASI. I'd say it's a great idea to disallow newlines in field declarations anyway. IIRC comma separated fields have been dropped for similar reasons, no? Thanks for your clarification on the evaluation order too, it does make sense as you state it. If my understanding about decorators is correct, it can always replace the field definition with its own - which makes setting properties to non-writable non-configurable no problem, right? Or did I get something about the specs wrong :-) |
It's an issue of timing. If the decorator sets the property to non-writable and non-configurable when it's being created, then it won't be possible to set a different value via a constructor argument (e.g.
If instance-level finishers are added to the decorators proposal, then we would have an additional step 3 where decorators could make further modifications to property descriptors. But that doesn't address your concerns about child classes re-defining a readonly property that was declared in a parent class. I think native support would be required for that, or at least I can't think of a good way to do it with decorators. Update: Well, there is one possible way: if decorators would have a special ability to recreate a non-configurable field. But currently they don't. |
@mbrowne IMO, modifiers and identifiers can be separated by newlines, that must not be enforced. |
@doodadjs Why? The same-line restriction seems to work fine for |
@mbrowne didn't know about "async function"... Anyway, that's something that must be enforced by a linter, not the language itself. |
For |
Also in Chrome (just tested it), and presumably any browser that supports async functions. |
Tested on Firefox, and browsers too. Doesn't mean it was a good idea, but anyway... |
TypeScript already enforced it. (readonly, public, private... and even get/set though JavaScript allow it cross-lines) |
I think it's possible to revise |
OK, I still don't see a workable proposal for semantics in this thread. Would it be possible to pursue this as a follow-on proposal? |
I think that's a good plan. The only thing that would need to be done now rather than later is reserve certain modifier words including So currently I'm leaning toward a |
OK, anything more to do in this bug thread, or should we close it? |
Whether decorator or keyword does not matter too much, I just want some way to have readonly properties. Insofar, @littledan, close the issue if you think this poses no major roadblock for a follow-on proposal Overall I find it hard to believe, that this is the best we can come up with:
Very noisy, and overall implicit and inconsistent notation. Private, protected and public are all completely different. Very awkward. I honestly can not follow why this seems like a good idea and the right way to go to anybody. I'd much prefer a cleaner and more consistent syntax. Also making the implicit explicit with modifiers.
|
@bschaepper How about this for a gradual evolution path?
|
To add to what I said above, I'd also be in support of proceeding on two fronts in parallel:
Ultimately, I agree it would be best if the most common use cases - the things that are core features in many other OO languages - were supported natively as modifier keywords. @littledan's proposed path makes sense and due to limited time, I personally will probably focus more on the decorator option for now, especially since I think instance-level finishers for decorators would be a good feature to have anyhow. But if anyone wants to draft a proper proposal for native readonly fields, I'll certainly follow that discussion and support the proposal (FWIW). Also, when I was thinking about modifier keywords I had forgotten about |
BTW, I think the |
@littledan your approach seems right 😊 I'm looking forward to finally having simple and real private state in JavaScript. I know it must be a bit frustrating at times, but keep up your good work 😊 👍 @mbrowne thanks for the constructive discussion 😉 |
Glad we could all get on the same page about next steps. See CONTRIBUTING.md for how to keep going here. |
Similar to Java
final
, there should be a convenient way to define a field explicitly as non-writable. With not to much unfamiliar syntax, this could look similar to this:Readonly fields must either be initialized directly, or in the constructor. Re-assigning them throws an Error, just as reassigning
const
variables does.The text was updated successfully, but these errors were encountered: