typestack / routing-controllers

Create structured, declarative and beautifully organized class-based controllers with heavy decorators usage in Express / Koa using TypeScript and Routing Controllers Framework.
MIT License
4.42k stars 394 forks source link

Proposal: Type Checked Links #196

Open henkkuli opened 7 years ago

henkkuli commented 7 years ago

Links form the backbone of hypertext documents and nothing is more annoying than dead links, especially those trying to point to other parts of a web application. That's why I propose type checked links.

Type Checked Links

Type checked links allow creating links to individual actions in controllers by their name in a type checked manner. They also allows passing parameters to those actions. This makes possible refactoring paths of the controllers and actions without needing to worry about broken links as links are automatically created by the proposed function. Changing the internal name of an action is also safe as type checked links enforce that the action really exists on the controller and raises a build time error otherwise.

Proposed function

I propose a function with the following signature to be added to the library:

function addressOf<Controller>(controller: new(...args: any[]) => Controller, action: keyof Controller, params: object = {})

The implementation of this function can be as simple as:

  1. Find the path of the action
  2. Substitute params to the path This can be done using path-to-regexp.

Examples

Here is a small example of using the proposed function.

@Controller("/user")
class UserController {
    @Get("/")
    public userList() {
        const users = [ /* ... */ ];
        const result = new Array<string>();

        for (const user of users) {
            const userAddress = addressOf(UserController, "showUser", { id: user.id });
            result.push(`<a href="${htmlspecialchars(userAddress)}>${user.name}</a>"`);
        }

        return result.join("<br />");
    }

    @Get("/:id(\\d+)")
    public showUser() {
        const userListAddress = addressOf(UserController, "index");
        return `<a href="${htmlspecialchars(userListAddress)}>Back to user list</a>"`;
    }
}
pleerock commented 7 years ago

Personally I almost never use html and I don't need this feature, however for people who use it this may be useful function. What do you think guys @19majkel94 @NoNameProvided

MichalLytek commented 7 years ago

We have nearly no features (only render) for server-rendered, classic apps. I don't think that in React&Angular time many people create server-rendered apps, only RestAPI/GraphQL/Websocket matters. We should redirect to the competitors 😆

pleerock commented 7 years ago

Our "competitors" are different they are using angular2, etc. and more like a full stack framework. If me and you don't do server-side rendered apps it does not mean other people dont do it. I know lot of people hard core fans of server side rendered apps (but I dont understand them TBH 😆 ) and they may not need nextjs but may need something like routing-controllers. I don't think we should strip out such people, otherwise we probably must remove "render" functionality and declare genocide to server side rendered apps :no_good_man:

As I told I almost never use html on server side. Those almost cases are for example when you generate a email with html inside, and lets say if you give a link to controller which returns pdf in this email this feature will be useful.

NoNameProvided commented 7 years ago
const userListAddress = addressOf(UserController, "index");

You use index as string so you loose your type the same way. However it can be achieved with the keyof TypeScript operator but then you would not be able to define functions or properties other than your route handlers in your class what is a no-go for me.

Also this maybe looks useful in this small example, but in real life we use rendering engines, and there is no way to pass custom stuff in there most of the cases. And even in the few like Handlebars as soon as you start to use a helper function you will loose your typings again, because templates are strings.

So in this form this don't add value or will work so I have to 👎

pleerock commented 7 years ago

@NoNameProvided I did not understand any of your argument TBH.

you would not be able to define functions or properties other than your route handlers in your class what is a no-go for me.

but what else you would like to define?

there is no way to pass custom stuff in there most of the cases

what do you call custom stuff here and pass where?

@henkkuli if you want this feature you'll need to provide a PR btw.

NoNameProvided commented 7 years ago

you would not be able to define functions or properties other than your route handlers in your class what is a no-go for me.

The whole idea is to bring type check to url generation.

Lets say we have a route handler like this:

@JsonController()
class TestController {
  @Get('/test') 
   public getTest() {
     return 'yay';
  } 
}

To generate an url for this route with the proposed changes we have to do something like

addressOf(TestController, "getTest");

The problem is that getTest is passed as a string so we loose typecheck, and the whole idea of the proposed change is to bring typecheck to generate links.

We can work around this with having a signature like

function addressOf<T>(controller: T, handler: keyof T);

But when we do this we would be not allowed to have anything on our controllers than route-handlers. So with the above example if I try to call

addressOf(TestController, "postTest");

it will fail and tells me TestController doesn't have a property called postTest.

But what about

@JsonController()
class TestController {
   constructor(
     public myService: MyService
  ) { }

  @Get('/test') 
   public getTest() {
     return 'yay';
  } 
}

Now suddenly both getTest, and myService is a key, however getTest is the only valid one. So again we lost the typing information we aimed to achieve.

So I dont see how this would work.

there is no way to pass custom stuff in there most of the cases

I means nobody send response like

return ```<html>
  blabla....
  <a src="${x.addressOf(TestController, "postTest")}">Link</a>
</html>

But we are using template renders like return pug.render(pathToMyTemplate, locals) and it's not trivial how to put the links in there. For sure we can pass them in the locals variable I would say it would be hard to work with it.

MichalLytek commented 7 years ago

As we have static routing, we can build on bootstrap the routing tree associated with the controller's classes and attach it to locals, so in rendering engine we could do routes.TestController.postTest which would be a path string to selected action 😉

marshall007 commented 7 years ago

@NoNameProvided worth noting that keyof type assertions fail for any non-public methods/properties of the object. If, in your example, myService was marked private then addressOf(TestController, "myService") would fail to compile.

pleerock commented 7 years ago

We can work around this with having a signature like function addressOf(controller: T, handler: keyof T);

its not a "work-around", its absolutely correct solution. If there is a solution, there is no problem.

Now suddenly both getTest, and myService is a key, however getTest is the only valid one. So again we lost the typing information we aimed to achieve.

if he sends "myService" as a key its not a problem to check if controller action with such name is registered and throw an exception. Its better then nothing for sure, because point of this not only in complete type-safety but also in usage convenience and refactoring issues.

I means nobody send response like

I know people who have only few places where they need server side rendering and they don't use template engines because its overhead for them. And even if they use template engines, they support creating a custom filters, so using "x.addressOf("TestController", "postTest")" may become absolutely possible. Besides it, this feature is useful not only for templates but also for such generic

Again, Im not a fan of server-side rendering, but I know loot of people who are. And I would like to support server side rendering in routing-controllers as much as possible to satisfy their needs as well.

I don't have a time to implement this feature right now, but if someone from community or even OP can do it - feel free to implement this feature.

github-actions[bot] commented 4 years ago

Stale issue message