4

Can anyone figure out what's wrong with my code below?

From documentation it looks like the this in Mongoose .pre('save') method should be the model itself, but in my code below this ends up being an empty object.

const Mongoose = require('./lib/database').Mongoose;
const Bcrypt = require('bcrypt');

const userSchema = new Mongoose.Schema({
    email: { type: String, required: true, index: { unique: true } },
    password: { type: String, required: true }
});

userSchema.pre('save', (next) => {

    const user = this;

    Bcrypt.genSalt((err, salt) => {

        if (err) {
            return next(err);
        }

        Bcrypt.hash(user.password, salt, (err, encrypted) => {

            if (err) {
                return next(err);
            }

            user.password = encrypted;
            next();
        });
    });
});

const User = Mongoose.model('User', userSchema);

When saving a user, I get the following error [Error: data and salt arguments required].

function createUser(email, password, next) {

    const user = new User({
        email: email,
        password: password
    });

    user.save((err) => {

        if (err) {
            return next(err);
        }

        return next(null, user);
    });
}

createUser('test@email.com', 'testpassword', (err, user) => {

    if (err) {
        console.log(err);
    }
    else {
        console.log(user);
    }

    process.exit();
});

If I remove the .pre('save') then it saves fine of course. Version of Mongoose I'm using is 4.2.6.

jawang35
  • 187
  • 2
  • 11
  • First of all, why do you use `const`? Use `var` instead, then `this` in pre script referers to the record not to the model. – michelem Nov 18 '15 at 08:09
  • I'm using `const` because those variables are never changing. I tried changing to `var` and the code runs the same. – jawang35 Nov 18 '15 at 08:12

3 Answers3

7

Problem here is a fat arrow functions. You must rewrite your callback with simple functions. Here small example to show the diff

var obj = {};

obj.func1 = function () {
    console.log(this === obj);
};

obj.func2 = () => {
    console.log(this === obj);
};

obj.func1(); // true
obj.func1.bind(obj)(); // true

obj.func2(); // false
obj.func2.bind(obj)(); // false
Alexey B.
  • 11,965
  • 2
  • 49
  • 73
  • Thanks. I had just figured this out and was writing up my own answer as you posted this. – jawang35 Nov 18 '15 at 08:38
  • They are just "arrow functions", not "fat arrow functions". – loganfsmyth Nov 18 '15 at 17:11
  • An arrow function expression (also known as fat arrow function) (с) MDN – Alexey B. Nov 19 '15 at 08:32
  • Thank you! A thousand times thank you! Couldn't figure out what was happening as it seemed I followed every instructions to the letter... Being pretty much a js noob, the differences between arrow functions and simple ones hadn't stuck in my head yet. This one will, for sure! Thanks again. – Gormador Jan 11 '17 at 19:31
5

I was able to figure out the issue. It turns out Arrow Functions in ES6 preserve the context of the declaring scope rather than using the context of the calling scope so changing the code to the below fixed the issue.

userSchema.pre('save', function (next) {

    Bcrypt.genSalt((err, salt) => {

        if (err) {
            return next(err);
        }

        Bcrypt.hash(this.password, salt, (err, encrypted) => {

            if (err) {
                return next(err);
            }

            this.password = encrypted;
            next();
        });
    });
});

Thank you Michelem for giving me the idea that ES6 may have been the culprit.

jawang35
  • 187
  • 2
  • 11
  • Also with the Arrow Functions capturing `this` from the declaring scope, there is no longer any need to declare the `user` variable. – jawang35 Nov 18 '15 at 15:53
  • arrow function was causing my issue, thanks for pointing out context/scope tip! – electblake Apr 28 '16 at 16:28
0

I think it should be:

userSchema.pre('save', (next) => {

    var user = this;

    // Try this
    // console.log(user);

    Bcrypt.genSalt(function (err, salt) {

        if (err) {
            return next(err);
        }

        Bcrypt.hash(user.password, salt, null, function (err, encrypted) {

            if (err) {
                return next(err);
            }

            user.password = encrypted;
            next();
        });
    });
});

But please check user is the right object.

michelem
  • 14,430
  • 5
  • 50
  • 66
  • Still not working. Other than changing `const` to `var` and changing syntax back to ES5, how is this code different from mine? – jawang35 Nov 18 '15 at 08:27