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

[Private] yet another alternative to #-based syntax #149

Closed
Igmat opened this issue Oct 17, 2018 · 74 comments
Closed

[Private] yet another alternative to #-based syntax #149

Igmat opened this issue Oct 17, 2018 · 74 comments

Comments

@Igmat
Copy link

Igmat commented Oct 17, 2018

Disclaimer

My intent here is to show that there are good alternatives to existing private part of this proposal, that were not properly assessed. Probably it happened, because committee has a very limited time and amount of members involved, while such long time for discussion around encapsulation (years or even decades) has lead to loosing clear sight on the problem and possible solutions.

Proposal

It's based on the my proposal in #134 and Symbol.private - I won't repeat them fully here, but I hope that you may visit these links to get more background.
This comment (#147 (comment)) to my initial proposal inspired me to make another one, which dismisses problem with this having more complex semantics than it has now.

Goals:

  1. Prevent confusion with private x / this.x pair
  2. Symmetry between this.x and obj.x
  3. Symmetry between declaration and access syntaxes

Solution

Use Symbol.private with existing lookup mechanism, existing mental model for accessing properties and small syntax sugar to make use of such symbols more ergonomic, by declaring symbol variable in lexical scope of a class.

Syntax 1 (less verbose):

class A {
    // declaring of new private symbols in lexical scope of a class
    symbol somePrivatePropertyName;
    symbol somePrivateMethodName;

    [somePrivatePropertyName] = 'this is private value';
    somePrivatePropertyName= 'this is public value with a `somePrivatePropertyName` property name';

    [somePrivateMethodName]() {
        // it is a private method
    }
    somePrivateMethodName() {
        // it is a public method with `somePrivateMethodName` name
    }
    methodA(obj) {
        this[somePrivateMethodName](); // call private method
        this.somePrivateMethodName(); // call public method
        this[somePrivatePropertyName]; // access private property
        this.somePrivatePropertyName; // access public property

        obj[somePrivateMethodName](); // call private method
        obj.somePrivateMethodName(); // call public method
        obj[somePrivatePropertyName]; // access private property
        obj.somePrivatePropertyName; // access public property
    }
}

const a = new A();
// throws `ReferenceError` since `somePrivatePropertyName` doesn't exist outside of class A
a[somePrivatePropertyName];
// access public property `somePrivatePropertyName` of `a`, as expected
a.somePrivatePropertyName;

Syntax 2 (more generic):

class A {
    // declaring of new private symbols in lexical scope of a class
    const somePrivatePropertyName = Symbol.private();
    const somePrivateMethodName = Symbol.private();

    // everything else is the same as in previous example

    // additional possibility of such syntax
    let numberOfInstanses = 0;

    constructor() {
        numberOfInstanses++;
    }
}

This option is a little bit more verbose, and probably such syntax isn't obvious enough to use for declaring private instance data, but as I shown above it could be used for something else, which is probably make some sense.

Advantages:

  1. Doesn't have any problems listed in FAQ
  2. No new semantics === easy to learn
  3. Full symmetry === prevents accidental missuses
  4. Possibility to extend in future, if needed
  5. Doesn't use #-sigil, so no risk for breaking community
  6. Compatible with TypeScript's private

Disadvantages:

  1. Not yet implemented
  2. Slogan # is the new _ couldn't be used for education

Conclusion

Since I don't pretend to replace existing proposal with this one right now, because, obviously, my proposal also needs additional assessment, preparing document that reflects changes to ES spec, creating tests, implementing babel-plugin for transpiling, adding support to JS-engines and most importantly wider discussion with community (BTW, existing also needs it), I only hope that this clearly shows the fact that there are still good alternatives, which weren't taken into account.

What is your opinion on that @littledan, @zenparsing, @jridgewell, @ljharb?

P.S.

This syntax also adds a bonus usage scenario:

class A {
    symbol marked;

    markForeignObject(foreignObject) {
        if (foreignObject[marked]) {
            // do something with knowledge that `foreignObject` was already passed to this method
        } else {
            // do something with knowledge that `foreignObject` enters here first time
            foreignObject[marked] = true;
        }
    }
}

And it's only one possible scenario, there could be dozens of them.

@shannon
Copy link

shannon commented Oct 17, 2018

Yes please. And we can finally start to use symbols for what everyone I know wanted to use them for in the first place.

P.S. I prefer the first syntax as it's such a minor change to existing syntax.

@hax
Copy link
Member

hax commented Oct 17, 2018

Syntax 2 (more generic):

class A {
    // declaring of new private symbols in lexical scope of a class
    const somePrivatePropertyName = Symbol.private();
    const somePrivateMethodName = Symbol.private();

    // everything else is the same as in previous example

    // additional possibility of such syntax
    let numberOfInstanses = 0;

    constructor() {
        numberOfInstanses++;
    }
}

Syntax 2... You use const/let in class body, they are instance vars with shorthand syntax we discussed in classes 1.1 proposal.

Copy from zenparsing/js-classes-1.1#45 (comment)

class Counter extends HTMLElement {
  const setX = (value) => { x = value; window.requestAnimationFrame(render); };
  const render = () => this.textContent = x.toString();
  const clicked = () => setX(x++);

  let x = 0;

  constructor() {
    super();
    this.onclick = clicked;
  }

  connectedCallback() {
    render();
  }

}

@Igmat
Copy link
Author

Igmat commented Oct 17, 2018

@hax I thought it could be a confusion between this proposal and classes 1.1, even though Syntax 2 looks like classes 1.1 it has very different meaning and main difference is:

  • this proposal adds class scoped variables that used to define private symbol which used to get per-instance private value
  • classes 1.1 adds instance scoped private variable

@rdking
Copy link

rdking commented Oct 17, 2018

@Igmat Just how useful is the distinction?

Case 1: (Class scoped private symbol used to add instance scoped private property)

  • private symbol can be directly shared (desirable)
  • property is not an "own property" (required)
  • instance is not accessible without the private symbol (required)

Case 2: (private data contained completely within a closure)

  • cannot directly leak private data (desired)
  • private data is not an own property (required)
  • data not accessible from scopes not created by class definition(required)

The kick in the shins here is that for case 1, if by some accident the private symbol is leaked, there's nothing to stop the leak of the private data. Whereas with case 2, if the developer wishes to share some of the private data, the developer must explicitly provide for that. Which is better?

@Igmat
Copy link
Author

Igmat commented Oct 17, 2018

@rdking in both cases to share private data developer has to explicitly provide access, so probability of unintended leaking is equal for both solutions.
There is no difference between this:

class A {
    symbol somePrivateName;
    leak() {
        return somePrivateName;
   } 
} 

and this:

class A {
    #somePrivate;
    leak() {
        return this.#somePrivate;
   } 
} 

in terms of protecting private data.

And since first allows additional usage scenarios built on that developer is able to share not only private data itself, but also private name, I think that first case is better.

@shannon
Copy link

shannon commented Oct 18, 2018

@Igmat it is different because you have leaked the key to the data. Not just the data as in the second case. But I personally think this ok as it's known to be a symbol and key. People shouldn't be returning the key directly and are unlikely to do it by accident. Returning this[somePrivateName] is the equivalent.

@Igmat
Copy link
Author

Igmat commented Oct 18, 2018

@shannon I didn't mean that those samples do same stuff, I meant that in terms of privacy it doesn't matter what had leaked data or key to data - in both cases encapsulation is broken.

BTW, I realized that leaking of a key seems to be even more unlikely happened than leaking of data.

@rdking
Copy link

rdking commented Oct 18, 2018

@Igmat There is a small but important difference. Leaking the data only puts a copy of or reference to the data in public hands. Leaking the key, puts the private property itself in public hands. In the former case, at most, the fields in an object can be changed, but the object itself will remain in the private property. In the latter case, an object in the private property can be completely replaced. If a library keyed something using that object, being able to replace it can break code. On this point, I think my usual opponents would agree with me, being able to leak the actual name of the private field is a worse problem than just leaking the data.

@shannon
Copy link

shannon commented Oct 18, 2018

@rdking yes yes, but footguns and flexibility and what not. This to me is a feature and not likely to be done by accident. Developers already understand that symbols are keys and this is a way to explicitly and simply provide access to those private properties.

@Igmat
Copy link
Author

Igmat commented Oct 18, 2018

@rdking IMO it's rather feature than foot-gun, because there is mostly no chance to unintentionally reveal private symbol.

symbol somePrivate;

this is definitely declaration, more of that - it's self-explaining declaration, and the most important it doesn't interfere with any existing syntax, so it can't be misunderstood like private or #.

Because this class scoped variable has special type - Symbol, it's very unlikely that it could be confused with its value, since last one most probably has different type, so:


class A {
    symbol somePrivate;

    leak() {
        return somePrivate;
    }
    notALeak() {
        return this[somePrivate];
    }
}

There is no chance that somebody will implement something like leak(), where actually wants notALeak(), and not only because difference is obvious, but also because first usage of leak() would show that returned result can't be used in place where it was meant to use private data to which it points.


class A {
    symbol somePrivate;

    leak() {
        return { somePrivate };
    }
    notALeak() {
        return { somePrivate: this[somePrivate] };
    }
}

This won't be confused for the same reason as previous sample.


class A {
    symbol somePrivate;

    leak(cb) {
        cb(somePrivate);
    }
    notALeak() {
        cb(this[somePrivate]);
    }
}

This won't be confused also because cb function most likely expects some value and not a Symbol.


class A {
    symbol somePrivate;

    leak() {
        return { [somePrivate]: `anything could be here even private symbol itself` };
    }
}

It's even not a leak, because to access private symbol or its value outer code must have it already.


class A {
    symbol somePrivate;

    leak(obj) {
        return obj[somePrivate]= `anything could be here even private symbol itself`;
    }
}

It's not a leak, for same reason as previous sample.


I can't imagine how private symbol could leak unintentionally, so it's definitely feature, and not a bug, unless you can provide some example when it happens by an accident.

P.S.

#-private has mostly the same problem if it points to object and not a primitive.

class A {
    #somePrivate = { a: 'some private string' };

    leak() {
        return this.#somePrivate;
    }
}

After calling leak method you can mutate #somePrivate in any way you want.
BTW, since # doesn't support any kind of dynamic access, using one private field (e.g. #state) and storing object with all privates there could become common practice, so last example may be even more common, than we assume now.

@rdking
Copy link

rdking commented Oct 18, 2018

@shannon

yes yes, but footguns and flexibility and what not.

Funny. Notice I never claimed that being able to do so was a bad idea. Just capable of a potentially worse problem. In the end, what you're offering is a throwback to proposal-private-fields and part of the foundation of the current proposal. It eventually became the "PrivateName" concept. So while you may have some arguing against you, know it's a viable concept because it's mostly already implemented, but with uglier notation. Btw, symbol privateData just looks too much like a typed declaration. Better to choose a different word.

@Igmat
Copy link
Author

Igmat commented Oct 18, 2018

@rdking, on one hand it makes sense

Btw, symbol privateData just looks too much like a typed declaration. Better to choose a different word.

on the other hand TS and flow popularized another type anotation:

let a: number;

So I don't think that symbol will be commonly treated as type instead of keyword.
But, obviously, we can change it if there is better candidate.
My thoughts :

  1. private - leads to [Private] Metaprogramming vs Security #134 (pros: already reserved keyword, cons: needs additional semantic for this )
  2. name/property - aren't self-explanaing enough IMO
  3. let/const/any other already used keyword - interfere with existing usage patterns and causes confusion
  4. abstract/transient/any other reserved but unused yet keyword - aren't seem to be related enough with proposed semantics

If you have any ideas for better keyword, I would love to take a look on them :)

@shannon
Copy link

shannon commented Oct 18, 2018

It might not even be necessary for the symbol to be declared in the class itself. As long as Symbol.private is implemented you can just pair it with the existing public properties proposal for it to be pretty useful. Set/Define debate aside.

const someName = Symbol.private();
class A {
   [someName] = 'private';
  ...
} 

It's easier to use than a weakmap and not incredibly verbose. This is really how I wanted symbols to work in the first place. When I learned that you could just get references to them via getOwnPropertySymbols I was very disappointed.

The symbol/whatever keyword could be added in a follow on as simple sugar. Which would make this just: drop private properties as a class construct and implement private symbols and call it a day.

@shannon
Copy link

shannon commented Oct 18, 2018

However, I know people are going to say it needs to be syntax to avoid monkey patching Symbol. So there is that to consider.

@Igmat
Copy link
Author

Igmat commented Oct 18, 2018

@shannon totally agree with that Symbol.private is enough by itself, but it seems that lack of simple and ergonomic syntax for declaring private properties with it is a deal-breaker for committee, so my proposals try to fulfill that gap.

@littledan
Copy link
Member

littledan commented Oct 19, 2018

The big issue with using private symbols in this sort of way is that it's unclear how to square their semantics with some integrity goals that @erights has been championing. Separately, I think that it'll be lighter-weight for people to adopt # than to switch to computed property syntax, even with the improvements in this proposal.

@aimingoo
Copy link

@Igmat

So, maybe you need a utils with some code typography skills only. for your case:

class A {
    // declaring of new private symbols in lexical scope of a class
    symbol somePrivatePropertyName;
    symbol somePrivateMethodName;

//    [somePrivatePropertyName] = 'this is private value';
//    somePrivatePropertyName= 'this is public value with a `somePrivatePropertyName` property name';

    [somePrivateMethodName]() {
        // it is a private method
    }
    somePrivateMethodName() {
        // it is a public method with `somePrivateMethodName` name
    }
    methodA(obj) {
        this[somePrivateMethodName](); // call private method
        this.somePrivateMethodName(); // call public method
        this[somePrivatePropertyName]; // access private property
   ...
}

^^.

There is a layout solution:

// A utils function
const definePrivated = (f, ...args) => f(...args);

// Define a class with privated names
A = definePrivated(( // define a privated name part
   somePrivatePropertyName = Symbol(),
   somePrivateMethodName = Symbol()) => class { // and a class part

   [somePrivateMethodName](x) {
      console.log("Yes, in private method:", this[somePrivatePropertyName], 'and', x[somePrivatePropertyName]);
      
      x[somePrivatePropertyName] = 'PrivateProperty is property and defaut is undefined value, updated.';
      x.methodB(x);
   }

   methodA() {
      this[somePrivatePropertyName] = 200;
      this[somePrivateMethodName](new A);
   }

   methodB(obj) {
      console.log(obj[somePrivatePropertyName]); // updated
   }

});

Please, try it:

> obj = new A;
> obj.methodA();
yes, in private method: 200 and undefined
PrivateProperty is property and defaut is undefined value, updated.

Done. hahaha~~

Now, No new concept! No No new semantics! No magic! No community breaking!

No new spec! Good! 👍

@shannon
Copy link

shannon commented Oct 19, 2018

@littledan

The big issue with using private symbols in this sort of way is that it's unclear how to square their semantics with some integrity goals that @erights has been championing.

Could you elaborate on the integrity goals you are referring to?

Separately, I think that it'll be lighter-weight for people to adopt # than to switch to computed property syntax, even with the improvements in this proposal.

# may be lighter weight in declaration by one character (not completely true because you have to declare the symbol first, but a computed property is only one character difference is what I meant to say). However, it's heavier in semantics, dynamic access limitations (redundant code), and I have to assume in implementation details.

@aimingoo Almost, except without Symbol.private you can access the symbols with getOwnPropretySymbols. So there would need to at least be spec for that.

Edit: for clarification

@aimingoo
Copy link

@Igmat @littledan

And, definePrivated with a ...args is a special design so that friendly classes can be implemented. such as:

passportcard = Symbol();

A = definePrivated(..., passportcard);
B = definePrivated(..., passportcard);

Both A and B Class can access both private members. ^^.

@littledan yes, need disable public these OwnPropertySymbols, get, enumerate, etc.

@Igmat
Copy link
Author

Igmat commented Oct 19, 2018

@aimingoo friend classes are achievable in my proposal too, since private symbol could be created outside of class and shared between several class declarations. My proposal is only trying to introduce more ergonomic syntax for symbols.

@littledan are you talking about Realms proposal? If yes, what problems could be caused with it and Symbol.private implemented together?

I think that it'll be lighter-weight for people to adopt # than to switch to computed property syntax, even with the improvements in this proposal.

I'm not sure about this assumption. I have opposite opinion, especially taking into account how many issues was caused by # in community and how widely symbols and computed properties already adopted.

@jridgewell
Copy link
Member

The big issue with using private symbols in this sort of way is that it's unclear how to square their semantics with some integrity goals that @erights has been championing.

All of MM's concerns can be solved with Private Symbols. His biggest, proxy transparency, can just have it so that proxies throw when the key they're trapping is a private symbol.

As for passing a reified private symbol through a membrane, he could decide to throw. Or just let it pass through, since it can't be used to extract data from the other side of the membrane (the membrane will throw on private symbol properties, per above).

Separately, I think that it'll be lighter-weight for people to adopt # than to switch to computed property syntax, even with the improvements in this proposal.

We can both have private symbols and # syntax. We just need to change branding semantics into normal prototype-lookup property semantics. I would love to get rid of the branding semantics anyways.

are you talking about Realms proposal? If yes, what problems could be caused with it and Symbol.private implemented together?

Yup. See the first part of my comment.

@littledan
Copy link
Member

See the notes of the September 2018 TC39 meeting where we discussed this issue. For example, about making private symbols work, @erights said at that meeting:

MM: There's nothing we can do to solve this with membranes. We tried, and it was a horrible mess. I'm not at all confident that this is possible.

Have you convinced @erights of your ideas above? I'm pretty sure the idea there is within the spectrum of things that was considered in the ES6 cycle and rejected.

@jridgewell
Copy link
Member

jridgewell commented Oct 20, 2018

Have you convinced @erights of your ideas above?

No, but that' doesn't change that it's possible. See zenparsing/proposal-private-symbols#7 (comment).

Note, too, that in the last TC39 meeting we were discussing this under the assumption of transparent proxies (where proxy[privateSymbol] would skip the proxy and go directly to the target). We can make proxies non-transparent (where they throw without consulting the proxy's hander nor the target), and they become nearly identical what would happen with private fields. This would also solve MM's goals.

There are really 3 orthogonal choices:

  1. Proxy transparency
  2. # Syntax vs [ ] computed
  3. Branding vs prototype-lookup

Private Fields chose non-transparency, # syntax, and branding. Symbols (as we discussed in the last meeting) chose transparency, [ ] syntax, and prototype lookup.

But Proxy transparency is an orthogonal decision to encapsulation. Private symbols can choose non-transparency to solve the MM's goals. I'm fine with doing whatever will satisfy membranes. I do not want an incorrect idea about proxy transparency to force our hand on branding.

That leaves us with the last two choices. Class fields choose # syntax, but private symbols can choose either (where #priv syntax would mean the [priv] where priv is a lexical binding). We can even use the current class fields syntax without changes and have it work with symbols (the field #priv = 1 declaration would inject priv into the lexical scope).

But branding is also an orthogonal decision to both proxies and syntax. This decision affects what we can reify to. If we choose branding, symbols just don't make sense. But if we choose prototype-lookup, we can reify to either a symbol or a (modified) PrivateName. If we choose to stick with # syntax, we don't even have to choose the reify yet, it can be deferred to decorators later.

@littledan
Copy link
Member

Personally, I would be fine with a follow-on proposal that reifies private symbols, with private symbols having the semantics that they are not transparent through Proxies and do not do prototype lookup. I see this as actually compatible with "branding"--we could include the same kinds of exception throwing behavior with [] syntax, if we figure out some way to differentiate the Set and Initialize operations. Such a follow-on proposal would "layer" well underneath this proposal.

A few issues with this proposal, which led me to not pursue it in this repository:

  • Private methods wouldn't work seamlessly--you'd need a decorator or something like it to set them up nicely.
  • # syntax would still be more beginner-friendly and lead to higher adoption (so, you'd want both private symbols and # syntax).
  • Symbol doesn't have all of the "integrity" that PrivateName does, leading to questions of capability leaks in practice (but this comes up less frequently since you don't use methods to read and write symbols)
  • Such a proposal wouldn't meet some invariants that @erights is maintaining about what it means for something to be a property. By saying that private fields are like a WeakMap instead of like a property, those invariants are maintained. If we're not maintaining property invariants, the [] syntax could be seen as misleading. @erights could probably explain the details better than I could.

@hax
Copy link
Member

hax commented Oct 20, 2018

@Igmat

even though Syntax 2 looks like classes 1.1 it has very different meaning

Yeah I know the difference. I just want to say use let for all-instance shared state does not match the expectation of most js programmers. I think you may want to use static instead of let/const in your cases.

@hax
Copy link
Member

hax commented Oct 20, 2018

@shannon

I know people are going to say it needs to be syntax to avoid monkey patching Symbol

If you never export (leak) the private symbol, there would be no way to monkey patch.

@shannon
Copy link

shannon commented Oct 20, 2018

@hax I meant monkey patching Symbol.private. So when Symbol.private is called it's not the original function and you can capture all private fields.

@Igmat
Copy link
Author

Igmat commented Oct 21, 2018

@littledan

Private methods wouldn't work seamlessly--you'd need a decorator or something like it to set them up nicely.

Could you please clarify this?

# syntax would still be more beginner-friendly and lead to higher adoption (so, you'd want both private symbols and # syntax).

It's very arguable. Do you have any facts (e.g. result of some poll) behind such statement?

Symbol doesn't have all of the "integrity" that PrivateName does, leading to questions of capability leaks in practice (but this comes up less frequently since you don't use methods to read and write symbols)

Are you talking about returning symbol from a method or something like this?

Such a proposal wouldn't meet some invariants that @erights is maintaining about what it means for something to be a property. By saying that private fields are like a WeakMap instead of like a property, those invariants are maintained. If we're not maintaining property invariants, the [] syntax could be seen as misleading. @erights could probably explain the details better than I could.

What invariants aren't maintained? Is zenparsing/proposal-private-symbols#7 one of them? You may read my last comment about it and why it's not an issue of private symbols at all.

@erights
Copy link

erights commented Nov 22, 2018

To interpret how these cases would apply to private symbols, assuming that private symbols are primitive (as symbols are), then there's no such thing as a "proxy to a private symbol". Instead, when "fT" represents a private symbol of interest on one side of the membrane, "fP" should be whatever the corresponding value is on the other side of the membrane. If private symbols pass though membranes unaltered (as all primitive values do, including symbols), then "fP" and "fT" are the same and we only have four cases, not eight.

@erights
Copy link

erights commented Nov 22, 2018

The bottom of that wiki page restates (and improves the phrasing of) the email at
https://mail.mozilla.org/pipermail/es-discuss/2013-July/032339.html

@hax
Copy link
Member

hax commented Nov 22, 2018

Thank you @erights ! I will try my hard to read it and understand it. If there was a runnable testcase I think it would be easy for us to investigate how the design affect membrane. Anyway, thank you for your information!

@erights
Copy link

erights commented Nov 22, 2018

If you'd like to try to create tests based on the eight cases, I could advise. Thanks!

@Igmat
Copy link
Author

Igmat commented Nov 22, 2018

@erights, as I said before there is a solution that makes your test reachable using Symbol.private, here's a sample:

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) {
    // `privateHandlers` are special objects dedicated to keep invariants built
    // on top of exposing private symbols via public API
    // we also need one-to-one relation between `privateHandler` and `original`
    const privateHandlersOriginals = new WeakMap();
    // we're keep track of created handlers, so we'll be able to adjust them with
    // newly exposed private symbols
    const allHandlers = new Set();

    // just simple helper that creates getter/setter pair for specific
    // private symbol and object that gets through membrane
    function handlePrivate(handler, privateSymbol) {
        const original = privateHandlersOriginals.get(handler);
        Object.defineProperty(handler, privateSymbol, {
            get() {
                return wrap(original[privateSymbol]);
            },
            set(v) {
                original[privateSymbol] = unwrapFn(v);
            }
        })
    }

    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 privateHandler = {};
        privateHandlersOriginals.set(privateHandler, original);
        allHandlers.add(privateHandler);

        //       note that we don't use `original` here as proxy target
        //                      ↓↓↓↓↓↓↓↓↓↓↓↓↓↓
        const proxy = new Proxy(privateHandler, {
            apply(target, thisArg, argArray) {
                thisArg = unwrapFn(thisArg);
                for (let i = 0; i < argArray; i++) {
                    if (!isPrimitive(argArray[i])) {
                        argArray[i] = unwrapFn(argArray[i]);
                    }
                }

                //          but we use `original` here instead of `target`
                //                           ↓↓↓↓↓↓↓↓
                const retval = Reflect.apply(original, thisArg, argArray);

                // in case when private symbols is exposed via some part of public API
                // we have to add such symbol to all possible targets where it could appear
                if (typeof retval === 'symbol' && retval.private) {
                    allHandlers.forEach(handler => handlePrivate(handler, retval));
                }

                return wrap(retval);
            },
            get(target, p, receiver) {
                receiver = unwrapFn(receiver);
                //       but we use `original` here instead of `target`
                //                         ↓↓↓↓↓↓↓↓
                const retval = Reflect.get(original, p, receiver);

                // in case when private symbols is exposed via some part of public API
                // we have to add such symbol to all possible targets where it could appear
                if (typeof retval === 'symbol' && retval.private) {
                    allHandlers.forEach(handler => handlePrivate(handler, retval));
                }

                return wrap(retval);
            },
            // following methods also should be implemented,
            // but it they are skipped for simplicity
            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;
    }

    return wrap;
}

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 originalTargets.has(proxy)
            ? originalTargets.get(proxy)
            : wrapOuter(proxy);
    }

    return wrap(obj);
}

const privateSymbol = Symbol.private();
const Left = {
    base: {
        [privateSymbol]: ''
    },
    value: '',
    field: privateSymbol,
};
const Right = membrane(Left);

const { base: bT, field: fT, value: vT } = Left;
const { base: bP, field: fP, value: vP } = Right;

// # set on left side of membrane
// ## set using left side field name
bT[fT] = vT;
assert(bP[fP] === vP);

bT[fT] = vP;
assert(bP[fP] === vT);

// ## set using right side field name
bT[fP] = vT;
assert(bP[fT] === vP);

bT[fP] = vP;
assert(bP[fT] === vT);

// # set on right side of membrane
// ## set using left side field name
bP[fT] = vT;
assert(bT[fP] === vP);

bP[fT] = vP;
assert(bT[fP] === vT);

// ## set using right side field name
bP[fP] = vT;
assert(bT[fT] === vP);

bP[fP] = vP;
assert(bT[fT] === vT);

@Igmat
Copy link
Author

Igmat commented Nov 22, 2018

I skipped fP->fT and fT->fP transformations in previous example for simplicity. But I'll mention such transformation here, since they are really easy handled when private symbols crosses boundary using public API of membraned object

set on left side of membrane

set using left side field name

bT[fT] = vT;

goes as usual, since happens on one (left) side of membrane

assert(bP[fP] === vP);
  • since fP is private symbol, get called on proxy target
  • which is special privateHandler object
  • call privateHandler[fP] getter:
    • unwrap fP to fT
    • get bT[fT] which equals vT
    • wrap vT to vP
  • assertion is TRUE

bT[fT] = vP;
  • unwrap vP to vT
  • set vT to bt[fT] as usual
assert(bP[fP] === vT);
  • proxy skipped to target (privateHandler)
  • call privateHandler[fP] getter:
    • unwrap fP to fT
    • get bT[fT] which equals vT
    • wrap vT to vP
  • wrap vT to vP
  • assertion is TRUE

set using right side field name

bT[fP] = vT;
  • unwrap fP to fT
  • set vT to bt[fT] as usual
assert(bP[fT] === vP);
  • wrap fT to fP
  • proxy skipped to target (privateHandler)
  • call privateHandler[fP] getter:
    • unwrap fP to fT
    • get bT[fT] which equals vT
    • wrap vT to vP
  • assertion is TRUE

bT[fP] = vP;
  • unwrap fP to fT
  • unwrap vP to vT
  • set vT to bt[fT] as usual
assert(bP[fT] === vT);
  • wrap fT to fP
  • proxy skipped to target (privateHandler)
  • call privateHandler[fP] getter:
    • unwrap fP to fT
    • get bT[fT] which equals vT
    • wrap vT to vP
  • wrap vT to vP
  • assertion is TRUE

set on right side of membrane

set using left side field name

bP[fT] = vT;
  • wrap fT to fP
  • wrap vT to vP
  • set vP to bP[fP]
  • proxy skipped to target (privateHandler)
  • call privateHandler[fP] setter:
    • unwrap fP to fT
    • unwrap vP to vT
    • set vT to bt[fT] as usual
assert(bT[fP] === vP);
  • unwrap fP to fT
  • get bT[fT] which equals vT
  • unwrap vP to vT
  • assertion is TRUE

bP[fT] = vP;
  • wrap fT to fP
  • set vP to bP[fP]
  • proxy skipped to target (privateHandler)
  • call privateHandler[fP] setter:
    • unwrap fP to fT
    • unwrap vP to vT
    • set vT to bt[fT] as usual
assert(bT[fP] === vT);
  • unwrap fP to fT
  • get bT[fT] which equals vT
  • assertion is TRUE

set using right side field name

bP[fP] = vT;
  • wrap vT to vP
  • set vP to bP[fP]
  • proxy skipped to target (privateHandler)
  • call privateHandler[fP] setter:
    • unwrap fP to fT
    • unwrap vP to vT
    • set vT to bt[fT] as usual
assert(bT[fT] === vP);
  • get bT[fT] which equals vT
  • unwrap vP to vT
  • assertion is TRUE

bP[fP] = vP;
  • set vP to bP[fP]
  • proxy skipped to target (privateHandler)
  • call privateHandler[fP] setter:
    • unwrap fP to fT
    • unwrap vP to vT
    • set vT to bt[fT] as usual
assert(bT[fT] === vT);
  • get bT[fT] which equals vT
  • assertion is TRUE

@rdking
Copy link

rdking commented Nov 22, 2018

Fun fact: This pattern works regardless of the privacy approach used.

Fun fact: According to @ljharb, we've got about a snowball's chance .... of getting TC39 to allow Proxy to be bypassed in a way that isn't used for all scenarios. (Again, correct me if I'm wrong).

In the end, this pattern may be the fix for Proxy itself, and all cases involving fP represent access to an internal slot.

@erights
Copy link

erights commented Nov 23, 2018

Hi @Igmat , I do not yet understand your code; I am still studying it. I'm not yet sure what's going on, but I do have a question. In your first example test, of

bT[fT] = vT;

and

assert(bP[fP] === vP);

does the membrane get access to the private symbol itself?

@Igmat
Copy link
Author

Igmat commented Nov 23, 2018

does the membrane get access to the private symbol itself?

@erights, yes but only to those which were explicitly exposed.

const exposedSymbol = Symbol.private();
const privateSymbol = Symbol.private();

class Base {
    [exposedSymbol] = 'exposed private data';
    [privateSymbol] = 'not exposed private data';
    methodA() {
        return this[exposedSymbol];
    }
    methodB() {
        return this[privateSymbol];
    }
}
const Left = {
    base: new Base(),
    field: exposedSymbol,
};
const Right = membrane(Left);

const bP = Right.base;

bP.methodA(); // `exposedSymbol` isn't trapped
bP.methodB(); // `privateSymbol` isn't trapped
// till this point `exposedSymbol` couldn't be directly used
// from `Right` side of membrane, only indirectly moving to methods
// or function that called on `Left` side of membrane

const fP = Right.field;
// from this point membrane is aware of `exposedSymbol`
// and since it could be directly used from `Right` side of membrane
// we have to trap such usages to keep original code invariants

bP.methodA(); // `exposedSymbol`  isn't trapped, since call to it happens on the left side
bP.methodB(); // `privateSymbol` isn't trapped
bP[fP] = 'mutated exposed private data'; // but here `exposedSymbol` IS trapped
// since `privateSymbol` wasn't intentionally exposed by original code
// membrane isn't aware of it and in no way intercepts its usages
// so not exposed symbols still grant hard privacy which couldn't be
// trapped by any code, including membrane

So exposing Symbol.private makes it the part of public API, which should be intercepted by Membrane, but until you do this intentionally, Membrane would not affect any usages of Symbol.private.

Also, if you want to keep all private symbols unexposed, regardless of what was the intention of code from one side of membrane, you're able to throw an exception when private symbols tries to cross the boundary.

@Igmat
Copy link
Author

Igmat commented Nov 27, 2018

@erights do you have any other questions to this implementation?

@erights
Copy link

erights commented Nov 27, 2018

I will once I have time to get back to it. Hopefully next week. Sorry for the delay.

@p-bakker
Copy link

p-bakker commented Nov 27, 2018

Maybe I'm missing something, but isn't it very easy to get to the private Symbols from the outside?

Consider this:

const s = Symbol(); //Using a plain Symbol here as I don't have a private one to work with currently
class Test {
  [s] = 10; 
  usePrivate() {
   this[s] //Just some code in the body of a public method that accesses a private field or method using the Symbol
  }
}

const secretGrabber = new Proxy({}, {
  get: (target, prop, receiver) => {
    console.log(prop); 
    Reflect.get(target, prop, receiver)
  }
})

const instance = new Test ()
instance.usePrivate.call(secretGrabber) //logs the Symbol

@Igmat
Copy link
Author

Igmat commented Nov 27, 2018

@p-bakker, const s = Symbol(); - this is usual symbol, not private one.

Quote from there which describes, why your code won't trap Symbol.private:

A private symbol is semantically identical to a regular symbol, with the following exceptions:
1. Private symbols are not exposed by Object.getOwnPropertySymbols.
2. Private symbols are not copied by Object.assign or object spread.
3. Private symbol-keyed properties are not affected by Object.freeze and Object.seal.
4. Private symbols are not exposed to proxy handlers.

@p-bakker
Copy link

Right, knew I must have missed something :-)

How well does this proposal do when thinking about future access modifiers like friend/inherited/protected, as discussed in #122?

The # sigil approach does leave a path to adding those, albeit an odd one if the private keyword is not added as mandatory to the current proposal, in the form of

class X {
  myPublicField
  public anotherPublicField
  #myPrivateField //if no access modifier keyword is specified, it defaults to private
  private #anotherPrivateField
  protected #myProtectedField
  ...
}

@Igmat
Copy link
Author

Igmat commented Nov 27, 2018

IMO much better then existing one.

Since Symbol.private just a variable it could be explicitely shared in a way you need out-of-the-box.
Making stuff like friend/internal is very easy:

const internalSymbol = Symbol.private();

class A {
    [internalSymbol] = 'something internal to specific module';
}

class A {
    method() {
        const instanceOfA = new A();
        console.log(instanceOfA[internalSymbol]);
    }
}

protected-like behavior is trickier, but still doable (if you're interested I'll try to provide some solution for this).

And this will be available with no additional standard changes, but talking about future extension of language spec, I see few available options. Each of them will require adding something like Symbol.protected, since this approach is a core of proposal.

  1. Adding access modifier to symbol shorthand in class:
    class A {
        symbol privateSymbol; // or it could be public?
        protected symbol protectedSymbol;
        private symbol definitelyPrivateSymbol;
        public symbol publicSymbol;
    }
  2. Or changing existing shorthand to following:
    class A {
        publicProperty = 1;
        [usualComputedProperty] = 2;
        private [privateProperty] = 3;
        protected [protectedProperty] = 4;
        public [publicSymboledProperty] = 5;
    }
    where 1 and 2 work as usual (e.g. existing proposal), while 3, 4 and 5 not only create property on the instace of A, but also creates class scoped Symbol.private, Symbol.protected and Symbol.

Second is my latest idea, probably I'll make more detailed proposal for that. But as you can see, even though there are ideas about more ergonomic shorthand syntax for Symbol.private and even Symbol itself, they aren't discussed, because core proposal haven't landed even to stage 1, so my major goal here to show that Symbol.private doesn't share same semantic problems as current proposal, which means that it at least should be treated as alternative design that could land into the process for further discussions and improvements.

Only after that, there weill be sense to discuss syntax shorthand.

But if, you're interested, I may prepare some bigger overview of possible solutions, but I'm not sure that it make sense to add them here as issues.

@hax
Copy link
Member

hax commented Nov 28, 2018

@p-bakker

The # sigil approach does leave a path to adding those

It's technically possible, but as my observation of the background logic of current proposal, the syntax for protected/friend will be even worse than #. For example, it's very unlikely there could be private/public/protected or any other form of keywords, because it will cause syntax storm. So the possible "solution" would be ##x and this.##x for protected. 🤪

On the other side, private symbol-based solution is much easy to offer a nice, keyword-based syntax.

@p-bakker
Copy link

@lgmat I think this will suffice for now: just wanted to know if there's a sensible path forward to such access modifiers using this approach, because one area of feedback on the current proposal is the (apparent) lack of a path forward

@hax don't think it'll cause a syntax storm, as the private/protected/public keywords could be added to change the meaning of the sigil (as discussed from #122 (comment) onwards), but I agree (as you mentioned in #122 (comment) as well) that A: changing the mental model of what the sigil means down the road ain't good (and might run into objections in the TC39 committee for just that reason) and B: adding access modifiers down the road causing an implicit private if the access modifier is omitted isn't nice/is confusing as well. @littledan said he's willing to explore the impact of the current proposal on the ability to add protected down the road (in #122 (comment) and #122 (comment)), but to my knowledge his request for a repo to discuss this angle hasn't been fulfilled. It could be argued whether its the communities responsibility to address such down-the-road design questions or the committee's, but right now the issue has been brought forward and interest in exploring it has been made public

@hax
Copy link
Member

hax commented Nov 28, 2018

@p-bakker What I call syntax storm is:
If we add private/protected keywords, it would be very weird have no public keyword, even it's totally redundant as current proposal. This will affect all class elements, methods or fields, static or instance.

Actually as I already said in many comments, keyword can solve some problem easily (like confusion of [[Define]] and [[Set]], new ASI hazard, etc.), but the committee refuse to add keyword, even some champions insist on the decision of "no keyword in public field" is not affected by "no keyword in private field", but to be honest, it's very weak from the view of outside of TC39.

@Igmat
Copy link
Author

Igmat commented Dec 10, 2018

@erights so did you have a chance to take a closer look on this? Do you have any objections to my statement that Membrane pattern could be implemented with Symbol.private approach?

@Igmat
Copy link
Author

Igmat commented Dec 11, 2018

I will once I have time to get back to it. Hopefully next week. Sorry for the delay.

@erights, it was 2 weeks ago.
I've seen your activity in other threads, but you haven't responded here.
Could you, please, stay in touch, since your opinion on currently discussed topic was decisive to drop Symbol.private approach as viable alternative to #-based private.

@hax
Copy link
Member

hax commented Dec 11, 2018

@erights I also read the @Igmat 's implementation and I think it should be able to satisfy membrane transparency. The idea is simple, whenever a private symbol is passed from the other side of membrane, use it to trap get/set and do wrap/unwrap. I used to think we need add knownPrivateSymbols to Proxy interface and let Proxy do such thing for us, but @Igmat 's trick just work.

But, we still need you to confirm whether it's ok (because I remember there is PrivateSymbolsWhitelist trap to do similar thing in the early day of Proxy design, but be rejected for some reason I don't know and understand). Thank you!

@littledan
Copy link
Member

The fundamental issue here is whether private symbols are viable. Seems like we've been able to engage with @erights in #158; I think it should be enough to have one issue to track this question.

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