Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"this" inside the decorator #72

Open
mjesun opened this issue Apr 20, 2016 · 5 comments
Open

"this" inside the decorator #72

mjesun opened this issue Apr 20, 2016 · 5 comments

Comments

@mjesun
Copy link

mjesun commented Apr 20, 2016

What should be the value of "this" inside the decorator? For example if you do something like:

@Decorators.decorate
class A {}

When being inside decorate, the value of this should be Decorators, right?

I'm asking that because I haven't been able to find this inside the proposed specification, and I think it's nice to have it defined. Also, if that's the case, the current plugin for babel (the babel-plugin-transform-decorators-legacy) should be changed to support it. Right now the compilation is done as:

(_dec = Decorators.decorate, _dec(_class = function () { ... }))

Which would be incorrect (this is undefined in strict mode, or the global object in loose mode).

@silkentrance
Copy link

silkentrance commented Apr 20, 2016

@mjesun Actually, the this is not defined. The decorator is considered to be a free function and I believe this to be the correct approach.

That way, you can define more complex decorators where you can provide your own this.

@mjesun
Copy link
Author

mjesun commented Apr 21, 2016

Maybe I'm missing something; but I guess you can't provide your own this, unless you bind your method beforehand; since you do not have any control on how the method is invoked. It's also simpler for me to expect that a decorator defined as @a.b respects the way this has been working since the very beginning in JS.

@otakustay
Copy link

I'm on the opposite side, the declaration and invocation does not happen at the same time, when you write code like:

@a.b

is just a declaration which just like:

var hereIsADecorator = a.b;

So when decorator is invoked, it is:

hereIsADecorator();

which should not provide any this value.

@mjesun
Copy link
Author

mjesun commented Apr 21, 2016

I can get that point; however I don't see the benefit of it. If this was used for something else, then I think I'd be OK. However, this is not used for anything, so why just not keeping the context value?

I assume you want to keep it undefined because you see the declaration and the invocation at different point in times, but I see that there are nicer advantages in keeping the context and considering that the declaration and invocation happens all at once (e.g. being able to use static class methods as mixins).

Still, the proposal is yours, so if I can't convince you I think we can close the ticket 😃

@otakustay
Copy link

I'm not the author but I believe assuming a this value in decorator impolementation could be a dangerous thing since the author is not arware how this decorator would being used

For example if we implement a decorator like this:

// decorators.js
export default {
    name: 'whatever',

    foo(target, key, descriptor) {
        target._hasDecorator = this.name;
    },

    bar() {
        // ...
    },

    // ...
};

but is used like this:

import decorators from 'decorators';

let {foo, bar} = decorators;

export default class OhMyGod {
    @foo
    @bar
    work() {}
}

The this value is lost

If this is required, it is better to bind it on implementation:

const context = {name: 'whatever'};
function foo() {
}
export default {
     foo: foo.bind(context)
};

This is much safer

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants