sjmeverett / express-decorators

ES2015 decorators for express
45 stars 9 forks source link

add typescript support #4

Closed miguelcobain closed 7 years ago

miguelcobain commented 8 years ago

Looks really cool. :)

addresses #3

import web, { controller, get, use } from 'express-decorators';

@controller('/hello')
export default class TestController {

  @get('/hello')
  list(request, response) {
    response.send('hello, world!');
  }

}
miguelcobain commented 8 years ago

I added the possibility to not provide a path, defaulting it to /. This allows me to write @controller() and @get().

sjmeverett commented 8 years ago

Thanks @miguelcobain, I'll try to look at this later today.

miguelcobain commented 8 years ago

Ok. All tests are passing.

sjmeverett commented 8 years ago

Apologies, I have no idea where the time goes... My day job saps up all my time and energy at the moment.

Not all of this is related to the stated purpose of the PR, namely "add typescript support"; e.g., why did you refactor the register function and export it?

Knowing very little about typescript, I'm just kind of taking your word for it that it is correct (I'm sure it is ;)) - is there something we could add like a unit test or something which demonstrates that the ts file is correct?

If not I guess I can just accept it and see if anyone else submits a bug!

miguelcobain commented 8 years ago

@stewartml no problem at all. I can relate. :)

The problem was that the register function was basically being "injected" in the class at the runtime by the decorator. If you're used to typed languages, this isn't really compatible with the paradigm. Basically, typescript couldn't know that the class has a register method and we get an error when trying to call it.

So, I basically export the register method and I declare it in the typings file. It is a "functional" approach instead of an OOP one. Another option would be to add a superclass that has a register method and then all our controllers would need to inherit it, but I felt that to be more intrusive.

This was an experiment. I ended up not using this in production, but it worked just fine in my tests. Also, the tests pass, so you can be certain it won't break existing functionality.

I'm not aware of any typescript test of that sort. I'm also fairly new at typescript. :) In any case, I believe that this is a step forward for typescript users.

It's your call and your project. No problem. :)

rundef commented 8 years ago

:+1:

sjmeverett commented 7 years ago

@miguelcobain I finally got around to learning TypeScript...

I've rewritten this library in TypeScript now, so the definitions are there. I took your suggestions onboard about separating out the register function as well.