-
Notifications
You must be signed in to change notification settings - Fork 12.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Suggestion: a streamlined way to "expose" private variables publically #11804
Comments
class Container<T> {
private value: T;
constructor(value: T) {
this.value = value;
}
public getValue(): T {
return this.value;
}
} The standard approach is to use a getter. class Container<T> {
private _value: T;
constructor(value: T) {
this._value = value;
}
get value() {
return this._value;
}
}
new Container("foo").value; // "foo" |
@aluanhaddad yes, and using a getter results in significantly more generated JavaScript on lower ES targets. The point of this proposal is to not have to do that. Without a getter: var Container = (function () {
function Container(value) {
this._value = value;
}
return Container;
}()); With a getter: var Container = (function () {
function Container(value) {
this._value = value;
}
Object.defineProperty(Container.prototype, "value", {
get: function () {
return this._value;
},
enumerable: true,
configurable: true
});
return Container;
}()); Creating a getter is a bit of extra work that I don't think should be necessary for this type of behavior. Edit: But yes, it would have been better for me to write "a possibility" instead of "a standard". |
@JoshuaKGoldberg Indeed there is more emitted code but performance in es5.1 environments should be quite good. |
What you are missing are two modifiers, |
Looks like the team is collectively not in favor of adding new keywords for this. That makes sense. How about combining existing privacy keywords?
class AllowedMemberPrivacySettings<T> {
public fullyPublic: T;
public protected publicGetsProtectedSets: T;
public private publicGetsPrivateSets: T;
protected fullyProtected: T;
protected private protectedGetsPrivateSets: T;
private fullyPrivate: T;
} The import { action, observable } from "mobx";
class Store<T> {
@observable
public private data: T;
@action
public setData(data: T): void {
this.data = data;
}
} |
This falls victim to the same problem as others - we don't distinguish the type of a property for reading vs the type/accessibility of it for writing, so the variance problems become rather hairy and you'd quickly find yourself doing unsound writes or reads. |
Right now, if you want to expose a class property with a public getter and a private setter,
the standarda possibility is to use methods (since gets and sets can't have different visibility):The code works fine, but with a caveat: there's an extra function defined for every exposure. That really bloats the generated JavaScript.
Edit: the same holds true for creating a getter. See the second and third comments.
I propose some sort of exposure functionality be added for these simple private values. They should still be publicly gettable but only privately settable.
A few options would be to mix
public
andprivate
, create a newexposed
keyword, or use a variation C#'s{ get; private set; }
I slightly prefer the keyword approaches, since in C# there's a difference between properties and fields, and using the property syntax on TypeScript fields could be viewed as a syntactic conflation with the existing
get
/set
properties.Note that there are prior, mostly-declined issues vaguely related to this. #2845 is the closest by asking for different access modifiers for getter and setter. I think this is a better approach for simulating C#'s
{ get; private set; }
This is also different from the
readonly
andimm
proposals. The goal here is to allow changing the property with less code. Arguably, allowing the language to prefer directly accessing properties actually embraces the third and fourth design goals by removing the need to rely on methods to guarantee property privacy and creating more streamlined output JavaScript.The text was updated successfully, but these errors were encountered: