Skip to content
This repository has been archived by the owner on Jan 25, 2022. It is now read-only.

Inline Class Decorators #22

Closed
dongryphon opened this issue Oct 7, 2016 · 14 comments
Closed

Inline Class Decorators #22

dongryphon opened this issue Oct 7, 2016 · 14 comments

Comments

@dongryphon
Copy link

For your consideration.

Greetings! I work at Sencha and we have spent considerable time in recent months deciding how we will use decorators and how they could replace the various aspects of our Ext JS class system.

Because these features are class-level concerns, we are seeing this unfortunate pattern emerge:

    @decoratorA({
        // maybe 1-100 lines
    })
    @decoratorB({
       // another long thing (maybe some html template)
    })
    class MyComponent extends Component {
        // empty? yes... often!
    }

I would like to prose an inline alternative syntax that would be equivalent to the above:

    class MyComponent extends Component {
        @decoratorA {
            // maybe 1-100 lines
        }

        @decoratorB {
           // another long thing (maybe some html template)
        }
    }

Basically a decorator name followed by an object literal. This maintains the aesthetic flow of describing the contents of a class within its body and does not "bury the headline" so to speak (that a class is being declared here).

You could see the benefits of this approach better with some simple decorators like @internal (a debug aid to detect accidental name collisions) or @statics (for better compression) or @prototype (to put data on the class prototype):

    class MyComponent extends Component {
        @statics {
            create () { ... },
            // many more static methods
        }

        @prototype {
             answer: 42,
             defaultText: 'Hello world'
        }

        @internal {
            deepThought () { ... },
            moreDeepThoughts () { ... }
        }
    }

For what it's worth, you see this same top-heavy class pattern in Angular 2 as well. The large numbers of lines above the class keyword marginalize the class statement.

I look forward to your thoughts. If this is not the right place to post suggestions, apologies in advance and I would greatly appreciate a pointer to the proper place. Thanks!

@jkrems
Copy link

jkrems commented Oct 7, 2016

My two cent: This seems to promote a "kitchen sink" approach to using decorators. E.g. @statics could just be normal static create() {} methods, @prototype looks like class properties, @internal could be private state, symbols, or at least method decorators. Especially since that decorator-heavy style makes static analysis / tooling almost impossible.

If a class contains nothing but tens or hundreds of lines of decorators, that sounds like a potential design issue in the class/framework. In the case of angular 2 there's solutions like pulling out huge strings into "real" code / constants instead of inlining everything in the decorators. I don't think it would be good to encourage others to fall into the same trap that angular fell into ("Let's have empty classes that consist of nothing but decorators!").

@dongryphon
Copy link
Author

dongryphon commented Oct 7, 2016

Thanks for taking the time to reply so quickly. While I agree Angular 2 has perhaps too much fondness for class decorators I understand their conundrum. :) Classes can currently contain only one thing: methods (well, 2 if you count static methods separately).

The @prototype suggestion there highlights the situation best I think (the other two are much less interesting or important). The current situation is this:

class Foo {
}

Foo.prototype.bar = 42;  // not tool friendly

While tools could detect this, imperative code is not always so clear ;) Using @prototype is more tool friendly:

class Foo {
    @prototype {
        bar: 42
    }
}

Of course tools can only deal with the decorators they understand so really decorators are always going to be more tool friendly than imperatives.

In the above case, if you include documentation in the mix you have:

/**
 * Some lengthy comments about the Foo class.
 */
class Foo {
    @prototype {
        /**
         * Some lengthy comments about the bar property.
         */
        bar: 42
    }
}

Using the current approach, the structure makes much less sense. You either have this:

@prototype({
    /**
     * Some lengthy comments about the bar property.
     * ?? how is this connected to Foo??
     */
    bar: 42
})
/**
 * Some lengthy comments about the Foo class.
 */
class Foo {
}

Or this:

/**
 * Some lengthy comments about the Foo class.
 * ?? Fun times connecting this with Foo. No regex approaches here! ??
 */
@prototype({
    /**
     * Some lengthy comments about the bar property.
     */
    bar: 42
})
class Foo {
}

Connecting doc comments with their associated target is complicated by mixing decorators in between, which is why the first is more likely the way many tools would have to be appeased. But since the Foo class document fragment has yet to be encountered, these properties will also take some effort to connect with the associated class.

@jkrems
Copy link

jkrems commented Oct 7, 2016

Maybe this is a misunderstanding - the example you gave can be implemented a lot nicer using public class fields which is what I meant by "class properties" (I think that was an earlier name of the proposal?):

/**
 * Some lengthy comments about the Foo class.
 */
class Foo {
  /**
   * Some lengthy comments about the bar property.
   */
  bar = 42;
}
console.log(new Foo().bar); // 42

It's not 100% the same thing (bar will be set on the instance) but I think it's easier to understand. For things that are supposed to be shared across instances, you can just use a static field:

class Foo {
  static bar = 42;
}
console.log(Foo.bar); // 42
console.log(new Foo().constructor.bar); // 42

@dongryphon
Copy link
Author

I am really using @prototype as an example of a general class of decorators whose role is to add members to the class in some way. As such they fit more logically inside the class declaration then above it. The Ext JS class system (the framework I manage) has other similar concepts.

While public class fields are quite useful, we use prototype fields to great effect since they are transparently overridden by instance properties. As in this.foo will be 42 unless that particular instance has a different idea in mind (think button text perhaps which defaults to "").

@jkrems
Copy link

jkrems commented Oct 7, 2016

But I assume you're using those class system features because you're providing features that didn't exist in ES5. In a world with real classes, the following "just works", without a custom class system:

class Button {
  color = 'blue';
}
class GreenButton extends Button {
  color = 'green';
}
const b = new Button();
console.log(b.color); // 'blue'
b.color = 'red';
console.log(b.color); // 'red'
console.log(new GreenButton().color); // 'green'

I get that these are just examples but my point is that "adding class members" isn't as big of a use case in ES6 classes as it was in ES5 "classes". :) As an example you could look at react's transition from React.createClass to class syntax.

@dongryphon
Copy link
Author

True there isn't as big a use case as pre-ES6... In the past we added methods and static methods so those are blessedly no longer an issue. :) But while ES6 and additional yet-to-be-finalized language features have made things much better, there really is a very wide range of things that folks will want to do with decorators. Not only Angular2, but Java and C# ecosystems demonstrate how widely this type of feature gets used (and abused no doubt) and for things well beyond these basics. :)

In the case of instance vs prototype members, we would not be up for paying the price of promoting all members currently inherited via prototype to instance members. In our component framework we have many classes with many such members and most instances override but a few. The new class keyword is great for what it does but the prototype system is not something we want to lose in the process.

A slightly more detailed example of this kind of thing is our configuration system. In this system we provide a boilerplate setter method for properties that need to be dynamically adjustable with related side-effects. The syntax for this would be much like the hypothetical @prototype decorator but would entail more interesting work per property.

@jkrems
Copy link

jkrems commented Oct 7, 2016

Prior art would definitely be a good argument. Do you have an example of annotations/decorators/attributes for the class appearing in the class body from Java/C#? So far I've never seen it in either language but I'm definitely no expert on their full language specs. :)

Is this mainly a performance argument (I assume memory..?) for the @prototype use case? If so, would a @prototype field decorator be a valid solution?

class X {
  @prototype // magical decorator that turns this into a prototype property instead of an instance field
  y = 42;
}

The "generate getter/setter methods" use case sounds a lot like the ember example in the current proposal text:

import DS from 'ember-data';
import { attr } from "ember-computed-decorators/ember-data";

export default class extends DS.Model {
  @attr firstName;
}

I'm not trying to argue that decorators aren't super useful for a wide range of use cases. It's just that there are 2 types of decorators right now: One is explicitly meant to introduce/extend/modify class members and one to express something about the class itself. Moving the latter into the class body seems potentially confusing.

@dongryphon
Copy link
Author

No prior art on inline class decorators in C# / Java... but you couldn't adjust the content of a class in those languages anyway :D

@dongryphon
Copy link
Author

Yes, perf of initialization of all classes and then perf of creating each instance is why we use the prototype. A per member function call to place each property on the prototype is probably not something we'd use but I see your point here.

The config property situation is possibly something that could fit the form of member decorators. It would be more expensive then processing a bulk of them in a loop but that would need some measurements to see.

@dongryphon
Copy link
Author

I agree that there is an elegance in the proposal. Whatever is finalized, it is a big step vs no decorators. My suggestion here is to see what others think about this category of thing (maybe not a decorator?) that best fits in the class body but is not directly connected to some declared member of that class.

The @prototype fellow would be a super simple and performant decorator vs the per-member flavor. The config properties concept is far-and-away our next biggest fish to fry. Beyond that... well I am curious to see if that excites discussion :)

@dongryphon
Copy link
Author

Maybe this should work?

class Foo {
    @prototype({
        bar: 42
    })
    constructor () {
        //...
    }
}

Currently this throws an error, but this would be more consistent with the concept of decorating things that follow them.

Any ideas what this is deemed an error?

@dongryphon
Copy link
Author

Moved this last one to a separate issue - tc39/proposal-decorators#23

@littledan
Copy link
Member

Would it be OK to leave this as a follow on proposal? I am a little sceptical of multiple styntactic forms for the same thing out of the box, but maybe we will develop a really strong need for it over time. If you want an indication that decorators proceeding a class declaration relate to the class, would a comment work? Or, did you intend for this to be the only syntax?

@littledan
Copy link
Member

Let's move discussion to tc39/proposal-decorators#49 .

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

No branches or pull requests

3 participants