search

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] yet another alternative to `#`-based syntax #149

Closed Igmat closed 5 years ago

Igmat commented 5 years ago

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 (https://github.com/tc39/proposal-class-fields/issues/147#issuecomment-430441686) 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.

hax commented 5 years ago

@erights @littledan @ljharb

Actually we are just requiring a simple runnable test case which will pass as current weakmap/slot based semantic, and will fail as private symbol semantic. And then we the supporters of private symbols can find the solution, or accept private symbol is doomed to satisfy membrane use case.

erights commented 5 years ago

Hi @hax @Igmat Apologies if I sound impatient. The following page is from the old EcmaScript wiki as of Dec 2014. It says (I said?) that much of it is based on a March 2013 tc39 meeting.

https://web.archive.org/web/20141214073847/http://wiki.ecmascript.org/doku.php?id=strawman:relationships

There I presented the following slides:

relationships.pdf

The key point starts on slide 17, which enumerates the eight cases that must work in order for assignment and lookup to be transparent across membranes.

In all cases, an assignment done on the left side of the membrane followed by a lookup on the right side of the membrane must agree. With the WeakMap model of private names, i.e., the PrivateName object from the current decorators proposal, all eight of these cases work because the assignment and lookup faults on the object reifying the field name, i.e., a get or set on the PrivateName instance, or the proxy for that instance. In the private symbol model, the fault would be on the base object, or the proxy for the base object. When faulting the proxy for the base object, what is passed to that proxy's handler? If you pass the private symbol itself, you've let the membrane steal a private symbol it should never have access to.

The next set of slides diagrams why the first of those eight cases works.

erights commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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

Igmat commented 5 years ago

@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 commented 5 years ago

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);
bT[fT] = vP;

set using right side field name

bT[fP] = vT;
bT[fP] = vP;

set on right side of membrane

set using left side field name

bP[fT] = vT;
bP[fT] = vP;

set using right side field name

bP[fP] = vT;
bP[fP] = vP;
rdking commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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

erights commented 5 years ago

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

p-bakker commented 5 years ago

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 commented 5 years ago

@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 commented 5 years ago

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 https://github.com/tc39/proposal-class-fields/issues/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 commented 5 years ago

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 commented 5 years ago

@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 commented 5 years ago

@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 https://github.com/tc39/proposal-class-fields/issues/122#issuecomment-426486176 onwards), but I agree (as you mentioned in https://github.com/tc39/proposal-class-fields/issues/122#issuecomment-429282391 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 https://github.com/tc39/proposal-class-fields/issues/122#issuecomment-421572848 and https://github.com/tc39/proposal-class-fields/issues/122#issuecomment-428610538), 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 commented 5 years ago

@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 commented 5 years ago

@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 commented 5 years ago

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 commented 5 years ago

@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 commented 5 years ago

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.