I found that there are 2 different ways to write node functions using promise or callback, the first way is like following defining the findByEmail function:

class Users{
  static async findByEmail(email: any ) : Promise<Users | undefined>{
    const user: any = await Pools.execute(
      "SELECT * FROM users WHERE email = ?",
      [email])
      .then(rows => { 
        return rows[0];
       })
      .catch(err => console.log(err) );
      return user;
  };
}

router.post(
  "/api/users/signin",
  async (req: Request, res: Response , next: NextFunction) => {
     const { email, password } = req.body;
     const existingUser = await Users.findByEmail(email);
});

And the second way would be like:

declare global {
  namespace Express {
    interface Response {
      user?: Users;
    }
  }
}

  static async findByEmail(req: Request, res: Response) {
    const user = await Pools.execute(
      "SELECT * FROM users WHERE email = ?",
      [req.body.email])
      .then(rows => { 
         res.user = rows[0];
       })
      .catch(err => console.log(err) );
  };




router.post(
  "/api/users/signin",
  async (req: Request, res: Response , next: NextFunction) => {
    await Users.findByEmail(req, res);
    const existingUser = res.user;
});

I am not sure if this is a "opinion based" question or not? However my purpose of asking this is to know which way is a better practice and why? According to performance and other possible issues?

In particular I like to know either it is better to write functions with the return value or using response object to add the returning value to that inside the then() function, like .then(res.user = user) instead of const user = await pool.execute(SELECT ...) ?

CodePudding user response：

Here's a way to impalement that makes the following improvements:

1. Makes findByEmail() into a utility function that is independent of the req and res objects and thus can be used generally.
2. Properly propagates all errors from findByEmail() back to the caller.
3. Implements some validation checks on incoming email field and makes separate error path for that.
4. Log all errors on the server
5. Check for all error conditions from the database request
6. Not mixing .then() and await.

Here's the code:

// resolves to null if email not found
// rejects if there's a database error
static async findByEmail(email) {
    const rows = await Pools.execute("SELECT * FROM users WHERE email = ?", [email]);
    if (!rows || !rows.length || !rows[0]) {
        return null;
    }
    return rows[0];
};

router.post("/api/users/signin", async (req: Request, res: Response, next: NextFunction) => {
    try {
        // validate incoming parameters
        if (!req.body.email) {
            let errMsg = "No email value present in incoming signin request";
            console.log(errMsg);
            res.status(400).send(errMsg);
            return;
        }
        let user = await Users.findByEmail(req.body.email);
        if (!user) {
            // do whatever you would do if user tries to signin with non-existent email
            // presumably return something like a 404 status
        } else {
            // do whatever you wanted to do here with the user object after login
        }
    } catch(e) {
        // some sort of server error here, probably a database error, not the client's fault
        console.log(e);
        res.sendStatus(500);
    }
});