Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v6 to v7 router migration #2368

Closed
MatejSkrbis opened this issue Jul 4, 2023 · 13 comments
Closed

v6 to v7 router migration #2368

MatejSkrbis opened this issue Jul 4, 2023 · 13 comments
Assignees
Labels

Comments

@MatejSkrbis
Copy link

Information

  • Version: 7.30.3

I am trying to migrate from TsED v6 to v7. I added one route /schemas on Server configuration and included that as a module in package.json file. I used this.app.rawRouter.get('/schemas', this.myMethod) to do it. After migration I tried to use injected PlatformRouter to do the same, but the endpoint no longer registers.

There is no error, it just doesn't work. If the logic is moved into the controller constructor, then it works and new endpoint is registered successfully. The problem is I am using this as a module for many different projects, and would like to register this endpoint before any other controllers. I am not sure if I am doing something wrong.

Also I noticed that $beforeRoutesInit in server is executed after the controller is already initialized. Is this expected behaviour?

Example.zip

@Romakita
Copy link
Collaborator

Hello @MatejSkrbis
I encourage you to provide a reproducible project example (as is explained in the issue guideline ;))

Also I noticed that $beforeRoutesInit in server is executed after the controller is already initialized. Is this expected behaviour?

yes it was always the case in v6 and it's the same thing in v7. Controller is a provider like a Service is it. So if you read the code about the Server lifecycle here (https://tsed.io/docs/hooks.html#introduction), all providers are created during the injector.load() step.

There is a special steps when the routers/controllers are attached to the Express/Koa app (mount all controllers).

But the different in v7 is the router itself. Instead of provide an Express/Koa router as injectable service, Ts.ED provide it's own Router abstraction. This router as the same methods provided by an ExpressRouter. The router store all method/handler created manually and then there information are added to the app.

This part works as expected, but maybe you have an usage that is different and not covered by my integration test. I don't use often this approach (excepted for Ts.ED Module, but isn't a controller).

See you

@MatejSkrbis
Copy link
Author

Hello @MatejSkrbis
I encourage you to provide a reproducible project example (as is explained in the issue guideline ;))

I am not sure what is wrong with the example I provided. In Server.ts there is a line of code (line 51)

this.router.get('/schemas', async(req: any, res: any) => {
      return 'This is my schema';
});

This does not registers /schemas path, while in v6 I was able to use this.app.rawRouter.get('/schemas', this.myMethod) to do it. Is there any other way to register it outside of controller?

@Romakita
Copy link
Collaborator

Please create a repository example!

@MatejSkrbis
Copy link
Author

I've added the example to the repository:
https://github.com/MatejSkrbis/tsed-v6-v7/blob/main/src/Server.ts#L51

Can you also please update the instructions it must be in a repository instead of copy/pasted code? Thanks.
image

@Romakita
Copy link
Collaborator

Romakita commented Jul 13, 2023

Ho sorry I haven't seen the Zip. My bad :D

@Romakita
Copy link
Collaborator

Romakita commented Jul 13, 2023

Can you also please update the instructions it must be in a repository instead of copy/pasted code? Thanks.

But in your case, isn't enough ;)

@Romakita
Copy link
Collaborator

Hoo ok, it's clear now!
PlatformRouter musn't be injected on Server level. Not sure why you doing that, because on v6 it was already not described as being possible to do this at the Server level. PlatformRouter is an instance type provider and is necessarily attached to a controller.

Just use the this.app.get('/schema') if you add routes on server level.

@MatejSkrbis
Copy link
Author

Just use the this.app.get('/schema') if you add routes on server level.

Thank you, but it seems the problem is the same as with this.router.get('/schema')

@Romakita
Copy link
Collaborator

Romakita commented Jul 13, 2023

import {join} from "path";
import {Configuration, Inject} from "@tsed/di";
import {PlatformApplication, PlatformRouter} from "@tsed/common";
import "@tsed/platform-express"; // /!\ keep this import
import "@tsed/ajv";
import {config} from "./config/index";
import * as rest from "./controllers/rest/index";

@Configuration({
  ...config,
  acceptMimes: ["application/json"],
  httpPort: process.env.PORT || 8083,
  httpsPort: false, // CHANGE
  disableComponentsScan: true,
  mount: {
    "/rest": [
      ...Object.values(rest)
    ]
  },
  middlewares: [
    "cors",
    "cookie-parser",
    "compression",
    "method-override",
    "json-parser",
    { use: "urlencoded-parser", options: { extended: true }}
  ],
  views: {
    root: join(process.cwd(), "../views"),
    extensions: {
      ejs: "ejs"
    }
  },
  exclude: [
    "**/*.spec.ts"
  ]
})
export class Server {
  @Inject()
  protected app: PlatformApplication;

  @Configuration()
  protected settings: Configuration;

  public $beforeRoutesInit(): void {
    console.log('Before routes init');

    
    this.app.use("/schema", async(req: any, res: any) => {
      return res.send('This is my schema');
    });
  }
}

works for me :)

Note: You can find more details on PlatformRouter as here: https://www.npmjs.com/package/@tsed/platform-router

@MatejSkrbis
Copy link
Author

Thank you!

@github-actions
Copy link

🎉 Are you happy?

If you appreciated the support, know that it is free and is carried out on personal time ;)

A support, even a little bit makes a difference for me and continues to bring you answers!

github opencollective

@Romakita
Copy link
Collaborator

You're welcome :)

@Romakita
Copy link
Collaborator

@MatejSkrbis Don't forget to add a star on Github project if you have time ;)

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

No branches or pull requests

2 participants