hagopj13 / node-express-boilerplate

A boilerplate for building production-ready RESTful APIs using Node.js, Express, and Mongoose
MIT License
7k stars 2.06k forks source link

Typescript conversion #153

Open CapsAdmin opened 3 years ago

CapsAdmin commented 3 years ago

This converts the entire repo to typescript. The first few commits try to keep the spirit of the project but later on it gets more opinionated to suit Typescript.

It's far from perfect. It has many type errors and a relaxed tsconfig. Tests should run fine though.

Some major changes are:

On a mid to high level I don't think this project has a good architecture for Typescript. To get the most out of Typescript, the code need to be structured in a more one-pass way rather than partially defined functions. I think this comes from the middleware pattern which makes more sense in javascript.

The middleware pattern is supported by typescript, but I personally don't think it's very typesafe.

For example, router.post('/register', validate(authValidation.register), authController.register); has the validation disconnected from the controller's register function from typescripts point of view.

One solution to this is to globally extend the Request type in express, but this makes the type available everywhere in all Request's.

Another more optimal way (IMO) is to avoid using middlewares and instead "plug in" the middleware so it becomes part of the control flow instead.

ie something like

export const registerSchema = z.object({
  body: z.object({
    email: z.string(),
    password: z.string(),
    name: z.string(),
  }),
});

router.post("user/register", async (req, res) => {
  const { email, password, name } = registerSchema.parse(req).body;

  const user = await userService.createUser({email, password, name});
  const tokens = await tokenService.generateAuthTokens(user);

  res.status(httpStatus.CREATED).send({ user, tokens });
});

The same goes for the mongoose plugins. There is no way for typescript to know that a schema will have a plugin unless you explicitly define the type after you've also called the plugin function.

So overall there will be a lot of double and tripple typing and telling typescript that some particular code has a certain type and some other code has the same type in order to make it typesafe while keeping the spirit of the project.

I tend to lean towards making things as typesafe as possible and make as few types as possible relying on inference. But I also recognize that for some this is not important or desirable.

markvital commented 2 years ago

Is there any work on this? @hagopj13 Are there any plans for TypeScript support?

daveydee33 commented 2 years ago

I really like this. I would really like to use a Typescript version of this great boilerplate project :)

CapsAdmin commented 2 years ago

I believe this is ready for a review. I haven't tested it that much apart from the test suite and there are some unresolved type errors left. Would be nice if someone else wants to do the rest.

One fundemental change I've done is to change auth and validate to be like this:

router
  .route('/:userId')
  .get(userController.getUser)
...
export const getUser = catchAsync(async (req, res) => {
  await auth(req, 'getUsers');
  const { params } = userValidation.getUser.parse(req);
  const user = await userService.getUserById(params.userId);
  if (!user) {
    throw new ApiError(httpStatus.NOT_FOUND, 'User not found');
  }
  res.send(user);
});

Rather than auth and validate being middleware. This allows us to more easily get the type of the request into our controller function.

auth also returns the user if sucesful, otherwise throws an UNAUTHORIZED ApiError.

I also think the pagination and toJSON mongoose plugins should be classes that are implemented or extended. Or maybe they can just be their own functions. (so paginate(user, ...) as opposed to user.paginate(...))

ghost commented 2 years ago

It would be nice if one could fix the hundreds of lint error. I've went through lots of them and it actually showed bugs here and there. But i dont know what to do to fix the type warning with "id" being "any" and used as a string

CapsAdmin commented 2 years ago

I don't have much motivation left to continue this, but overall I personally think it's best if the original author rethinks how this should be done with typescript from the ground up. I feel the overall structure resembles more of how you'd setup a Javascript project.

This might be fine for some Javascript developers wanting to try Typescript though.

ghost commented 2 years ago

I ended up using https://github.com/ljlm0402/typescript-express-starter