0

I've taken a look at Brad Traversy's User routes for DevConnector, a project he uses to teach Node.js. The code does not look very clean or self-explanatory enough in my opinion; take a look at the /register route for example - it's all written in one big block. I am wondering if wrapping promises within other promises can solve this issue.

Below is what my alternative looks like:

router.post('/register', (req, res) => {
    const firstName = req.body.firstName;
    const lastName = req.body.lastName;
    const email = req.body.email;
    const password = req.body.password;
    const dateOfBirth = req.body.dateOfBirth;

    buildUserIfNotExists(email, firstName, lastName, dateOfBirth)
        .then(user => hashUserPassword(user, password, 10))
        .then(user => saveUser(user))
        .then(user => {
            sendActivationLink(user);
            res.json(user);
        })
        .catch(errors => {
            if (errors.internalError) {
                console.log(errors.internalError);

                res.status(500).json({
                    internalError: 'An internal error occured.'
                })
            } else {
                res.status(400).json(errors);
            }
        });
});

An example of promise wrapper as I see it would be:

function saveUser(user) {
    const errors = {};

    return new Promise((resolve, reject) => {
        user
            .save()
            .then(user => resolve(user))
            .catch(err => {
                errors.internalError = err;

                reject(errors);
            })
    });
}

So far I've got no issues with this approach, everything works as expected. Are there any downsides to this that I am missing? Is there any way to simplify this even further?

vladek
  • 577
  • 1
  • 4
  • 17
  • 1
    Possible duplicate of [Is wrapping a promise in a promise an anti-pattern?](https://stackoverflow.com/questions/34681315/is-wrapping-a-promise-in-a-promise-an-anti-pattern) – MTCoster Aug 24 '19 at 13:58
  • 1
    Though the title of this matches the title of the [above proposed dup](https://stackoverflow.com/questions/34681315/is-wrapping-a-promise-in-a-promise-an-anti-pattern), the content is not at all a duplicate. That other questions is about a particular case of retrying and is not the same as this. There probably is a dup for this, but that above link is not it. – jfriend00 Aug 24 '19 at 20:19

1 Answers1

0

I am not very experienced with JavaScript but I found the following simplification; instead of:

.then(user => saveUser(user))

I can simply do:

.then(user => user.save())

Actually, after a few modifications, my code looks like this:

router.post('/register', (req, res) => {
    const newUser = new User({
        name: req.body.name,
        hometown: req.body.hometown,
        dateOfBirth: req.body.dateOfBirth,
        email: req.body.email,
        activationHash: nanoid()
    });

    ensureUserNotExists(newUser)
        .then(() => hashUserPassword(newUser, req.body.password))
        .then(() => newUser.save())
        .then(() => {
            sendActivationLink(newUser).then(() => res.json(newUser))
        })
        .catch(errors => {
            if (errors.internalError) {
                console.log(errors.internalError);

                res.status(500).json({
                    internalError: 'An internal error occured.'
                })
            } else {
                res.status(400).json(errors);
            }
        });
});
vladek
  • 577
  • 1
  • 4
  • 17