-
Notifications
You must be signed in to change notification settings - Fork 19
Dynamic Access of Private FIelds #104
Comments
@wycats because the observer pattern is critical to ember, I'm wondering if you had any thoughts on this. |
If you want dynamic access, then store a Map in a single private field, and you can use anything you like as the key. I don't see why enabling direct dynamic access of private fields is desired or cleaner, given that this is an easy option |
You may be interested in the decorators proposal, which has an integration with this proposal here. I think it would let you accomplish approximately what you've laid out here with a single class decorator, and I think is a better route for this sort of metaprogramming. (It also includes a |
@bakkot
The second issue is the biggest in my eyes since property access is everywhere, and last time I did the perf test using getters and setters added a significant penalty. Getters and setters have their place, just not when it comes to property observation IMO.
That is a decent workaround, for the case of observable fields, would not work for debugging private fields though.
It's not necessarily always easy, sure from the ground it is. If your modifying existing code that uses private fields into a single map, that's going to be a real pain. It's desired because it allows more flexible development, and from where I can see doesn't present any cons that weren't already there. |
The same argument could be made for adding private fields since there are other solutions (like using WeakMaps) that are no different than your suggestion of using a Map. Just about any argument you could make against using a WeakMap to store private data would also have an analog for using a Map to make for dynamic access of private fields. @bakkot |
There are indeed reasons why WeakMaps are insufficient for private fields; none of them apply to what this issue is asking for. |
The desire for hard privacy is very much real and mandatory to prevent various things like those that come up somewhat often with people potentially misusing/relying on internal APIs in Node (an example from just a few days ago nodejs/CTC#164 ). I have no desire for soft private state when Symbols already fulfill that use case. |
@bmeck dynamic access does not equate to soft privacy. Private fields (soft or hard) will also not remove the necessity of having internal api's that are actually public, with the expectation that they are for internal use only. It is already possible in some scenarios to use something like rollup to remove access to internal apis. |
Maybe I am not understanding then:
Appeared to be stating the intent here is to allow soft privacy by exposing the private fields for the 2 use cases mentioned. Is that incorrect? I am not concerned as much about exposing things via public get/set mechanisms as that is possible today by creating stuff similar to your example above. |
Only the second is intended to allow soft privacy, as opt in, and primarily for library maintainers with the expectation that such if statement would be striped from production libraries. |
@robbiespeed One option available today is to use helpers similar to what you had above without needing to reify a class Cat {
#hunger = 0;
constructor () {
if (ENV.DEBUG_PRIVATE) {
// Get uses the observable `get` method to virtualize storage
this.__getPrivate = (privateKey) => {
// last argument specifies location
if (privateKey === 'hunger') return get(this, 'hunger', 'private');
};
// if not using an observer library
this.__getPrivate = (privateKey) => {
if (privateKey === 'hunger') return this.#hunger;
};
// Set uses the observable `set` method
this.__setPrivate = (privateKey, value) => {
// last argument specifies location
if (privateKey === 'hunger') return set(this, 'hunger', value, 'private');
};
// If not using a observer library
this.__setPrivate = (privateKey, value) => {
// last argument specifies location
if (privateKey === 'hunger') return this.#hunger = value;
};
}
// last argument specifies location
observe(this, 'hunger', () => {
if (this.__getPrivate('hunger') > 10) {
this.growl();
}
}, 'private')
setInterval(() => {
this.__setPrivate('hunger', this.__getPrivate('hunger') + 1);
}, 1000);
}
growl () {
console.log('Meeooow');
}
eat (food) {
this.__setPrivate('hunger', this.__getPrivate('hunger') - food.value);
}
}
const cat = new Cat();
// Private fields still directly innaccessible from outside class body
cat.#hunger; // throw error
cat.#hunger = 2; // throw error
cat.__getPrivate('hunger'); // return 0
cat.__setPrivate('hunger', 10); // return 10
// log > Meeooow
cat.eat({ name: 'fish', value: 3 });
cat.__getPrivate('hunger'); // return 7 Edit: everything should be using |
@robbiespeed, I wouldn't generalize from performance characteristics of regular properties to performance characteristics of private fields. Among other things, private fields are a lot more static. Also, I'm pretty sure a class-level decorator would allow you to decorate every field without needing to explicitly mark which ones were observed (though I cannot imagine finding this to be good programming practice). Anyway, if it's just for debugging, you might as well just use class Foo {
#a; #b;
constructor() {
if (debug) this.debug = name => eval(`this.#${name}`);
}
} |
@bmeck I think the issue you linked (in particular this comment), has given me a better understanding of your concerns. I can't see anything specific about dynamic private fields that would cause leakage of node internals, unless a leak was explicitly defined. @bakkot I think you are right a class level decorator could work for libraries that need to declare fields as observable. I honestly had not thought of |
OK, I'm confused. I thought the whole thing you were asking for was to be able to do a function call when writing to or reading from a field?
Mine too, but we talked about this a while ago. Committee consensus was that
What's wrong with class Foo {
#a; #b;
constructor() {
if (debug) this.debugSet = (name, val) => eval(`this.#${name} = val`);
}
} ? |
My apologies, nothing wrong with that.
Writing to yes, reading from no. I don't want added access time, but added write time is fine, hence the use of the |
I see a number of solutions presented to the filer's problem in this thread; it seems like there's not an extremely strong case for adding another feature to the proposal. |
It's stated in the FAQ that
this.#x !== this['#x']
this makes sense in the same way thatthis.x.y !== this['x.y']
, however that means we currently have no way of dynamic access.That answer also gives the argument that dynamic access "is contrary to the notion of 'private'". This is not true at all, of course private fields shouldn't be dynamically accessed outside of the class body, but inside there are many reasons it can be valuable.
Not Possible Without Dynamic Access
I want to be clear here, I'm not advocating that private fields should be settable or retrievable from outside the class body, unless the library maintainer explicitly adds such feature.
Observer pattern
If I library were to have
set(obj, key, value)
andobserve(obj, key, callback)
methods there would be no way to internallyobserve
orset
private fields, this would necessitate anyone using such library to have all fields public.Intentional Escape Hatch For Debug and Tests
Possible Solutions
I would like to open this up to discussion, I don't quite know myself the best way to make this possible, or all the concerns people have about introducing such a feature.
Just to get things started though one possibility might be a new Private Key type. A example might look something like:
Open to better ideas for the syntax
The text was updated successfully, but these errors were encountered: