tc39 / proposal-class-fields

Orthogonally-informed combination of public and private fields proposals
https://arai-a.github.io/ecma262-compare/?pr=1668
1.72k stars 113 forks source link

Private fields/properties & the membrane pattern... #158

Closed rdking closed 5 years ago

rdking commented 6 years ago

Ok. Can someone explain to me what the difficulty is with the membrane pattern and private fields, because I'm not sure that I understand the problem at all. For me, it seems fairly obvious that the correct choice is to tunnel private field access through Proxy. If private fields are supposed to be completely transparent to running code other than Function.prototype.toString, then Proxy should forward all requests involving a "PrivateName" through to the corresponding Reflect handler.

So what am I missing?

I get that there's some concern that tunneling will expose private fields and allow data to pass through the membrane unwrapped, but I just don't see how.

class Ex {
  #foo = { foo: "bar"};
  method() { return this.#foo; }
}
var mHandler = {
  data: new WeakMap(),
  get(target, prop, receiver) {
    var retval = Reflect.get(target, prop.receiver);
    if (prop == "isProxy") {
      retval = true;
    }
    else {
      retval = new Proxy(retval, mHandler);
    }
    return retval;
  },
  set(target, prop, val, receiver) {
    if (val && ["object","function"].includes(typeof(val))) {
      val = new Proxy(val, mHandler);
    }
    return Reflect.set(target, prop, val, receiver);
  },
  apply(target, thisArg, args) {
    var _this = mHandler.data.get(thisArg) || thisArg;
    for (var i=0; i<args.length; ++i) {
      if (!args[i].isProxy) {
        args[i] = new Proxy(args[i], mHandler);
      }
    }
    var retval = Reflect.apply(target, _this, args)
    if (retval && ["object","function"].includes(typeof(retval))) {
      retval = new Proxy(retval, mHandler);
    }
    return retval;
  }
};
const MEx = new Proxy(Ex, {
  construct(target, args, newTarget) {
    var result = Reflect.construct(target, args, newTarget);
    var retval = new Proxy(result, mHandler);
    mHandler.data.set(result, retval);
    mHandler.data.set(retval, result);
    return retval;
  }
});

var ex = new MEx();
console.log(`ex.foo = ${JSON.stringify(ex.method(), null, '\t')}`);
console.log(`ex is a proxy = ${!!ex.isProxy}`);

If private fields were allowed to tunnel, I don't see how I'd get back anything other than:

ex.foo = {
  foo: "bar"
}
ex is a proxy = true
jridgewell commented 6 years ago

Cross posting https://github.com/zenparsing/proposal-private-symbols/issues/7#issuecomment-431632963:

If you take {} (an object) from one side of a membrane, it cannot be strict equal to the object that is returned by accessing an the object through membrane. This is the blue-yellow semantics enforced by membranes:

const obj = {
  prop: {}
};

const membrane = new Membrane(obj);
assert.notEqual(obj.prop, membrane.prop);

There must be some way for the proxy to either trap the property and wrap the value, or the proxy must throw. Either is acceptable for blue-yellow.

Now imagine using privates across the membrane (this requires a reified PrivateName, or a private symbol to be passed through):

const priv = Symbol.private();
const obj = {
  [priv]: {},
  symbol: priv
};

const membrane = new Membrane(obj);
// pass the private through
const priv = membrane.symbol;

// Private access is non-trapable, using transparent semantics
// This is a break in blue-yellow
assert.equal(obj[priv], membrane[priv]);
rdking commented 6 years ago

@jridgewell Thanks for the explanation. I was under the impression that when it comes to Private Names, none but the declaring class and any decorators used on a private field will ever have access to it. Further, even if a decorator did pass along a PrivateName, it would still be useless outside the decorator or class. Since a membrane won't affect a decorator (decorators are declaration time while membrane is instance time), how is it that there's a worry over the use of PrivateNames?

rdking commented 6 years ago

Btw, I get the scenario problem for Private Symbols. What I'm not able to comprehend is why this is a problem for allowing a class containing private fields to operate properly while allowing the private field accesses to tunnel past the developer's handler.

Igmat commented 6 years ago

@rdking it's not a problem, it just requires a little bit more complex Membrane code to preserve both tunneling privates through proxy and keep blue-yellow semantic.

rdking commented 6 years ago

@Igmat If it's not a problem, then why the decision to have Proxy break on access of a private?

Igmat commented 6 years ago

@rdking, because if Proxy tunnels privates, then brand-checking out of the box no longer works.

In case of current semantics:

class A {
    #priv;
    someMethod() {
        this.#somePriv; // <- brand-check happens here and `this` is definitely built with constructor of A
    }
}

If proxy will tunnel privates, then this in previous example could be proxy. There are some rare invariants that committee wants to keep, so forces brand-checking, even though it could be added explicitly, like this:

class A {
    #brand = this;
    #brandCheck() {
        if (this.#brand !== this) throw `Brand check failed`;
    }

    brandCheckedMethod() {
        this.#brandCheck();
    }
}

Also, there were thoughts that proper Membrane is impossible with addressable Symbol.private, but since the only problem is handling situations when symbols are explicitly shared via public API, and Membrane could handle such case by implementing something like sample in https://github.com/zenparsing/proposal-private-symbols/issues/7#issuecomment-424859518 (obviously, there are a lot of stuff missed there, but if needed I can implement full private symbol handler mechanism), IMO it should be revisited.

rdking commented 6 years ago

@Igmat Maybe we have different definitions of tunneling. When I say tunneling, I mean that the Proxy will (in the case of a get call for example) test the type of the property name. If the type is a PrivateName, then it will immediately forward the call to Reflect.get(target, prop, receiver) instead of calling handler.get(target, prop, receiver), thus completely bypassing the developer's attempt to catch the call. Wouldn't that work?

Igmat commented 6 years ago

@rdking,

class A {
    #priv = 1;
    someMethod() {
        return this.#priv;
    }
}
const a = new A();
const aProxy = Proxy(a, {
    get(target, name, receiver) {
        if (name = 'somMethod') {
            return a[name].bind(aProxy);
        }
    }
});
// in case of tunneling following line will return 1
// because `this.#priv` when applied to `this` which is Proxy,
// instead of calling any traps, immediately applied to its `target`
// in case of NOT tunneling, following code will throw exception,
// since `aProxy` can't have such `#priv` field
aProxy.someMethod();

In both cases proxy traps DON'T get called when access to #priv happens.

rdking commented 6 years ago

@Igmat Of course that fails. Didn't you forget to catch the function calls?

const handler = {
  get(target, name, receiver) {
    var retval = Reflect.get(target, name, receiver);
    if (retval && ["function", "object"].includes(typeof(retval)) {
      if (typeof(retval) == "function") retval = retval.bind(receiver));
      retval = new Proxy(retval, handler);
    }
    return retval;
  },
  apply(target, thisArg, args) {
    for (var i=0; i<args; ++i) {
      if (args[i] && ["function", "object"].includes(typeof(args[i]))
        args[i] = new Proxy(args[i], handler);
    }
    var result = Reflect.apply(target, thisArg, args);
    if (retval && ["function", "object"].includes(typeof(retval))) {
      retval = new Proxy(retval, handler);
    }
    return retval;
  }
}
const aProxy = new Proxy(a, handler);

I tend to write like this. If we replace your proxy definition with this, then in the case of tunneling, you'll still get a 1, but you'll know the proxy trapped properly if you change #priv to a non-primitive. If #priv is not a primitive, then it will be wrapped. get will not trap for tunneling cases, but that's the correct behavior. Since the only way to return private data is via a public function, the appropriate trap to monitor is apply.

Igmat commented 6 years ago

@rdking you missed my point. I've skipped apply trap, because it wasn't related to what I tried to show, but seems like I failed in explaining, so here is more details.

function isPrimitive(obj) {
    return obj === undefined
        || obj === null
        || typeof obj === 'boolean'
        || typeof obj === 'number'
        || typeof obj === 'string'
        || typeof obj === 'symbol'; // for simplicity let's treat symbols as primitives
}

function createWrapFn(originalsToProxies, proxiesToOriginals, unwrapFn) {
    return function wrap(original) {
        // we don't need to wrap any primitive values
        if (isPrimitive(original)) return original;
        // we also don't need to wrap already wrapped values
        if (originalsToProxies.has(original)) return originalsToProxies.get(original);

        const proxy = new Proxy(original, {
            apply(target, thisArg, argArray) {
                thisArg = unwrapFn(thisArg);
                for (let i = 0; i < argArray; i++) {
                    if (!isPrimitive(argArray[i])) {
                        argArray[i] = unwrapFn(argArray[i]);
                    }
                }

                const retval = Reflect.apply(target, thisArg, argArray);
                return wrap(retval);
            },
            get(target, p, receiver) {
                receiver = unwrapFn(receiver);
                let retval = Reflect.get(target, p, receiver);

                return wrap(retval);
            },
            // following methods also should be implemented, but it doesn't matter
            // for problem that I'm trying to show
            getPrototypeOf(target) { },
            setPrototypeOf(target, v) { },
            isExtensible(target) { },
            preventExtensions(target) { },
            getOwnPropertyDescriptor(target, p) { },
            has(target, p) { },
            set(target, p, value, receiver) { },
            deleteProperty(target, p) { },
            defineProperty(target, p, attributes) { },
            enumerate(target) { },
            ownKeys(target) { },
            construct(target, argArray, newTarget) { },
        });

        originalsToProxies.set(original, proxy);
        proxiesToOriginals.set(proxy, original);

        return proxy;
    }
}

function membrane(obj) {
    const originalProxies = new WeakMap();
    const originalTargets = new WeakMap();
    const outerProxies = new WeakMap();

    const wrap = createWrapFn(originalProxies, originalTargets, unwrap);
    const wrapOuter = createWrapFn(outerProxies, originalProxies, wrap)

    function unwrap(proxy) {
        return proxyToOriginals.has(proxy)
            ? proxyToOriginals.get(proxy)
            : wrapOuter(proxy);
    }

    return wrap(obj);
}

It's skeleton for Membrane pattern implementation. And it doesn't have any problems whether or not privates are tunneled (there are some invariants that are broken if tunneling happens, but IMO it's related to brand-checking and not to Membrane).

Prove:

Let's assume we have following class:

class A {
    #somePriv = 1;
    #a() {
        return this.#somePriv;
    }
    b() {
        return this.#a();
    }
    #c() {
        return this.b();
    }
    d() {
        return this.#c();
    }
    e() {
        return this.d();
    }
}

Let's wrap instance of this class in Membrane and use it:

const original = new A();
const a = membrane(original);
console.log(a.e()); // prints `1`

And important here is execution flow:

  1. evaluating e, context - a outside membrane
  2. evaluating get trap, context - a outside membrane
    1. evaluating e, context - original inside membrane
    2. wrap result
  3. evaluating e(), context - a outside membrane
  4. evaluating apply trap, context - a outside membrane
    1. evaluating e(), context - original inside membrane
    2. evaluating d, context - original inside membrane
    3. evaluating d(), context - original inside membrane
    4. evaluating #c, context - original inside membrane
    5. evaluating #c(), context - original inside membrane
    6. evaluating b, context - original inside membrane
    7. evaluating b(), context - original inside membrane
    8. evaluating #a, context - original inside membrane
    9. evaluating #a(), context - original inside membrane
    10. evaluating #priv, context - original inside membrane
    11. wrap result

If privates aren't addressable they will be executed always inside of Membrane, so it doesn't matter what decision was made for tunneling, if we're talking about this pattern.

So why do we need tunneling?

But while Membrane pattern is important, it's not the only way for using Proxies. And while I've seen usages for this pattern (e.g. vm2), IMO other use cases are much more common (e.g. MobX, Vue, Aurelia), like following:

function isPrimitive(obj) {
    return obj === undefined
        || obj === null
        || typeof obj === 'boolean'
        || typeof obj === 'number'
        || typeof obj === 'string'
        || typeof obj === 'symbol'; // for simplicity let's treat symbols as primitives
}
function doSomethingUseful(...args) {
    // this function could add some useful functionality
    // e.g. logging, reactivity, and etc
}
function notMembrane(obj) {
    const proxies = new WeakMap();

    function wrap(original) {
        // we don't need to wrap any primitive values
        if (isPrimitive(original)) return original;
        // we also don't need to wrap already wrapped values
        if (proxies.has(original)) return proxies.get(original);

        const proxy = new Proxy(original, {
            apply(target, thisArg, argArray) {
                const retval = Reflect.apply(target, thisArg, argArray);
                doSomethingUseful('apply', retval, target, thisArg, argArray);
                return wrap(retval);
            },
            get(target, p, receiver) {
                const retval = Reflect.get(target, p, receiver);
                doSomethingUseful('get', retval, target, p, receiver);
                return wrap(retval);
            },
            // following methods also should be implemented, but it doesn't matter
            // for problem that I'm trying to show
            getPrototypeOf(target) { },
            setPrototypeOf(target, v) { },
            isExtensible(target) { },
            preventExtensions(target) { },
            getOwnPropertyDescriptor(target, p) { },
            has(target, p) { },
            set(target, p, value, receiver) { },
            deleteProperty(target, p) { },
            defineProperty(target, p, attributes) { },
            enumerate(target) { },
            ownKeys(target) { },
            construct(target, argArray, newTarget) { },
        });

        proxies.set(original, proxy);

        return proxy;
    }
    return wrap(obj);
}

Main difference comparing to Membrane is that context for calling methods, functions, getters, setters and everything else is always proxy.

Example

Let's assume we have following class (actually, it is the same as in section about membrane):

class A {
    #somePriv = 1;
    #a() {
        return this.#somePriv;
    }
    b() {
        return this.#a();
    }
    #c() {
        return this.b();
    }
    d() {
        return this.#c();
    }
    e() {
        return this.d();
    }
}

Let's wrap instance of this class in notMembrane and use it:

const original = new A();
const a = notMembrane(original);
console.log(a.e()); // prints `1` if tunneling happens
                    // throws if tunneling doesn't happen

And again important here is execution flow:

if privates are tunneled

  1. evaluating e, context - a
  2. evaluating get trap, context - a
    1. evaluating original e, but context - a
    2. doSomethingUseful is executed
  3. evaluating e(), context - a
  4. evaluating apply trap, context - a
    1. evaluating original e() but context - a
    2. evaluating d, context - a
    3. evaluating get trap, context - a
      1. evaluating original d, but context - a
      2. doSomethingUseful is executed
    4. evaluating d(), context - a
    5. evaluating apply trap, context - a
      1. doSomethingUseful is executed
    6. evaluating original d() but context - a
    7. evaluating #c, context - a, but since a is Proxy, it tunneled to original
    8. evaluating original #c() but context - a
    9. evaluating b, context - a
    10. evaluating get trap, context - a
      1. evaluating original b, but context - a
      2. doSomethingUseful is executed
    11. evaluating b(), context - a
    12. evaluating apply trap, context - a
      1. doSomethingUseful is executed
    13. evaluating original b(), but context - a
    14. evaluating #a, context - a, but since a is Proxy, it tunneled to original
    15. evaluating original #a() but context - a
    16. evaluating #priv, context - a, but since a is Proxy, it tunneled to original

if privates aren't tunneled

  1. evaluating e, context - a
  2. evaluating get trap, context - a
    1. evaluating original e, but context - a
    2. doSomethingUseful is executed
  3. evaluating e(), context - a
  4. evaluating apply trap, context - a
    1. evaluating original e() but context - a
    2. evaluating d, context - a
    3. evaluating get trap, context - a
      1. evaluating original d, but context - a
      2. doSomethingUseful is executed
    4. evaluating d(), context - a
    5. evaluating apply trap, context - a
      1. doSomethingUseful is executed
    6. evaluating original d() but context - a
    7. evaluating #c, context - a, THROWS

Since, there are no privates right now, libraries that use such approach are already built/in-progress (and I'm an author of such library, same as @yyx990803 (Vue.js) and @EisenbergEffect (Aurelia)), and there are NO WAY to achieve same goal and I don't want to write in my readme:

NOTE: this library can't be used together with ESnext privates

I don't want to restrict otherwise normal intent of my users to encapsulate implementation details, only because somebody decided that encapsulation without brand-checking isn't perfect.

P.S.

Decorators aren't proper replacement.

ljharb commented 6 years ago

@Igmat note that internal slots on builtins also don’t tunnel, so this is already an existing problem - or non-problem, depending on your perspective. Private fields only increase the potential number of objects it can occur on.

Igmat commented 6 years ago

@ljharb thanks for pointing it again and again. I do know about internal slots, but user can't add them to his/her classes (at least, I'm not aware of any common possibilities for it), so only thing I need to do is to add special-case handling for such objects from standard library.

rdking commented 6 years ago

@Igmat

If privates aren't addressable they will be executed always inside of Membrane, so it doesn't matter what decision was made for tunneling, if we're talking about this pattern.

This is 1 part of the reason why I raised the question.

Main difference comparing to Membrane is that context for calling methods, functions, getters, setters and everything else is always proxy.

I'm aware of this as well. I make use of this fact quite often. As for your execution flow in the tunneled case, I was expecting something different at 4.viii. Tunneling the way I conceived it would mean the context would be original, nota`. This would mean that a private function's actions are just as invisible to the proxy as the function itself. However, from where I sit, either approach is good.

What I still don't see is why this has anything to do at all with brand-checking? Isn't just a duck-type test to see if a particular PrivateName is known to a class instance?

ljharb commented 6 years ago

The user can extend builtins, which allows their own class instances to have those very internal slots.

shannon commented 6 years ago

@ljharb Even if they extend builtins it's easy enough to do instanceof <builtin> and code around a small set of well known and defined builtins right?

ljharb commented 6 years ago

@shannon no, instanceof doesn't work cross-realm; it's simply not a reliable mechanism.

shannon commented 6 years ago

@ljharb Ok, are there are other cross realm ways to detect these builtins? Similar to https://github.com/ljharb/is-typed-array

ljharb commented 6 years ago

Nothing generic; all of the "is-" modules I publish are built for precisely that reason - to provide production-ready solutions to cross-realm brand-checking.

shannon commented 6 years ago

Yeah sorry, I get that. But if you have a small set of well defined objects it's not quite the same as an arbitrary number of arbitrary objects that would be introduced with privates. If you are careful you may be able to work around all the built-ins. I just don't know if I would say the problem already exists.

ljharb commented 6 years ago

I'll agree it's a different scale. Keep in mind there's language and browser/engine builtins to consider, all of which might have internal slot-like behavior. As for user objects, if they're already using a WeakMap to simulate private fields, the same problems would occur on an arbitrary number of arbitrary objects.

EisenbergEffect commented 6 years ago

I wouldn't say the problem already exists. Scale is essential to consider in this case. Once private fields are introduced everyone is going to start using them everywhere. That's going to greatly increase the chance of issues. While Aurelia and its community can potentially work around this a bit and provide guidance, it's going to be painful and we'd still prefer a solution that allows tunneling or something like it. I also think that a less error-prone integration of private field and Proxy is better for the language and the average JavaScript developer.

Call me crazy, but if branding and cross-realm detection of brands is the main thing holding back something like tunnelling, then perhaps these concerns should be separated and a specific solution to the cross-realm brand problem should be proposed. It seems to me that there's a conflation of issues which could be avoided.

ljharb commented 6 years ago

@EisenbergEffect note that I’m fully in favor of figuring out a way to address this tunneling issue - i just don’t think it needs to block class fields from progressing, since any solution to it will also handle all those other cases.

rdking commented 6 years ago

@ljharb Of all the issues I and others have raised with class-fields, no single issue is enough to block class-fields from progressing. On this, we agree. What's bothersome, however, is that the amalgamation of all of those issues doesn't appear to be enough in the perspective of the proponents. But let me not drift off topic...

I get that Proxy already has issues with handling built-ins other than Array. What does that have to do with tunneling private fields? From what I've read in here, the issue of tunneling has nothing to do with the issue of brand-checking. If a Proxy properly tunnels access to private fields, then it is responding to them in the same way as the target object would, making any attempt to brand-check the proxy equivalent to brand-checking the target.

So if that's not right, what did I miss?

ljharb commented 6 years ago

If you do brand checking now, with WeakMaps, i believe you’d run into the same issue - the receiver would be the proxy, which isn’t in the weakmap, and the brand check would throw.

Igmat commented 6 years ago

@ljharb, I said that I'm not aware of common possibilities.

The user can extend builtins

This isn't common use case.

instanceof doesn't work cross-realm

Even though this is true, it doesn't dismiss the fact that only thing I have to do in order to workaround issue with InternalSlots is to handle several special cases and as @EisenbergEffect has already mentioned, scale is essential.

if they're already using a WeakMap to simulate private fields

This is also very uncommon practice - in most cases users simulate private state (actually soft private) using Symbols, which is much simpler and fits most of their needs.

And even though I agree that hard private > soft private, so encapsulation provided by this proposal should be hard, I can't agree that WeakMap semantic should be used as-is, because it introduces brand-checking which has its own, separate from encapsulation, use cases and will surprise users when their intent was to hide encapsulation details, but not brand-check their classes.

@rdking,

making any attempt to brand-check the proxy equivalent to brand-checking the target.

First of all, this is problem for brand-checking. Proper brand-check should throw, when brand-checked method called on proxy and not instance for which it was designed. Second, tunneling of privates doesn't lead to such equivalent, it only leads to that brand-checking should be more explicit, like here:

class A {
    #brand;
    constructor() {
        this.#brand = this;
    }
    brandCheckedMethod() {
        if (this.#brand !== this) throw "Brand check is failed";
        // do some useful stuff, knowing that you work with
        // direct instance of A and proxy around such instance
    }
}

or using WeakMap/WeakSet.

shannon commented 6 years ago

If it helps. When I need to workaround the Proxy / WeakMap issue I use an extended WeakMap and a specialized Proxy that allows me to tunnel to the target.

Something like this:

const targetMap = new WeakMap();
function createProxy(target, handler) {
    const proxy = new Proxy(target, handler);
    targetMap.set(proxy, target);
    return proxy;
}

class ProxyWeakMap extends WeakMap {
    get(key) {
        key = targetMap.get(key) || key;
        return super.get(key);
    }

    set(key, value){
        key = targetMap.get(key) || key;
        return super.set(key, value);
    }

    has(key) {
        key = targetMap.get(key) || key;
        return super.has(key);
    }

    delete(key) {
        key = targetMap.get(key) || key;
        return super.delete(key);
    }
}

const obj = {};
const map = new ProxyWeakMap();
const proxy = createProxy(obj, {});

map.set(proxy, true);

console.assert(map.has(obj) === true);
console.assert(map.has(proxy) === true);
rdking commented 6 years ago

@ljharb

If you do brand checking now, with WeakMaps, i believe you’d run into the same issue - the receiver would be the proxy, which isn’t in the weakmap, and the brand check would throw.

I do this all the time with no issue. That's not to say it works effortlessly. I intercept construct and replace newTarget with a Proxy around newTarget.prototype. This gives me the access I need to intercept attempts to get/set private members in the constructor. It also lets me peek in on attempts to get public properties before set is called against one of them, which allows me to defer setting them on the instance where I can't see them until after the constructor returns an instance to my trap. After the instance returns, I fix the prototype of the instance back to newTarget.prototype, stage the WeakMap against the instance, wrap the instance in a Proxy, and return that Proxy instance. No issues whatsoever.

This is how I do private fields via WeakMap, and also one of the reasons why I've been campaigning so hard against the "field" concept. If it's not part of the prototype, I can't see it in a Proxy while the constructor is running. As I've often repeated, there are good use-cases for data properties on prototypes.

erights commented 5 years ago

@rdking @Igmat @jridgewell @hax @littledan @ljharb @isiahmeadows @zenparsing @caridy @bakkot , sorry for the long delay!

I am trying to understand what semantics is being proposed for private symbols. Please forget proxies and membranes for the moment while we clarify some more basic things. What does the following code do:

const x = Symbol.private();
const y = Symbol.private();
const z = Symbol.private();

const obj = Object.freeze({
  [x]: 1
});

// assume the following are tried separately, say at a command line,
// so that if one throws the next is still tried.

obj[x] = 2;
obj[y] = 3;
typeof z;
Object(z) === z;

Thanks.

rdking commented 5 years ago

@erights From what I understand of private-symbols:

obj[x] = 2; //gets silently ignored, or throws if strict mode is on
obj[y] = 3; //gets silently ignored, or throws if strict mode is on
typeof z; //I don't know, probably returns "symbol"
Object(z) === z; //returns false. A private symbol is a primitive

In general, unless I'm mistaken, private symbols are identical to public symbols with the exception that no attempts to reflect properties created with them will work. They don't even show up in getOwnPropertySymbols. Proxy handler functions also forward access attempts against private symbols directly to the engine invariant function, bypassing the proxy handler.

erights commented 5 years ago

Doesn't

obj[x] = 2; //gets silently ignored, or throws if strict mode is on

make private symbols useless for encapsulated mutable instance state of frozen instances?

erights commented 5 years ago

Btw, I should apologize once again for the name Object.freeze. It is not about primarily immutability. It is about tamper-proofing API surface, in order to enable defensive objects. It is about immutability for some special cases, but those cases should not include objects representing encapsulating abstractions. Rather, enabling anyone to use freeze to prevent those objects from mutating their encapsulated state violates their encapsulation, and thereby destroys the integrity of the abstraction they represent.

dead-claudia commented 5 years ago

@erights

Doesn't

obj[x] = 2; //gets silently ignored, or throws if strict mode is on

make private symbols useless of encapsulated mutable instance state of frozen instances?

Not really. If you need mutable state that persists through an object's "frozenness" (for lack of a better term), you can just set the field's value to a mutable child object before freezing the host object, like this:

const x = Symbol.private()

const obj = Object.freeze({
  [x]: {value: 1}
})

obj[x].value = 2
console.log(obj[x].value) // 2, not 1

As for your question:

obj[x] = 2;      // Throws in strict mode, no-op in sloppy mode
obj[y] = 3;      // Throws in strict mode, no-op in sloppy mode
typeof z;        // Returns "symbol"
Object(z) === z; // Returns `false`

It's basically a symbol in every respect other than visibility. This even extends to Symbol.private("foo").toString() === "Symbol(foo)", which would evaluate to true. The only real difference is that it's not exposed to proxies or Reflect.ownKeys/etc. You can get and set them, but you can't iterate them or otherwise discover them without the creator of the symbol allowing it.

(As an exception, an engine might choose to expose an API for debugging and inspection, but this would not exist in the spec and is beyond the scope of this proposal.)

dead-claudia commented 5 years ago

@erights You could make an argument for excluding private symbols from the freezing process, though, and they'd become a lot more weak map-y. The more I think about that possibility, the more I think it might be a good idea to special-case that.

/cc @zenparsing, who might have a differing opinion. (It's his proposal.)

erights commented 5 years ago

@rdking @isiahmeadows I'm glad I asked. Your first answer surprised me, but there's nothing terribly wrong with @isiahmeadows 's solution. @isiahmeadows 's second answer is what I expected, and it leads to many more questions. Thanks.

dead-claudia commented 5 years ago

@erights Edit: they're excluded from [[OwnPropertyKeys]] via an invariant, so actually, the first one, obj[x] = 2, would work as advertised. It's the second one, the obj[y] = 3, that needs an exception (carved out in various places spread out all over the place - just search for IsExtensible( and it's easy enough).

I forgot about how we made them invisible to Reflect.ownKeys and friends. (@zenparsing and I independently came up with basically the same proposal, and we only found out a few weeks later the other had a nearly identical proposal created only days apart.)

dead-claudia commented 5 years ago

The reason why the first one, obj[x] = 2, works is because when getting the list of properties to freeze, private symbols are skipped and so Object.freeze doesn't see them to freeze in the first place. It's just that the new one, obj[y] = 3, triggers an IsExtensible check that fails, so it ends up throwing the error. (That's what I was suggesting we change.)

erights commented 5 years ago

From the range of answers, it sounds like you find it at least plausible that any of these answers may be compatible with your goals for this proposal, and the proposed reconciliation of private symbols with membrane security. Do I have that right?

Is it important for the goals of this proposal that private symbols be symbols? What would change if they were, instead, a special kind of object? Could it be a PrivateName-like object?

erights commented 5 years ago

On the proposed reconciliation with membranes, I think I have seen several variations and do not know what the current state of thinking is. Let's call a membrane operating according to this reconciliation an rMembrane, unless someone has a better suggestion.

const target = function...;
const x = Symbol.private();
...
const rMembrane = makeRMembrane(target, ...);
rMembrane(x);

What happens when a private symbol is passed as an argument through an rMembrane?

rdking commented 5 years ago

Why would making obj[y] = 3 work require changes all over the place? Wouldn't it be as simple as a check in IsExtensible to see if the key being used is a private symbol? If `IsExtensible were passed an optional extra parameter that contained either null or a key, it could test that key. If the key is a private symbol it would return true early, otherwise it would continue its normal function.

dead-claudia commented 5 years ago

@erights I'll defer to @zenparsing for actual intent, but the general idea was that private symbols are very much like standard symbols, just symbols you can't observe the presence of. Private symbol accesses would run through proxies as normal, but they wouldn't trigger any calls to handlers (to avoid discoverability). This doesn't address membranes' weak map problem, but just gives an idiomatic escape hatch for classes to avoid screwing with proxies.

In theory, you could adjust weak maps to operate based on synthetic, internal, per-instance private symbols, resolving the weak map issue, but that's out of scope of the private symbol proposal.

const target = function...;
const x = Symbol.private();
...
const rMembrane = makeRMembrane(target, ...);
rMembrane(x);

What happens when a private symbol is passed as an argument through an rMembrane?

It just gets passed as an argument. It dodges proxy handlers as a key, not as a value. Sorry if that wasn't clear enough.

dead-claudia commented 5 years ago

@rdking IsExtensible only accepts the object itself - it works independently of key. In addition, most of the changes that would need made would have to do more than just operate as if that call returned true - it needs to skip other method calls and internal operations as well. It's the code surrounding various usages of IsExtensible that need changed, not the operation itself.

erights commented 5 years ago

Sorry if that wasn't clear enough.

It wasn't, but that's probably because I missed it. In any case, thanks for clarifying. What about

Reflect.defineProperty(rMembrane, x, {...});
Reflect.getOwnPropertyDescriptor(rMembrane, x);
dead-claudia commented 5 years ago

@erights Those are equivalent to doing the same to target (as opposed to rMembrane).

erights commented 5 years ago

That is what people mean by "tunneling"?

Another terminology apology: The terms "target" "real target" and "shadow target". These were a fossil left over from an earlier stage in figuring out the transition from the first proxy proposal to the direct proxies that we standardized on. For purposes of this thread (and as often elsewhere as I can manage) I will avoid the unqualified term "target" and use only "shadow" and "real target". The proxy mechanism knows only about shadows. In this terminology, just to restate,

Those are equivalent to doing the same to shadow (as opposed to rMembrane).

This leaves the question about how the rMembrane keeps the shadow and the real target in sync. IIUC, the idea is that an accessor property be installed on the shadow, whose getter reads from the real target and whose setter updates the real target. Is this right? Is this accessor named with the same private symbol?

dead-claudia commented 5 years ago

Visibly, yes, this would be the correct understanding.

erights commented 5 years ago

Thanks. Curious why the qualifier "visibly". Is there another level of abstraction, other than implementation, at which something else is going on?

What causes the rMembrane to install this accessor on a shadow? What shadow does it install this accessor on?

If the shadow is itself a proxy, does it further tunnel through to that proxy's shadow, etc, like, for example, isArray does?

erights commented 5 years ago

Hold on. Back to

Reflect.getOwnPropertyDescriptor(rMembrane, x);

This operation is not normally supposed to invoke a getter. Rather, it is supposed to reveal the descriptor containing both the getter and setter, without invoking them. So I remain confused about how the rMembrane is supposed to keep the shadow and real target in sync, if this operation tunnels without causing a handler trap. I don't see how an accessor property could fix it.

dead-claudia commented 5 years ago

@erights

Thanks. Curious why the qualifier "visibly". Is there another level of abstraction, other than implementation, at which something else is going on?

How I view proxies is more like an exotic view of an object than an object with its own properties computed from another object. How I see it, proxies present a view (your "real target") of a target (your "shadow target"), and so my answer was reflected based on that. It's a lot like a map that delegates all its methods to a child map.

This should also explain my choice of terminology, if it helps.

What causes the rMembrane to install this accessor on a shadow? What shadow does it install this accessor on?

The same process that leads ordinary properties to get installed on the shadow. The only difference is that it acts as if all the handlers that would otherwise be called are just not present.

If the shadow is itself a proxy, does it further tunnel through to that proxy's shadow, etc, like, for example, isArray does?

Yes.

rdking commented 5 years ago

Just so I don't get confused while trying to follow:

dead-claudia commented 5 years ago

@erights

Hold on. Back to

Reflect.getOwnPropertyDescriptor(rMembrane, x);

This operation is not normally supposed to invoke a getter. Rather, it is supposed to reveal the descriptor containing both the getter and setter, without invoking them. So I remain confused about how the rMembrane is supposed to keep the shadow and real target in sync, if this operation tunnels without causing a handler trap. I don't see how an accessor property could fix it.

This is where we intentionally keep them out of sync. Keeping the proxy handler and shadow target out of sync here is by design.