Decorator call signatures are ambiguous

[dongryphon]

Consider:

@foo
class A {
}

@foo(A)
class B {
}

I am attempting to provide a decorator that accepts fully optional arguments including optional class parameters. Is there some idea brewing how to reliably determine the context of such things?

Given the current spec as I understand it (using babel-plugin-transform-decorators-legacy), there is no way to distinguish the above forms. To support case 1 you would do:

  function foo (C) {
      // decorate class C
  }

To support case 2, you would do:

  function foo (p) {
      return C => {
          // decorate C using p
      };
  }

But since the parameter is a class there is no way to tell which form is being invoked.

If I could wish :) I would rather write decorators like so:

  function foo (args, C) {
      // decorate C using optional additional args
  }

The idea there is that args is whatever (if anything) is supplied in parens after the decorator name. So to handle both cases:

  function foo (args, C) {
      if (args && args.length) {
          // args is an array if @foo() of some form was used
         let p = args[0];
      } else {
          // @foo w/no args was used...
      }
  }

One could debate whether args should be an empty array to distinguish @foo from @foo() I suppose.

[MMeent]

You can try for the 'options object' option:

function foo(opts, ...args) {
  if (args.length  0) // function was not used as a class decorator
    {cls} = opts;
    return C => {...};
  else // used as a class decorator
    return dec_cls(opts, ..args);
}

@foo
class A {}

@foo({cls: A})
class B {}

... or something like that

[dongryphon]

Thanks for the suggestion @MMeent - that is a good workaround if this ambiguity is not resolved before the spec is finalized.

I do hope we can change the argument passing along the lines I suggested there... because the current functional zen (to me) is a really an unfortunate way to express the goal here :)

[Download]

I find it amazing that we had this issue on the old repo: class decorator optional opts #wycats/23, with 156 comments (!!) and, afaik, no formal response from the spec leads (or any sign that any discussion had taken place to address this concern) and I've been following the progression of the spec ever since.

And now, one year and nine months later, I see everything has been moved to a new repo. The old issues are gone, like they never existed and here, on this new repo, we get the exact same question.... Hell even the issue number is only one off: 24 vs 23. Seems that's about how long it takes for people to catch up to this hole in the spec. Where are all the ideas mentioned in the discussions on the other repo addressed? Was any consideration given to the problems mentioned? I even came up with an idea to address it myself, but never had any response from anyone involved in the spec.

This is what I wrote:

Download - Stijn de Witt - commented on 20 Aug 2016 To be honest I am a bit disappointed. There has been lots of discussion here on this and other related issues about the duck typing / decorator vs factory / with/without parentheses issues, but I see nothing mentioned on it anywhere in the new proposal. It looks like the whole discussion has been completely ignored...

(could be I'm reading it all wrong and it has in fact been addressed or at least mentioned somewhere... if so, please point me to the relevant text because I can't find it)

Is the issue tracker here just for fun and to show off how open source Ecma is, or will you guys actually respond to community feedback / questions?

What is the idea for this issue? Will we attempt to address it? You can just wait for confusion around this... It would be a shame if the spec was progressing and this issue that has been brought up so early and often was never dealt with.

[littledan]

Could someone provide a concise summary of the issue? Does the current draft, at proposal-unified-class-features, still not address it?

[Download]

Really concise summary:

@decorator
class SomeClass {}

!==

@decorator()
class SomeClass

People are not happy about this.

Please just spend some time reading the feedback. People placed 150 comments in 2 years time. I can't summarize all that.

[littledan]

Why wouldn't overloading based on "does this look like a class descriptor" work here?

[dongryphon]

@littledan that wouldn't work for the OP which passes a class as a decorator parameter

[littledan]

@dongryphon I don't see the contradiction; a class looks completely different than a class descriptor, as this proposal uses. You can check typeof, for one--one is an object, the other a function.

[loganfsmyth]

@littledan Do class-level decorators get a descriptor? From the spec it seems like they are

function dec(cls: Class<T>, parent: ?Class<T>, elements: Array<ElementDescriptor>): { 
  constructor?: Class<U>, 
  finisher?: Finisher,
  elements?: <I forget>
};

@dongryphon Either way, class-level decorators don't just take a single argument anymore, so that should help disambiguation. The behavior in babel-plugin-transform-decorators-legacy does not match the current spec.

[allenwb]

@littledan The root cause here seems to be user confusion about the semantics and proper usage of DeclaratorMemberExpression. The feature design seems to be contributing to this confusion.

As specified in the proposal a DeclaratorMemberExpression is expected to evaluate to a "decorator function" ie, a function whose signature is rigidly defined by the specification. This means that when a DeclaratorMemberExpression is a DeclaratorCallExpression, the function that is being called with the provided Arguments isn't actually a "decorator function" (DF) but instead a higher order function that acts as a factory for DFs. Let's call such higher order functions a "Decorator Factory Function" (DFF). It's important to know that DFs and DFFs have completely different signatures and are called at different times in the class definition process.

However, the simplistic conceptual model of decorators that is likely to be taught (and which is implicit in some of the example in the specification) completely blur the important distinction between DFs and DFF. Consider the deprecate decorator from the specificaton:

import { deprecate } from 'core-decorators';

class Person {
  @deprecate
  facepalm() {}

  @deprecate('We stopped facepalming')
  facepalmHard() {}

  @deprecate('We stopped facepalming', { url: 'http://knowyourmeme.com/memes/facepalm' })
  facepalmHarder() {}
}

Is deprecate a DF or a DFF? It turns out that the usage decorating facepalm requires that it be a DF. However the usages decorating facepalmHard and facepalmHarder require that it be a DFF. That means a some points in processing class Person deprecate will be invoked as a DFF and at other points as a DF. How does it determine which kind of invocation it is responding to? How is deprecate actually coded to make this work?

export function deprecate(...DFFargs) {
  function deprecate_DF(elementDescriptor) {
     // body of the DF that is called to actually decorate elements.
     // note that it has access to the DFFargs that were passed to the DFF that created it
  }

  if (args.length == 0 or args.length > 1) return deprecate_DF;

  //if args.length is one then we don't know whether we are being invoked as a DF or a DFF
  //so we need to infer it by duck typing the argument.
  const arg = DFFargs[0];
  if (arg.kind && arg.property && "isStatic" in arg && arg.key && arg.descriptor) then {
     //looks highly likely that we are being invoked as a DF (but in rare cases we could be wrong)
     return deprecate_DF(arg);
  }
  //probably was called as a DFF with one argument.
  return deprecate_DF;
}

(Note that the duck typing would be easier and more reliable if the specification used a symbol to brand element descriptors.)

Any decorator that is going to access optional arguments is going to have to be written something like the above. This seems like a significant burden upon framework developers (or anyone who wants to define decorators). Also, this sort of confusion between the type of a higher order factory function and type of the functions that such a factory produces seems like a significant bad design smell.

It would actually be easier and conceptually cleaner if Decorator : @ DecoratorMemberExpresson always implicitly called the value of its DecoratorMemberExpression. In that cases "decorators" would exclusively be DFFs and the duck typing would not be necessary. Having to always code a decorator as a higher order function would still be a conceptual burden but it would be a smaller burden than have to code it as a conditionally higher order function.

[littledan]

@loganfsmyth It sounds like you're looking at an outdated version of the spec. In the current draft, there's a single ClassDescriptor given as an argument to the class decorator. Would this permit the overloading that the OP is requesting?

[dongryphon]

@loganfsmyth @littledan @allenwb Thanks for all the clarification. Now that the spec has evolved to passing an element descriptor vs the old Babel transpiler I can see a way through the mist. I still wish for the removal of the higher-order function approach (the "DFF" thing) and the ancillary type sniffing but it can be made to work. Appreciate the details!

[loganfsmyth]

@littledan Ah right, sorry there are so many different specs now that I've kind of lost track of which one is supposed to be the source of truth at this point.

[littledan]

@dongryphon I've seen type sniffing in existing decorators designed to serve double-duty as directly called functions, so it seems like this is already an OK practice in at least some of the community, for better or worse. If you have other use cases that can't be served this way, I'd love to hear about them.

[littledan]

@loganfsmyth Sorry about that; I put it on my todo list to clean up the repository situation here.

[allenwb]

@dongryphon I've seen type sniffing in existing decorators designed to serve double-duty as directly called functions, so it seems like this is already an OK practice in at least some of the community, for better or worse. If you have other use cases that can't be served this way, I'd love to hear about them.

Wait, that is not a good justification for a troublesome design. You see it already because, given the current design, it is the only valid way to code a decorator that accepts optional arguments. That doesn't mean it is "ok in practice". It just means that some people have found the way to work around the design issue. Have you look for places it was miscoded or talked to people using that pattern to see how long it took them realize it was necessary? How many decorators are there out there that are just waiting to fail because somebody puts a () after them?

As was pointed out up thread (https://github.com/tc39/proposal-decorators/issues/24#issuecomment-281804327) there is a two year old 150+ comment thread (https://github.com/wycats/javascript-decorators/issues/23) about this very issue. Why are the concerns expressed there and in this thread being blown-off in this manner.

The purpose of Stage 2 is at least as much about finding and fixing problems in a proposal as it is about pushing the proposal forward to Stage 3. Let's fix this problem.

[allenwb]

See issue #43 for a possible way to eliminate this problem.

[Download]

A bit disappointing to basically see the whole discussion we had two years ago be repeated now again.... But at least it seems this time the people working on the spec are actually paying attention.

Wait, that is not a good justification for a troublesome design. You see it already because, given the current design, it is the only valid way to code a decorator that accepts optional arguments.

So true! That's the whole reason the original discussion was created. Over 150 comments were added by community members, with all kinds of different solutions to this problem. But almost everyone involved agreed that it sucked that

@deco 
class MyClass {}

and

@deco()
class MyClass {}

Don't do the same thing. Now some people found ways to work around it by duck typing the incoming arguments... But in some situations this could not be done in a fool-proof way. I guess with the new proposal you can duck type this without having edge cases that are impossible to detect? E.g. in the old design if your optional param was a function you were in trouble because you could not detect whether the function was the class itself or the optional param... Is this solved in the new spec?

But still, with duck typing, the problem remains that authors of decorators need to opt-in to making the braces optional. If they don't do the duck typing trickery, then both code samples shown above might still end up doing completely different things... So users of decorators need to check for each and every decorator whether this decorator requires braces or cannot have them or whether it does duck typing so they can be used or not at will. Lots of confusion around a very basic use case.

[littledan]

I'm sorry, I'm not trying to blow off any concerns, but I won't have time to read the entirety that thread right now. I thought it'd be better for me to engage here than continue to ignore the issue (which I was not aware of until just now). I don't think any open source project contributors can read every single thing everyone says about it. Having 150 comments actually makes things harder, not easier, to get up to date on.

E.g. in the old design if your optional param was a function you were in trouble because you could not detect whether the function was the class itself or the optional param... Is this solved in the new spec?

This particular thing has changed. Rather than decorators being passed the whole class, they are passed a descriptor, which is an object and not callable.

But still, with duck typing, the problem remains that authors of decorators need to opt-in to making the braces optional. If they don't do the duck typing trickery, then both code samples shown above might still end up doing completely different things... So users of decorators need to check for each and every decorator whether this decorator requires braces or cannot have them or whether it does duck typing so they can be used or not at will. Lots of confusion around a very basic use case.

This is very true. If the semantics that everyone wants are that it always acts like this, and calls the argument, that's a really easy, small change. I don't really have a problem with it. It's pretty different from allowing overloading between functions passed as arguments and this thing being invoked as a decorator, though (which, as I explained above, changed since the thread happened).

I'm more skeptical of the class-based approach described above, for the reasons described in https://github.com/tc39/proposal-decorators/issues/43#issuecomment-337710197 . I think we could do just the () part without the part of treating it entirely as classes. What would you think of that approach, @Download ?

[jka6502]

I think I revitalised this issue with a comment that I subsequently deleted, once I realised that the spec had advanced since my last read - and because I naively assumed that the recently released Babel transform babel-plugin-transform-decorators exposed the latest Stage 2 approach - which it does not. It is effectively a bugfix and compatibility upgrade to the old legacy transform, but without the legacy suffix - slightly surprising.

That comment was motivated by the frustration of attempting to use the legacy decorator approach, again, and being unhappy that I could not consistently enforce @decorator == @decorator(), nor even detect accidental misuse, and throw - the variety of failure states, and subtleties of them are unnerving.

The approach in the latest spec certainly improves the situation, previously, one impossible edge case occurred when passing an optional constructor function, to, say, denote a type as a parameter - since there was no way to disambiguate between that, and an invocation of the decorator on a target class.

@Validate
class SomeType {

   @Validate(String)
   method(param) {}

}

Being able to duck type by inspecting the shape of the passed descriptor is better, but it still seems like there is a terrible code smell to having two possible interpretations of the same function - that adding or omitting braces changes semantics. That a decorator function can be either (or both), a factory, or (and) an explicit function, seems inherently dangerous - particularly since the onus is on the developer exposing decorators, to decide, and determine which use cases are valid. This will almost certainly lead to a wide mixture of libraries, with different approaches:

• some will insist that the developers RTFM, and explicitly always use, or omit braces / params
• some will try to unify both approaches by property detection
  • some will include bugs in that detection, for instance: if they encounter a parameter object with a kind property
• end users of such libraries will be put into a position of using a mixture of approaches
  • end users will have to commit to memory which decorators accept optional braces / params, and which do not
  • for decorators that do not, users will be required to know which require params, and which do not

It feels like a recipe for a lot of subtle bugs.

[littledan]

@jka6502 Thanks for writing out these thoughts. After all this discussion, which alternative are you most in favor i?

[jka6502]

@littledan - No problem, I actually like a few of the suggestions, though the most elegant, imho, would be adapting https://github.com/wycats/javascript-decorators/issues/23#issuecomment-190834352 to the new spec, something along the lines of:

function decorate(descriptor, ...args) {}

Where @decorate is exactly equivalent to @decorate(), in that they both invoke the decorate function on the following type, method (or property - once class properties stabilise, too) directly, with a zero length args spread, i.e. just the descriptor.

Alternatively, forcing a factory for every decorator seems viable too - its the mixture of both, and inability to determine which is occurring, that makes it problematic:

function decorate(...args) {
   return (descriptor) => { /* ... */ };
}

I'd imagine some would prefer to stabilise on decorators always representing a factory function, to provide simpler memoization based on arguments - though that could be achieved either way (I actually don't mind either, as long as the parameters of a decorator become consistent, regardless of its method of invocation - i.e., the author knows whether they are writing a factory, or a function):

function decorate(descriptor, ...args) {
   return someFactory.get(...args)(descriptor);
}

It seems like a simple grammar change can accommodate the optional nature of the arguments / braces (https://github.com/tc39/proposal-decorators/issues/43 - @allenwb opened that issue with a simple and clear explanation of the changes needed, and their precedent), too.

An interesting note, that I forgot from my previous comment, and that was raised in the previous issue thread, is that there are precedents for both approaches (@decorate == @decorate() in Java Annotations, and @decorate != @decorate() in Python decorators), and there will certainly be a significant set of developers that use whatever approach is finally chosen, with presumptions about how it works.

Whether you consider that "their own fault" or not, almost analogous existing mechanisms in other languages are going to influence the likelihood of people using decorators correctly (and thus the perception of the usefulness / difficulty of decorators), and, interestingly, those with Python decorators based preconceptions will provide consistent brace / arg usage, which will not fail with equivalent semantics (==), but those with Java Annotations based preconceptions will likely mix and match, which will cause regular bugs with current ambiguous semantics (!=) unless the decorator authors attempt to unify both styles, under the hood.

All of the potential solutions suggested affect the signature of decorator functions (or boiler plate class, in one suggestion), meaning that the requirement of deeper understanding lies with those creating decorators, rather than those using them - which sits very well with me (as someone who has encountered this, predominantly, while trying to write decorators).

[Download]

I think we could do just the () part without the part of treating it entirely as classes.

Couldn't agree more! If we can get @deco to be equivalent to @deco() I think that solves most peoples complaints.

[borela]

As in @jka6502's comment, I would prefer:

function decorate(descriptor, arg1, arg2, arg3) {}
// And I could use the spread operator if I need.
function decorate(descriptor, ...myArgs) {}

I am not sure the other option is a good alternative:

function decorate(...args) {
   return (descriptor) => { /* ... */ };
}

Correct me if I am wrong but this would create an instance of the function each time the decorator is called which could affect the performance as I am expecting decorators to be used heavily in UIs.

[jka6502]

@borela - On the second approach, perhaps it would be a tad slower, though the enclosed function is still precompiled, rather than entirely unique per invocation of the enclosing function, closures are very well optimised in every engine, since they are a common idiom - they are really just functions with additional 'arg' references defined in the enclosing call site.

I personally still prefer the spreadable args approach as well - just pointing out that anything that removes the ambiguity works for me (and the factory function approach clearly has some support - but is achievable manually, regardless of the approach) - I fully intend to use decorators for UI purposes too, but my, admittedly, naive testing has shown that the module load time influence of decoration is pretty negligible, even with heavy use (I've been using https://github.com/jkrems/babel-plugin-transform-decorators-stage-2-initial - though it appears to have died, and is incomplete, unfortunately).

[yGuy]

IMHO: I don't think it is absolutely necessary to actually "improve" the situation, here. I don't find the arguments so convincing. After all the problems that are presented are the very same problems every JS developer is facing every day; they are used to these problems and trying to "solve" this problem for decorators does not really improve the situation a lot. Consider that (non-trivial) decorators will not be used by total newbies (they have this slightly "enterprisey" touch that you won't be seeing in the majority of github code bases): If you compare the situation that we have in the "regular" JavaScript world, it's not that bad actually. Consider APIs for geometric points, e.g.: some will have have a "function x()" as accessor to the x "property" like this:

p.x()

and some will have properties or fields:

p.x

Unless you RTFM you will run into problems if you use it the wrong way at run-time. And in the case of the first type of API the problem will be extremely hard to debug because doing p.x * 5 in the function case will not trigger an immediate error but will cause all kind of issues later on somewhere else in the code when the NaN will bite you.

The same holds true for omitting a required argument or passing too many arguments to most APIs out there - many of them will not make your logic fail immediately but all kind of side-effects will crash the application or make it behave unexpectedly somewhere later on. These are the kind of problems devs have to deal with every day in the JS world. And they can only avoid those problems if they know the API and/or have read the docs. So why should this spec jump through hoops in order to improve the situation specifically for decorators, only, sacrificing flexibility and possibly performance at the same time?

If API vendors/authors can come up with a consistent API and carefully implement their decorators and properly write their documentation, devs will be able to work with them in no time.

Don't get me wrong: I think simplification and removing possible pit falls is a good thing. But not for any price. So keep in mind that many devs are OK with APIs that could be "ambiguous" - that's the situation they are facing today, anyway most of the time: if the APIs are properly documented and implemented, they will be fine with that. APIs which aren't consisted or properly documented or a pain to use will die anyway soon, one would hope. Devs won't be happy, however, if they need to sacrifice performance are even be forced to like always have to append () or always need to implement a decorator as a class. That's not what they would expect from a language that's as flexible as JS, I think. At least that's not what I would expect the spec to look like.

[littledan]

@jka6502 We're no longer actually passing a target in. Instead, there's a descriptor passed in, and another descriptor passed out.

It sounds like the biggest thing that this thread is asking for is to make @x be the same as @x(). This should be pretty easy to do in the specification; if we make a change here, my plan would be to make this narrow change, but keep everything else the same. How would that be?

[borela]

@littledan that's perfect for me, the rest of the specification seams fine as I only had trouble with optional arguments and detecting whether the class passed in was the target or just an argument.

[yGuy]

For the record: I am pretty sure that not everyone agrees that @x be the same as @x() if you look at it from a higher level. In fact I am very positive that for the vast majority of APIs everyone agrees that someObject.someName is something different and should not magically behave the same way as someObject.someName(). Why should decorators be treated differently, here? The current solution does work. JavaScript works. Devs are used to these "subtleties", invocation makes a difference and is something specific to the API that I am using. Why change it just for decorators and force one way of doing it, if both ways work?

[dongryphon]

Why should decorators be treated differently, here?

They already are treated differently here in that foo is invoked when used as:

    @foo
    class A {}

This invocation is part of the syntactic sugar. So I don't see the analogy as applicable here. The suggestions so far boil down to making the difference between @foo and @foo() easily determined vs horrible/impossible-to-be-certain (as per current spec).

[littledan]

I guess the difference depends on whether you see @foo() as something with an arbitrary expression foo(), or whether you see it as a sort of declarative form, which might optionally take arguments in parentheses, and then if you really dig into it, you see that using parens is just part of the expression, and leaving them off results in something totally different. I can understand why people would fall into the latter camp, even if, from a language designer perspective, the former seems obviously correct.

[jkrems]

I was slightly in favor of "magically added parenthesis" but the example of @obj.prop makes me wonder if that's really a good idea. I'm already running into people who get confused why new X and new X() do the same thing. Adding () if the decorator accepts parameters doesn't seem like a huge burden.

[borela]

@awesomeDecorator
class SomeClass {
  @deprecate
  myPropA = 1

  @deprecate('custom message')
  myPropB = 1

  @sub.deprecate
  myPropC = 1
}

// vs

@awesomeDecorator()
class SomeClass {
  @deprecate()
  myPropA = 1

  @deprecate('custom message')
  myPropB = 1

  @sub.deprecate()
  myPropC = 1
}

It is cleaner without the unnecessary parenthesis.

[yGuy]

@dongryphon

I see people writing this:

document.addEventListener("foo", something.fooListener()) 

where it should be

document.addEventListener("foo", something.fooListener);

Just because fooListener is invoked eventually doesn't mean they are the same. Adding () to them implicitly, because you could do that, would be very bad, here.

Correct me if I am wrong, but with the current state of the spec it's not impossible. With the updated spec, it can be implemented to work except for the one single very awkward use-case where your "factory" method takes as arguments a descriptor. Only for those decorators where you would want both, no-arg and argument usage, you would have to implement the switch in the code. Good library authors can make it such that both approaches will work or document it explicitly that their library is different and only works with parenthesis. Bad library authors will do neither, but then again, who will use their APIs, anyway? Except for a handful decorators, most useful decorators will require arguments anyway. But having to implement the simple no-args decorators as factory methods "just because" is not nice, IMHO. I would certainly hate it every time I wanted to implement a simple no-args decorator. I think what @jkrems said is a good example of why it should not be done. I hate it every time I see new X without the parenthesis and I believe many will agree that only code minifiers should use this "optimization" (https://eslint.org/docs/rules/new-parens). Implicitly adding parenthesis does not make the language easier!

[dongryphon]

It's not really the same thing to say you are "calling the decorator function" in one case and not the other since it is called immediately in all cases. But this analogy to all other things "function" is really an implementation issue imho. The idea behind decorator syntax is (as I understand it) to feel declarative.

Given:

import { deprecate } from 'core-decorators';

class Person {
  @deprecate
  facepalm() {}

  @deprecate('We stopped facepalming')
  facepalmHard() {}

  @deprecate('We stopped facepalming', { url: 'http://knowyourmeme.com/memes/facepalm' })
  facepalmHarder() {}
}

Under the current spec, one would have to write this to process the ambiguity (from here):

export function deprecate (...DFFargs) {
    function deprecate_DF (elementDescriptor) {
        // body of the DF that is called to actually decorate elements.
        // note that it has access to the DFFargs that were passed to the DFF that created it
    }

    if (args.length == 0 or args.length > 1) return deprecate_DF;

    // if args.length is one then we don't know whether we are being invoked as a DF or a DFF
    // so we need to infer it by duck typing the argument.
    const arg = DFFargs[0];
    if (arg.kind && arg.property && "isStatic" in arg && arg.key && arg.descriptor) then {
        //looks highly likely that we are being invoked as a DF (but in rare cases we could be wrong)
        return deprecate_DF(arg);
    }

    // probably was called as a DFF with one argument.
    return deprecate_DF;
}

A better conversion of the syntax would make all forms easily recognizable:

export function deprecate (descriptor, ...args) {
}

For decorators that do not care you would simply not have anything beyond descriptor. I personally find this mapping simpler in every way. The use of a HOF here when @deprecated(...) is used is merely one (confusing) way to map the syntax to the function that implements it.

[yGuy]

The good thing about the HOF/factory/currying variant is that you can do costly preprocessing easily and reuse certain "instances" of decorators.

Would the following still be possible with the "new", simplified implicit parenthesis approach?

import { deprecate } from 'core-decorators';

const hardDeprecation = deprecate("STOP IT, STUPID, It's too slow!", fibonacci(4242));

class Person {
  @deprecate
  facepalm() {}

  @hardDeprecation
  facepalmHard() {}

  @hardDeprecation
  facepalmHardAgain() {}

  @deprecate('We stopped facepalming', { url: 'http://knowyourmeme.com/memes/facepalm' })
  facepalmHarder() {}
}

Wouldn't implying the parenthesis make this kind of currying impossible? With the old spec, it was easily possible to implement the above.

[borela]

@yGuy

Why would it make it impossible?

const hardDeprecation = (descriptor, ...args) => deprecate(descritor, ...args, "STOP IT, STUPID, It's too slow!", fibonacci(4242))
// Or...
const hardDeprecation = (...args) => deprecate(...args, "STOP IT, STUPID, It's too slow!", fibonacci(4242))

[yGuy]

OK - "impossible" is the wrong word - I can always "decorate a decorator" and implement a new one that calls the former like you have shown. Still with your approach it would be one additional function call for each usage. That may not sound like a lot, but for example the API that I was using decorators (modeled after the old spec) for, there were more than 10K decorator usages. I was hoping to be able to get rid of these extra calls, because they are likely to slow down application startup.

I simply don't like the fact that writing decorator annotations in two different ways would be required to result in the same behavior because the parenthesis would be added implicitly if omitted. What would that mean in this case:

@decoratorFactory.createMegaDecorator()
class Test {}

// same as?
@decoratorFactory.createMegaDecorator
class Test {}

would that be different from this

const megaDecorator = decoratorFactory.createMegaDecorator()

@megaDecorator
class Test {}

// same as?
@megaDecorator()
class Test {}

or even

const megaDecorator = decoratorFactory.megaDecoratorFactoryFunction

@megaDecorator()
class Test {}

// same as this?
@megaDecorator
class Test {}

I think that would be really confusing (the part that they would all behave identically). What would you expect to happen here?

I am against the "automatic parenthesis insertion" because "refactorings" like above would not be trivial to do and understand anymore.

[Download]

There are definitely things to say for both making the with/without parenthesis cases equivalent and not doing so. However, I feel that the use cases described where the current situation is confusing vs those where treating with/without parentheses as equivalent is confusing are clearly the more basic use cases.

The examples shown where decorators are properties of other objects, or where those properties are functions creating decorators, are imho much more contrived than say the @deprecate example. In fact I see a trend in JS toward less OO-like use and more simple functions. Consider stuff like

addHandler(({x, y, z}) => {
  // object deconstuction is used to get direct references to members x, y, and z
})

The trend seems to be to get direct references to anything. So given a megaDecorator, I expect actual real-world use to look more like this:

const { createMegaDecorator } = decoratorFactory
const deco = createMegaDecorator()

@deco
class MyClass {}

Of course, this stuff is all personal. But looking at real-world examples, my feeling is that the feature to be able to easily write and use decorators with/without parentheses is more important.

[yGuy]

@Download: I totally agree with you - it should be possible to implement decorators that do not require parenthesis. My point was that this is already possible and can be implemented even without the forced braces behavior that I am opposing. The thread started off that it would be hard for the implementers of the decorator to implement support for both usages. This is not the case. For those decorators that will be used by lots and lots of people, developers will add that feature. But for those cases where you do not want the automatic parenthesis insertion, with the proposed solution this would become impossible.

TL;DR; : we can (and will) have @deprecate and @deprecate("foo"), even without the proposed "force-as-call"-constraints. The new proposal would be more restrictive and that's why I would be against it. It will not become difficult to implement decorators that support both variants. As this thread has shown this feature can be added easily on top of the current spec and we don't use the flexibility.

[Download]

@yGuy Note how I stressed and use. If we leave the behavior up to the implementors, users of decorators will never be sure of that behavior without first checking the docs for each decorator they are using. Also there will be differing / wrong implementations. Please have a look at the current use of decorators with the old proposal and you will see that this already started to happen. It becomes a wild-west/jungle situation where each decorator has to be treated differently.

Sometimes, being more restricitive is a good thing. See also new keywords such as const, which are there to allow programmers to enforce stuff. More restrictive often translates to more clear, simpler. The uses of decorators where adding/leaving off braces would give extra functionality are much more advanced uses than just using @deprecate vs @deprecate() vs @deprecate('use function X instead').

Also I say you can rewrite every instance of decorator use where this would actually matter and the code would end up being simpler and more easy to understand.

[yGuy]

@Download Re: Having to look up documentation: Are you saying devs would really be using some decorators without having read or knowing the documentation? Then how should they be using classes and how should they find out how to use a function, what parameters need to be used, provided, etc. How can they even know that they are allowed to use the decorator without any arguments without reading the documentation? In JavaScript you can never be sure what to expect without reading the documentation - in fact most of the time you cannot even without reading the sources ;-) ! Don't forget that the whole discussion is just about decorators that do not have any arguments. How are you supposed to use those decorators "without reading the documentation?"

Then again: This new additional restriction will not prevent implementors from creating wrong implementations, either. They will mess up some of the contracts or create buggy decorators, either way. However this restriction will be a restriction for those who could write more sophisticated code or want to get that last tiny bit of performance boost.

Why should decorators feel totally different than anything else in the JS world? I don't understand why we are sacrificing flexibility (and adding more magic at the language level!) here just to make it slightly more difficult to shoot yourself in the foot in this specific situation when this is what devs are doing all the day: shooting themselves in the foot because they can!

To be honest: I either don't understand your other two paragraphs or I don't see how they should be an argument in favor of the proposed restriction.

Last but not least I would like to add that if we add this restriction we are clearly changing it for all typescript users, which are used to doing it like this since day one:

Decorators use the form @expression, where expression must evaluate to a function that will be called at runtime with information about the decorated declaration.

from: https://www.typescriptlang.org/docs/handbook/decorators.html

The new spec would have to differ and would result in the more complex wording somewhere along the following lines:

Decorators use the form @expression-or-function-invocation, where expression-or-invocation must evaluate either to a function that will be called at runtime with information about the decorated declaration or be something that looks like a function-invocation which actually isn't one and that won't actually be called immediately, but only at runtime and then with different arguments.

Is this really what you want the spec to look like (not literally, but I hope you see what I mean) ?

There is a similar discussion going on in the "dynamic imports" spec. An import looks like a regular function invocation, but they are debating whether it actually should behave like one.

I feel that we have the same issue here: If the decorator looks like a regular function invocation, it should be one. If you want to use a declarative approach that works differently with more magic involved, the whole syntax should be changed instead, so that it doesn't look like an ambiguous invocation anymore. If this is how this spec ends up, I would rather like to see a totally different, unambiguous syntax like e.g.:

@deprecated{reason = "It's ugly!"}
class DeprecatedClass {}

function deprecate (descriptor, decoratorConfigObject) {
   console.log(decoratorConfigObject.reason); 
   .....
}

That's actually a bit more similar to the way Java and c# do it: Java Annotations use a special syntax for passing arguments to the annotation, which can only used for annotations. So did c# initially, although in newer releases with default and named arguments method calls look quite similar, too, now. None of them allow complex expressions, though, but always require the type, directly.

So maybe it's either arbitrary expressions that need to evaluate to a function just like in TypeScript or a very strict syntax that does never allow arbitrary expressions like in Java and c#. And only in the latter case you could allow the "force parenthesis" approach, possibly using a totally different syntax.

I would feel that the arbitrary expression without forced invocation/parentesis solution is a lot more JavaScript-like.

[jka6502]

@yGuy

Determinism vs non-determinism

The reason those of us suggesting the function decorate(descriptor, ...args) {} syntax are so polarised is because there is simply no way to absolutely determine intent from the current spec - we simply cannot achieve our goal. Its a vast improvement on the previous, stage 0/1, (constructor, key, descriptor) situation - but it is still non-deterministic - a descriptor like object cannot be a singular parameter to a decorator, and there is no way for an decorator author to absolutely, deterministically, separate those cases.

In contrast, every counter example you have provided is still possible, but, perhaps, requiring a change in how it is applied.

The suggested implementation above (which necessitates @dec() == @dec) is, of course, merely a preference, and has been expressed as such - and we all have a right to our preferences - but the crux of the matter is that the choice of usage of a decorator implies vastly different intent - is my decorator a function that applies to a type, or type member, or is it a higher order function, that returns a function, that applies to a type, or type member? Should there be a case where a single function absolutely has to be both a direct function, and a HOF of itself? Would you use that anywhere else?

I'm not entirely bound to the solution discussed, but I believe the above needs fixed somehow.

You list an example in https://github.com/tc39/proposal-decorators/issues/24#issuecomment-341105943:

@decoratorFactory.createMegaDecorator()
class Test {}

// same as?
@decoratorFactory.createMegaDecorator
class Test {}

Why wouldn't they? What would you expect to happen? Something clearly denoted as a factory, that returns a decorator function, by your own naming, should be capable of being applied as a direct decorator function too? Why would you want that? Do you have an actual use case where differentiating is beneficial? Perhaps I'm just failing to see the wood for the trees here, but all of these examples seem to merely highlight the ambiguity.

Also, in https://github.com/tc39/proposal-decorators/issues/24#issuecomment-340517869:

import { deprecate } from 'core-decorators';

const hardDeprecation = deprecate("STOP IT, STUPID, It's too slow!", fibonacci(4242));

class Person {
  @deprecate
  facepalm() {}

  @hardDeprecation
  facepalmHard() {}

  @hardDeprecation
  facepalmHardAgain() {}

  @deprecate('We stopped facepalming', { url: 'http://knowyourmeme.com/memes/facepalm' })
  facepalmHarder() {}
}

What if the implementer decided that this should only be a factory function, the @deprecate instance would silently drop a decorator function on the floor, by the old spec, and give a very unclear error message, in the new spec (probably some failure to interpret the returned descriptor).

The very example you cite aims to unify usage, for exactly the same reasons: https://github.com/jayphelps/core-decorators/blob/master/src/deprecate.js#L26 https://github.com/jayphelps/core-decorators/blob/master/src/private/utils.js#L22

Alignment with existing JS concepts

Where does this argument stop, could additional parameters raise to a second order, should we allow @decorate(1, 2, 3)(4, 5, 6)? Should we allow arbitrary expressions?

I would argue that this is a new feature, which doesn't precisely align with any existing concept in JavaScript - similar yes, the same, no. If it were, surely we'd wrap the type, or member, in parenthesis, to clarify that this is a function invocation on that element? (@decorate(@deprecate(class Type {}))).

How do multiple decorations of one member compose? Does that align with existing JavaScript semantics (also, hasn't the composition order changed, due to the previous spec applying descriptors between - meaning that @nonconfigurable or @readonly needed applied last...)?

class SomeClass {
   @deprecate
   @readonly
   property = 7;
}

I do agree that keeping a simple, consistent, approach to as much of the language as possible is very valuable, it is one of the factors that continually draws me to JavaScript - but I don't believe this is being violated in any way here, regardless of approach - as has been pointed out in this discussion, decorators are declarative - this does feel a bit like forcing a square peg through a round hole, because it is "most like the round pegs".

Also, neither Java, nor C# have custom calling mechanisms for Annotations - they use parenthesis (and both treat @Annotation == @Annotation(), or [Annotation] == [Annotation()]). The only variance introduced is that Java adopted named parameters, to prevent ugly ordinal param problems (think @Annotate(null, null, 0, "Only bit I'm interested in")).

The good thing about the HOF/factory/currying variant is that you can do costly preprocessing easily and reuse certain "instances" of decorators.

HOF maybe (though only sometimes), but currying? Would you actually curry that way? At worst, this is premature optimisation, at best, confusing syntactic sugar, for something entirely achievable regardless of the approach taken - decorators can always be the result of currying factories, the argument here is that they shouldn't absolutely have to be currying factories, in every case, too (as well as direct functions).

function doSomethingToNumber(message, number) {}

// Would you expect passing something other than a number to cause that function to self-curry, and return a variant of itself?  I would raise this as a serious, serious code smell in review, in my place of work.

let a = doSomethingToNumber('Damnit Dave', 7); // TADA - Number returned
let b = doSomethingToNumber('Damnit Dave'); // Should this return doSomethingToNumber(arg), with curried second parameter?  Really?

// I've never seen the above in practise anywhere - surely, you'd curry from outwith the function:
function curryMessageParam(message) {
   return function(number) { return doSomethingToNumber(message, number); };
}
let doSomethingToNumberSpecialised = curryMessageParam('Damnit Dave');
// But, equally, I wouldn't expect: curryMessageParam('Damnit Dave', 7); to magically directly invoke, either...

Prior art

I am against the "automatic parenthesis insertion" because "refactorings" like above would not be trivial to do and understand anymore.

Last but not least I would like to add that if we add this restriction we are clearly changing it for all typescript users, which are used to doing it like this since day one:

Decorators use the form @expression, where expression must evaluate to a function that will be called at runtime with information about the decorated declaration. from: https://www.typescriptlang.org/docs/handbook/decorators.html

This is a stage 2 proposal, quantity of adoption of an unstable spec, and difficulty in adjusting as the spec moves forward simply cannot influence the end result - that would be madness, even the very link you provided, after the first paragraph, warns of this:

NOTE  Decorators are an experimental feature that may change in future releases.

Every proposal could end up tethered to its initial first draft by that rational - Isn't the staging process designed to prevent exactly this?

[dongryphon]

I think it is unhelpful to focus the discussion on any of the following snippets around "function calls". This is because the goal of the snippets below is not to make a function calls (the fact that deprecate is even a function is irrelevant to the user):

    @deprecate
    class Foo { ... }

    @deprecate()
    class Foo { ... }

    @deprecate("whatever")
    class Foo { ... }

The goal of the above is rather to decorate a class with appropriate metadata. Because all of this is declarative in nature, I believe the vast majority devs using decorators would reasonably expect the same outcome in the first 2 cases above.

This expectation is only called into question (afaict) when devs learn that decorators are implemented as functions.

Whatever the above de-sugars into is not really the main point. How many functions and calls are involved here is a distant secondary concern... except as it relates to how implementations would achieve the desired and expected outcome... ideally without even thinking about it.

[Download]

Because all of this is declarative in nature, I believe the vast majority devs using decorators would reasonably expect the same outcome in the first 2 cases above.

Yes! Thank you!

If you want to use a declarative approach that works differently with more magic involved, the whole syntax should be changed instead, so that it doesn't look like an ambiguous invocation anymore. If this is how this spec ends up, I would rather like to see a totally different, unambiguous syntax like e.g.:

@deprecated{reason = "It's ugly!"}
class DeprecatedClass {}

You know, I actually like this idea!

But all things considered, I am ok with the current syntax. In fact I'd say I'm ok with the entire spec, except for the way it treats the two cases as completely different when semantically there is no difference. In both cases we have a decorator without any arguments. Let it be the same.

[yGuy]

@dongryphon @Download Don't get me wrong: I have to say it again: I am totally with you in that I also want the deprecate example to behave the same in both cases (with and without parens). However I don't want to make the new spec/API not feel like JavaScript and I believe with all the magic that you want to add to the behavior it would not feel like JavaScript anymore. I think that the cost for implementing the decorator once "properly" in a way that it will be used thousands of time is totally negligible. This is a price I would expect developers to pay (it's a one time cost!) and we would gain a simpler, easier to understand, more flexible, more efficient and performant API that involves a lot less magic. The solution to this would be the way typescript does it. If you change the spec so that the annotation is not "a simple function" call anymore but something way more magical, then you should consider disallowing all more complex expressions (to avoid the new ambiguity of expressions that only yield decorator function instances ("createMegaDecorator")) and maybe even consider getting rid of the function invocation-like syntax to make it clear that this is not a function invocation but a "declarative" decorator. But this would not feel like JavaScript anymore, at least for me. So I am in favor of the alternative without the magic but with the slightly more difficult implementation.

[yGuy]

@jka6502

a descriptor like object cannot be a singular parameter to a decorator, and there is no way for an decorator author to absolutely, deterministically, separate those cases.

Yes, I agree - but I still think this is a very low prize to pay - can you think of any reasonable use-case where you would want to have a descriptor like argument as the first parameter of a decorator "constructor"? I cannot. If we made the descriptor a proper type that can be checked with instanceof (and maybe only constructible by the VM so that you know that this cannot be a "real" argument), then this can be avoided completely, IMHO. Just my 2c.

[dongryphon]

I guess I am not seeing this de-sugaring as violating any JS concept:

    @whatever.expression(1, 2)
    class Foo {}

    // becomes:
    whatever.expression(descriptor...stuff, [1, 2])

    // or:
    whatever.expression(descriptor...stuff, 1, 2)

[MMeent]

@whatever.expression(1, 2)
class Foo {}
// becomes:
whatever.expression(descriptor...stuff, [1, 2])```

Hmm. What would happen here if you'd do this?

@decorator.call(thisarg, args)
class foo {}

The desugarization would be nontrivial for normal js developers. Yes, it would be bad API design on the side of the decorator-library, but it's a valid use case I guess?

On the 'inexplicit parens on decorators', a similar construct exists in JS with the new operator: new MyClass is the same as new MyClass(), so it wouldn't be that difficult to explain.

[dongryphon]

I would expect this:

@decorator.call(thisarg, args)
class foo {}

To translate (given the above idea) in to:

class foo {}

decorator.call(descriptor...stuff, thisarg, args)

Would it work? Depends on what decorator.call is...