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

Decorators on static and instance method order #39

Open
just-boris opened this issue Sep 25, 2015 · 5 comments
Open

Decorators on static and instance method order #39

just-boris opened this issue Sep 25, 2015 · 5 comments

Comments

@just-boris
Copy link

I din't found in this document description of decoration applying order. But I think that is good to have it standardized.

So, what's the order of decorations, who goes first: class, static or prototype decorators?

Also, babel now applies prototype decorators before static, which causes issues, but that is technically correct, because the spec has nothing about it. See a Babel REPL with unexpected result.

@silkentrance
Copy link

You don't seem to understand how decorators work. There is no issue with the way decorators work. You are calling isTestable on the target prototype, i.e. the prototype of the decorated class.

Please close as invalid/personal learning process.

@just-boris
Copy link
Author

@silkentrance Well, I fixed the code example, now it is called on instance.

Is it really changing something?

@silkentrance
Copy link

@just-boris yes, this is correct, now you just have to fix your logic. Hadlner should be called when testable, right? So, it actually is testable as it is being called and will throw the exception.

@just-boris
Copy link
Author

@silkentrance well, thank you that you pointed that my example is not clear.
I have rewritten it from the scratch: see link

I am going to use the conjunction of two decorators to set up debugging for my classes. First, mark some methods as onlyInDebug do disable them in production. Then if you want to debug some classes, just add the debugEnabled on top of these classes. As for me, it looks better and flexible alternative rather than the current way to do it with NODE_ENV variable.

@silkentrance
Copy link

@just-boris again, your logic is off. class decorators get applied after that the class was defined.

see

var SecondClass = (function () {
  function SecondClass() {
    _classCallCheck(this, _SecondClass);
  }

  _createDecoratedClass(SecondClass, [{
    key: 'warning',
    decorators: [onlyInDebug],
    value: function warning() {
      console.warn('This warning only for testing');
    }
  }]);

  var _SecondClass = SecondClass;
  SecondClass = debugEnabled(SecondClass) || SecondClass;
  return SecondClass;
})();

where debugEnabled gets called right before the defined class will be returned.

And before you go and cry out about this being wrong. See for example the way Python decorators work. In fact, the Python interpreter will first call any method / property decorators and will at the very last call upon the class decorators. And this is good, since it has enable the Python community to come up with great frameworks based on that approach, see for example Zope.Interface and what not.

So you need to invert your logic here, marking these methods as in debug only, for example with the symbol and then, when it comes to your class decorator, you will have to eliminate those which should be available during debug only.

But then be sure to also decorate your first class, otherwise it will not work for that, too.

And, perhaps, you should consider trying a different approach?

class Foo {
   method() {
        this.debugMethod();
   }

    debugMethod() {
        if (DEBUG) {
          ...
        }
    }
}

The above is a well known and commonly accepted pattern.

Alternatively you could rewrite your debugOnly decorator like this

function debugOnly(target, attr, descriptor) {
     const result = {...descriptor};
     const ofunc = descriptor.value;
     result.value = function () {
         if (DEBUG) {
             return ofunc.apply(this, arguments);
         }
     };
     return result;
}

or something like that.

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

2 participants