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

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

Closed
rdking opened this issue Oct 22, 2018 · 99 comments
Closed

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

rdking opened this issue Oct 22, 2018 · 99 comments

Comments

@rdking
Copy link

rdking commented Oct 22, 2018

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
Copy link
Member

Cross posting zenparsing/proposal-private-symbols#7 (comment):

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
Copy link
Author

rdking commented Oct 23, 2018

@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
Copy link
Author

rdking commented Oct 23, 2018

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
Copy link

Igmat commented Oct 23, 2018

@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
Copy link
Author

rdking commented Oct 23, 2018

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

@Igmat
Copy link

Igmat commented Oct 23, 2018

@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 zenparsing/proposal-private-symbols#7 (comment) (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
Copy link
Author

rdking commented Oct 23, 2018

@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
Copy link

Igmat commented Oct 23, 2018

@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
Copy link
Author

rdking commented Oct 23, 2018

@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
Copy link

Igmat commented Oct 24, 2018

@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
Copy link
Member

ljharb commented Oct 24, 2018

@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
Copy link

Igmat commented Oct 24, 2018

@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
Copy link
Author

rdking commented Oct 24, 2018

@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, not a`. 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
Copy link
Member

ljharb commented Oct 24, 2018

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

@shannon
Copy link

shannon commented Oct 24, 2018

@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
Copy link
Member

ljharb commented Oct 24, 2018

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

@shannon
Copy link

shannon commented Oct 24, 2018

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

@ljharb
Copy link
Member

ljharb commented Oct 24, 2018

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
Copy link

shannon commented Oct 24, 2018

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
Copy link
Member

ljharb commented Oct 24, 2018

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
Copy link

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
Copy link
Member

ljharb commented Oct 25, 2018

@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
Copy link
Author

rdking commented Oct 25, 2018

@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
Copy link
Member

ljharb commented Oct 25, 2018

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
Copy link

Igmat commented Oct 25, 2018

@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
Copy link

shannon commented Oct 25, 2018

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
Copy link
Author

rdking commented Oct 25, 2018

@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
Copy link

erights commented Dec 30, 2018

@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
Copy link
Author

rdking commented Dec 30, 2018

@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
Copy link

erights commented Dec 31, 2018

@rdking Many people have asked us why "wet"/"dry" or "blue"/"yellow"? Even when talking among ourselves in the same room with whiteboard, when we said things like "opposite side" we always meant the other opposite. I know I still sometimes do it myself anyway, but I try not to.

I don't think of a membrane's proxy as being any kind of target, so I'm still puzzled.

@erights
Copy link

erights commented Dec 31, 2018

Wouldn't that blow the whole point of having a private symbol? Think about the work done for class-fields.

That's exactly what I'm exploring: what is the point this proposal is actually trying to achieve, and what are incidental points that are coming along for the ride, perhaps because we had not known how to unbundle them?

Will Proxy be able to trap operations against a private field's reified name?

This is actually one of the points that got me thinking again in these directions. Somewhere, on one of these threads, someone (perhaps you?) pointed out that the non-membrane use of proxies to keep track of state change does not have a problem with WeakMaps, because it would wrap the WeakMap in a proxy, and so see the updates to the WeakMap. I suggest that it should be so as well for PrivateName-like special objects.

If anyone knows where I may have seen this point, please post the link. Thanks.

@rdking
Copy link
Author

rdking commented Dec 31, 2018

That's close to what I said. Not quite right though. Go look here to see what I meant. There is a way in user-land code to make Proxy and WeakMap compatible. It essentially unwraps the Proxy before handing it off to WeakMap. Think of it as an indirect Membrane around WeakMap.

@rdking
Copy link
Author

rdking commented Dec 31, 2018

Here's the link to the original comment.

@erights
Copy link

erights commented Dec 31, 2018

I may be confusing two things, but I think the one I'm thinking of was less invasive --- putting proxies around individual WeakMap instances rather than replacing the whole WeakMap class.

@erights
Copy link

erights commented Dec 31, 2018

I only remember seeing a prose description, not code. And I only vaguely remember that :(

@rdking
Copy link
Author

rdking commented Dec 31, 2018

I did mention it one other time before this, but the post I linked to is the first time where I revealed the code that makes it work.

@erights
Copy link

erights commented Dec 31, 2018

Link please?

@erights
Copy link

erights commented Dec 31, 2018

In any case, whether we dereference my vague and possibly misunderstood memory or not, the important question is, would this work and satisfy the goals?

I don't really understand the constraints of this data-binding use case for proxies. For WeakMap-based private state, could you just put a proxy around weakmap instances in order to observe the updates there? If so, would the same work for PrivateName-like special objects?

@rdking
Copy link
Author

rdking commented Dec 31, 2018

@erights Wow. It's amazing how hard it is to find a single post in these threads. When I find it again, I'll post the link here. I could implement that logic I showed you using Proxy, but it would take more work than the way I did it. The logic I created could easily be adapted to transform such objects into keys. I can take the logic I've already created and whip up an example if you'd like.

@erights
Copy link

erights commented Dec 31, 2018

I can take the logic I've already created and whip up an example if you'd like.

That would be awesome, thanks!

It should really help us figure out what we would technically lose if we move from a) your private symbols or b) tc39's current stage 3 proposal to c) PrivateName-like special objects with name-side faulting (which needs a better name than PNLSOWNSF ;) ).

Process-wise, I don't deny that any such change in direction would be painful and difficult. But I certainly agree we should explore the technical issues first, and build any case from there.

@rdking
Copy link
Author

rdking commented Dec 31, 2018

@erights You know what's funny? I remembered it so vividly because it was the last post in this thread before you started posting today!

@erights
Copy link

erights commented Dec 31, 2018

Yes, that was it! Rereading it, I still don't understand all of it, but the central point is there.

@rdking
Copy link
Author

rdking commented Dec 31, 2018

Basically, if I wrap the Proxy constructor and map all created Proxy objects back to the RT, provide a has() function to check to see if an object can be unwrapped, and an unwrap function to retrieve the RT, then extend WeakMap so that each of the CRUD methods calls Proxy.unwrap() for each object that passes Proxy.has(), then all code that uses the extended WeakMap and wrapped Proxy will simply work. Since both WeakMap and Proxy can be replaced with their alternates, no code in the wild need be augmented to gain this capability.

@rdking
Copy link
Author

rdking commented Dec 31, 2018

Wait. I just re-read my own post without the distractions around me. That's a different concept. In that post I was referring to how I use Proxy to peek in on a constructor's actions. With that kind of logic, I can catch any public property being set. What I can't do, however, is observe any private actions. This kind of code would not be able to catch the setting of private symbols. I'll work this into the example code and post a link here when it's finished.

@Igmat
Copy link

Igmat commented Dec 31, 2018

It seems that I missed very important conversation while was asleep.

@erights, it seems that part of the conversation and your questions here is very related to #183.
I propose to keep track with Membrane implementation for Symbol.private there (especially, since I've updated my implementation, big thanks to @jridgewell with his help in test cases and pointing out some edge cases I missed). so it preserves invariants that you were talking about in #149 (comment). has check is still in progress, but I assume it's not as important, because you haven't even mentioned it in your presentation

Answering some of your question from this thread:

For WeakMap-based private state, could you just put a proxy around weakmap instances in order to observe the updates there?

No, we can't. The only way to fully workaround it is monkeypatching of WeakMap class in a way which will return same object for object and proxiedObject.
There is also partial workaround - force library users to extend all their classes from one we provided with a lib, this class won't add any methods/fields/properties (so, at first glance, there is no sense to extend from it), but it will return our data-binding proxy from its constructor, so all issues with WeakMap will be solved, because this always refers to proxy.
Unfortunately, such way is both undesirable (we have to force our users to make their class hierarchy in specific way) and doesn't provide same functionality (our users don't always have an option to change a very base class).

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

With my implementation of a Membrane all shadow targets got new get/set pair for such private symbol, so they'll be able to properly do wrap/unwrap/revoke operations with this symbol and real target.

I don't really understand the constraints of this data-binding use case for proxies.

The real constraint is that we need to use proxiedObject instead of real object as this in all our handlers. I've tried to explain it earlier in this thread in #158 (comment) in part titled with So why do we need tunneling?. Execution flow and differences with between tunneling/not tunneling are precisely described there too.
Do you need more real-life example than that?

@rdking
Copy link
Author

rdking commented Jan 2, 2019

@erights This demo should reasonably describe the behavior of private symbols, tunneling Proxies, Proxy-safe WeakMaps, and how to snoop in on what the constructor is doing to its context object.

The one thing you'll notice is that private symbols simply don't show up anywhere outside where they are defined, unless they are leaked. The only odd things about my implementation of private symbols are that:

  1. Private symbol is an object that returns a Symbol as its valueOf().
  2. That object returns the internal Symbol when you call toString() to prevent the return value of valueOf from being flattened into a string when used as a key in [].

@littledan
Copy link
Member

littledan commented Jan 2, 2019

I have to say, I am very confused by this thread. I pursued WeakMap semantics partly because @erights has repeatedly said over the past 4-5 years that it is the only acceptable semantics for private. I wanted to get consensus in committee, and that included consensus with @erights' requirements. I summarized some reasons that I thought were very important for the committee about this choice at https://gist.github.com/littledan/f4f6b698813f813ee64a6fb271173fc8 -- I don't see why this particular aspect of membrane-ability invalidates any of those reasons.

In the end, has this all been about Symbols being primitives rather than objects?

@hax
Copy link
Member

hax commented Jan 2, 2019

has this all been about Symbols being primitives rather than objects?

@littledan If I understand correct, yes. And strictly speaking, it's about "primitives can not be proxied" (but PrivateName can). But @Igmat invent a trick to overcome it. And adding "whitelist of private symbols" for proxies can also solve it. (It seems @jridgewell would prefer that way?)

@rdking
Copy link
Author

rdking commented Jan 2, 2019

@hax @littledan
From what I've been able to gather, part of the issue was that private symbols can pass through membrane boundaries unwrapped since they are intended to be primitives like regular symbols. Since use of a private symbol as a key is not something that can normally be caught by a Proxy, passing a private symbol through the membrane provides a communication channel between the wet and dry sides of the membrane in which it cannot intervene.

Unless I'm mistaken, 2 solutions were forwarded for this problem:

  1. Provide Proxy with a "whitelist" of private symbols it is able to observe (as opposed to immediately tunneling).
  2. Construct a Proxy handler for Membranes that manages a "shadow-target". When the membrane detects a return value that is a private symbol, attach a getter and setter for that private symbol to the shadow target. This way, even though the Proxy cannot see the private access attempt, the getter and setter make up for the lack of visibility.
       Membrane
   ----------------
   |              |
PT - handler - ST - RT

IMO, whitelists are a nice but easily forgotten solution to the problem. If the use of a constructor to declare public instance variable can be considered a foot-gun due to ignorance of or forgetting the correct way to call super, then a whitelist for Proxy would be equally such a foot gun. It would require that for each private symbol returned from a public function of the class, that the same symbol would need to appear in the whitelist of the membrane. That doesn't seem like something people will readily remember to do. Building membranes to be aware of private symbols makes more sense, and doesn't require extra language modification to support.

While I'm more in favor of not allowing private symbols to be leaked at all, that's more of a topic for a different thread.

@hax
Copy link
Member

hax commented Jan 3, 2019

If the use of a constructor to declare public instance variable can be considered a foot-gun due to ignorance of or forgetting the correct way to call super

I never agree that 😂 . Especially it's very easy to discover the "bug" of forgetting, so it should never be treat as real "foot-gun" as my understanding of "foot-gun".

then a whitelist for Proxy would be equally such a foot gun.

Also disagree. Only membrane usage need whitelist, all other main use cases of Proxies never need it. Consider the complexity of membrane implementation, I think the author of membrane library will never "forget" it. 😆

@rdking
Copy link
Author

rdking commented Jan 3, 2019

@hax That comment was targeted toward those who think public-fields as currently defined in this proposal is a good idea. I just mentioned on of the reasons I was given for why pseudo-declarative instance-specific properties (fields) is a good idea. For those of us who don't buy it, of course that argument doesn't hold. 😜

@erights
Copy link

erights commented Jan 6, 2019

In case anyone is watching this thread but not #183 , I should mention that this discussion is currently continuing there.

@littledan
Copy link
Member

I think we've concluded by now, through discussion in this thread, other threads, and at the January 2019 TC39 meeting, that we're leaving the Proxy semantics of private fields as is.

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