Can't dynamically set private field in initializer

[mbrowne]

I was trying to upgrade my bound-decorator library (which was based on the Sept. 2018 version of this proposal) to the current proposal, but I ran into an issue with private methods. I'd like to maintain support for private methods as my library currently does, with the goal looking like this:

class ComponentWithPrivateMembers {
    #x = 1

    @bound
    #handleClick() {
        console.log('this.#x = ', this.#x)
    }

    simulateClick() {
        const handler = this.#handleClick
        handler()
    }
}

new ComponentWithPrivateMembers().simulateClick()

The replacement of the method with the bound method needs to happen at initialization time for new instances so that it has access to this, but it seems that context.addInitializer() won't work in this case because there's no way to dynamically set a private field. e.g. this['#x'] is not the same as this.#x.

Am I missing something? If this is indeed impossible with the current class field and decorators specs, is there some future add-on proposal that could address this?

[ljharb]

The decorator should be receiving a get and set function that provides you with access to the private field, but that may not work for private methods.

[mbrowne]

@ljharb Thanks. I just tried it and unfortunately the context.access object only contains get and has methods (not a set method) for methods...

[mhofman]

Interesting, private methods are technically installed as instance fields but I don't believe there is any way to redefine them currently, so I suspect some implementations likely share a class shape for all instances. Allowing to replace the value for specific instances would likely introduce implementation complexity. I'm wondering if there should be a way in private methods decorators to flag that the return value is an initializer (like for class fields) instead of a replacement method.

Edit: actually we'd have to change the behavior for all private methods decorators as it's not possible to chain an initializer returning decorator into a method override decorator. That seems suboptimal for the common wrapping use case.

Edit2: thinking about this more there may be a way to combine both without the decorators having to coordinate, but it's a bit complicated (the value would have to be a function that forwards to the current function for the instance in the stack of decorators)

[mbrowne]

In case it helps, for this use case at least it wouldn't be necessary to replace the method (although maybe that would be most straightforward)...turning it into a regular private field that happens to have a value of a function would work just fine.

[mbrowne]

Of course, if you want to use private fields and methods, there's always this workaround—technically a private field (arrow function) rather than a private method:

class ComponentWithPrivateMembers {
    #x = 1

    #handleClick = () => {
        console.log('this.#x =', this.#x)
    }

    simulateClick() {
        const handler = this.#handleClick
        handler()
    }
}

...which doesn't suffer from the same downsides as public properties for bound methods (related to the prototype and inheritance, which are irrelevant for private methods). But ideally it would be nice to have consistent syntax, with a @bound director that you could use on both public and private methods.

[mhofman]

Here is another thought. Since decorators are now structured to never change the type of the thing it decorates, I'm wondering if allowing the accessor modifier on private methods would make sense? However to be honest, if we're going to require the author to change the syntax of the declaration, maybe asking to declare it as a private field containing a function in the first place may not be too much of a stretch.

[mbrowne]

@mhofman

I'm wondering if allowing the accessor modifier on private methods would make sense?

I think it could be a bit weird to expand the concept of "accessor" to include accessing a field or method internally that you can already access...what would it actually mean to have such an accessor?

Do you think this option is still worth exploring:

Edit2: thinking about this more there may be a way to combine both without the decorators having to coordinate, but it's a bit complicated (the value would have to be a function that forwards to the current function for the instance in the stack of decorators)

[biro456]

@mbrowne this is the workaround I came up with to make @bound work with private fields:

Actually only works for singletons 🤦

export function bound<T extends object, V extends (this: T, ...args: any[]) => any>(target: V, context: ClassMethodDecoratorContext<T, V>): V {
    let instance: T;

    context.addInitializer(function () {
  	  instance = this;
    });

    return function (this: T, ...args: Parameters<V>): ReturnType<V> {
  	  return target.call(instance, ...args);
    };
}

It's not exactly the behaviour one would expect, but it gets the job done.

[mbrowne]

@biro456 Thanks, it's nice to know this is possible. I'm not sure if the this: T part in the function you're returning is necessary unless you found that something didn't type-check correctly without it. I was just testing in JS (not TS) but this worked for me (I also changed .call to .apply but that doesn't really matter):

    return (...args) => {
        return target.apply(instance, args)
    }

[biro456]

And a day later I realize it only works for singletons. 🤦

[mbrowne]

Aww ok, I hadn't inspected it closely yet but that makes sense. I believe the bound function only runs once.

[anlexN]

WHEN is proposal-decorators final?

[ljharb]

It's already stage 3, so unless implementers discover a previously unknown problem while implementing it, it's pretty much as final as it'll ever be.

[anlexN]

It's already stage 3, so unless implementers discover a previously unknown problem while implementing it, it's pretty much as final as it'll ever be.

so it is almost final, not true final. do it have roadmap?

[ljharb]

There is never a “true final” for anything.

[nicolo-ribaudo]

@anlexN Browsers are implementing this proposal right now. Once two of them ship it enabled by default, the champion will propose advancing this proposal to Stage 4 (the final stage) and it will be merged to the main specification.

[pzuraq]

So this was discussed previously, it was shot down due to performance concerns around being able to redefine private class methods. It's something that could be considered in the future, definitely, as it wouldn't be a breaking change to add the access.set method to context I don't think.