@bound example - documentation suggestion

[mbrowne]

It has become a common practice (especially in the React community) to create bound methods using a property initialized to an arrow function:

class MyComponent extends Component {
    handleClick = (event) => {
        ...
    }
    ...
}

So at first I was surprised to see that the readme for this proposal has a @bound decorator as one of the examples, given that using an arrow function seems more straightforward. Then I found this:
tc39/proposal-class-fields#80

...and I remembered that I myself had previously encountered issues with inheritance when using arrow functions in this way.

I'm guessing that this is why a @bound decorator is used as an example - since it's a more consistently readable way to bind methods, and thus preferable to an arrow function.

In any case, it could be helpful to mention something about this in the readme, in case others also wonder, "Why not just use an arrow function"? I understand if this is a low-priority change, but thought it was worth suggesting.

[littledan]

I'm not sure that the performance property you're referring to is predictably the case across implementations. I'd rather not use performance as the design principle here. Let's continue discussion about more appropriate examples on the other issue that you linked to above.

[ljharb]

A bigger downside than performance of this approach in the React community is that it makes it very hard to mock out these functions in tests. Because it's common, people invariably run into this problem, file bugs on enzyme, and are corrected and told that arrow functions should never go in class properties, stop using them, and then don't run into problems after that.

[mbrowne]

Actually I wasn't referring to performance, but rather issues relating to the method not being on the prototype at all anymore when using a property initializer and arrow function. As I said in the other issue, I don't think the Medium article's claims about performance are even correct. I'm happy to continue the discussion in the other thread, but just to make sure -- you saw that that issue is in the class fields proposal, right? Wouldn't this repo be the more relevant place for a discussion about decorator examples?

[littledan]

Yes, I agree that this repo is better for such discussion. Sorry for my misunderstanding.

[mbrowne]

Reposting my suggestion from the other thread (to make the discussion easier to follow):

how about a @readonly decorator?

@littledan

@mbrowne The examples that are there now contribute to a working code example. Having a bound function is actually necessary for the code to execute. Are there any decorators which might be more directly involved in functioning that could be referenced?

There are certainly realistic examples of a @readonly decorator that I could suggest...I don't know if a Counter element is the best place for it. I was thinking of something like an id property, e.g.:

class User {
    @readonly id
    ...
}

[doodadjs]

@mbrowne

I think of "@configurable(onoff)", "@enumerable(onoff)" and "@writable(onoff)". Where onoff is either true or false.

"@readonly" could be a combination of "@configurable(false)" and, if there is no getter/setter, "@writable(false)". If there is a setter, it could throw an error.

[mbrowne]

Sure, that would work. For the sake of an example I'm not sure all those separate decorators would be needed, just as they might not be needed in every project if you're not going to be modifying just configurable, just enumerable, or just writable very often. Of course, if they would already be included in a standard decorators library, then the example should definitely use them. But the standard library idea is still in a very early stage (#54), so it's too soon to decide based on that.

[mbrowne]

I just realized that a @readonly decorator might not be as simple of an example as I thought, although perhaps there's room for improvement in the proposal itself to make it easier. I thought that the finisher option applied to all decorators but I now see that it only applies to class-level decorators. The use case I was thinking of is having a read-only property that you set when instantiating the object, e.g.: *

class User {
  @readonly id
  ...

  constructor(id = null) {
    this.id = id
  }
}

The problem here is that unless the placement of instance properties has completely changed since the first version of this proposal (and no such change was mentioned in the list of changes for member descriptors), then the id property here would be set to writable: false prior to setting it to the passed value, causing an error:

class User {
  constructor(id = null) {
    Object.defineProperty(this, 'id', {value: undefined, writable: false})
    this.id = id  // error, since property isn't writable
  }
}

Using Babel with the older decorators proposal (babel-plugin-transform-decorators-legacy), I had to write a rather ugly workaround to get a @readonly decorator working.

I suppose this could be solved by additionally having a class-level decorator that would be required in order to use the property-level @read-only decorator, but if the goal is a simple example, then that probably disqualifies @read-only, at least as I envisioned it.

Perhaps I should open a new issue about the new concerns I raised above...or am I misunderstanding something?

---

*In this particular example of a domain model, it would be even nicer to have a generic way of setting initial state as I described here, but that's not relevant to the decorator implementation, probably.

[doodadjs]

@mbrowne Effectively, in my framework, I had to apply them (@configurable, @enumerable and @writable) AFTER the constructor has finished.

I was thinking of storing decorators in the property descriptor, then having something that queries them and apply things like "finalizers", ... They could also be queried by user code.

[mbrowne]

What if the initializer function for property/method decorators were passed the same arguments that were passed into the constructor? That way you wouldn't need any extra steps to handle this case. I agree that using the extras property of the property descriptor to store metadata could still be useful if desired, but it doesn't seem like it should be a requirement for this.

[doodadjs]

@mbrowne I'm not sure to understand...

Assuming :

function readonly(/* All the proper arguments */)) {
    // Do the magic here
}

What I mean is :

class MyClass {
  @readonly id
}

Might be equivalent to :

const MyClass = function() {};
Object.defineProperty(MyClass.prototype, 'id', {decorators: [readonly()]});

[mbrowne]

@doodadjs Have you read this document? https://github.com/tc39/proposal-decorators/blob/master/METAPROGRAMMING.md. Your above code doesn't seem to reflect the current proposal, at least as I understand it.

[mbrowne]

@littledan @ljharb I looked in the spec for where the initializer function is executed and didn't find it...is the spec complete with the features described in the metaprogramming doc, and if so could you point me to where this is specified? Sorry if I'm being thick here, I just only saw specs for how the initializer is defined and passed around, but not executed.

[doodadjs]

@mbrowne I've edited my comment. Is it better ?

[doodadjs]

@mbrowne The problem I see with that approach (a function instead of a class) is on how to identify applied decorators... But that's another issue.

[mbrowne]

@doodadjs I'm still confused by your example; are you trying to show how you would implement it using the current proposal, or proposing some alternative to the current proposal? A decorator in the current proposal would be equivalent to something like the following, assuming the decorator didn't change the default 'instance' placement:

class MyClass {
  constructor() {
    // assuming idDecorator is the decorator, obtained by the JS engine somehow...
    Object.defineProperty(this, 'id', {
      ...idDecorator.descriptor,
      value: idDecorator.initializer ? idDecorator.initializer(): undefined
    })
  }
}

...obviously this is not intended to show what real transpiled code would look like, but just the concept (also it doesn't handle the case of multiple decorators on the id property).

[doodadjs]

@mbrowne Sorry for the confusion.... I suggest to store the decorators within the descriptor, and make them query-able.

[mbrowne]

See #58 for further discussion about how the @readonly decorator I mentioned above might be implemented.

[mbrowne]

Resolved via #59