HerringtonDarkholme / av-ts

A modern, type-safe, idiomatic Vue binding library
MIT License
216 stars 11 forks source link

[proposal] Change the workings of @Lifecycle #37

Closed Evertt closed 7 years ago

Evertt commented 7 years ago

Right now you put @Lifecycle in front of a method which (hopefully) has the same name as a lifecycle event.

There's a few issues with this:

  1. You can't use TypeScript's type-checking to check if the method was spelled right and is in fact a known lifecycle method.
  2. You let developers know in the documentation that they shouldn't call lifecycle-methods from other methods, but if they don't read that and they try it then TypeScript again won't complain.
  3. VueJS actually does allow you to give a normal method the same name as a lifecycle-method, as long as you put it into the methods: {} option. And there might be times where the most appropriate name for a method happens to be created. However, this library doesn't allow for that.

All these issues could be resolved by redesigning @Lifecycle to use the following syntax:

@Lifecycle('created')
initializeStuff() {
  // initialize stuff
}
  1. With this syntax you can make typescript do compile-time checking if the provided string 'created' is actually a valid lifecycle.
  2. Typescript allows developers to still call this.initializeStuff() which in this case is perfectly valid, since initializeStuff() will just be added to the methods: {} option.
  3. Now it's perfectly valid to make a normal method called created and treat it as a normal method, because it will also just be added to the methods: {} option.

And I guess it should be possible to use the same lifecycle decorator on two different methods, right?

So what do you guys think?

HerringtonDarkholme commented 7 years ago

You can't use TypeScript's type-checking to check if the method was spelled right and is in fact a known lifecycle method.

Yes, you can. Try decorating a method of which the name isn't lifecycle.

if they don't read that and they try it then TypeScript again won't complain.

They should. And there are other usages requiring users to read the doc.

VueJS actually does allow you to give a normal method the same name as a lifecycle-method

It is not recommended. And if you want you can delegate the lifecycle method to another method.

The proposed solution will unconditionally add one more field to methods, which is undesirable.

HerringtonDarkholme commented 7 years ago

If you want to use class style library with Vue. I definitely recommend the official library. It gets more support. And it is better for the community since we have too many bike sheding.

Evertt commented 7 years ago

Yes, you can. Try decorating a method of which the name isn't lifecycle.

Damn, I spoke too soon.

It is not recommended. And if you want you can delegate the lifecycle method to another method.

True

The proposed solution will unconditionally add one more field to methods, which is undesirable.

Well in that case I guess we could do both, couldn't we? We could offer several overloads of the @Lifecycle method. One where the developer does not supply a string, in which case it will use the method-name to determine which lifecycle-event it stands for. And another overload where the developer does enter a string, which represents the name of the lifecycle-event.

Evertt commented 7 years ago

If you want to use class style library with Vue. I definitely recommend the official library. It gets more support. And it is better for the community since we have too many bike sheding.

I prefer your library because I think the code is cleaner, more extensible and offers things like mixin-support.