normalize parameters

[ksmth]

I've been playing around with feathers-hooks and I've been trying to find a way to reuse code. As it's easier to understand with an example, consider the following code:

UsersService.prototype.after = {

    create : function (data, requestData, params, callback) {
        delete data.password;
        callback(null, data, requestData, params);
    },

    find : function (data, params, callback) {
        var filtered = _.each(data, function (user) {
            delete user.password;
        });

        callback(null, filtered, params);
    },

    get : function (data, id, params, callback) {
        delete data.password;
        callback(null, data, id, params);
    },

    update : function (data, id, requestData, params, callback) {
        delete data.password;
        callback(null, data, id, requestData, params);
    },

    patch : function (data, id, requestData, params, callback) {
        delete data.password;
        callback(null, data, id, requestData, params);
    },

    remove : function (data, id, params, callback) {
        delete data.password;
        callback(null, data, id, params);
    }

};

What I would like to do would be something like this:

UsersService.prototype.after = {

    create : deletePassword,

    find : function (data, params, callback) {
        var filtered = _.each(data, function (user) {
            delete user.password;
        });

        callback(null, filtered, params);
    },

    get : deletePassword,
    update : deletePassword,
    patch : deletePassword,
    remove : deletePassword

};

The only way to achieve this is for the deletePassword function to check the number of arguments and act accordingly, which makes the intent less clear in my eyes. Would it be feasible to normalize the parameters to something like the following?

UserService.prototype.after = {
    create : function (obj, callback) {
        console.log(obj);
        // => { data : { ... }, id : ..., requestData : { ... }, params : { ... }  }
        callback(null, obj, callback);
    }
};

// or

UserService.prototype.after = {
    create : function (data, obj, params, callback) {
        console.log(obj);
        // => { id : ..., requestData : { ... } }
        callback(null, data, obj, params, callback);
    }
};

[daffl]

I like

UserService.prototype.after = {
    create : function (obj, callback) {
        console.log(obj);
        // => { data : { ... }, id : ..., requestData : { ... }, params : { ... }  }
        callback(null, obj, callback);
    }
};

I noticed it too, that it is a little annoying having to pass all the arguments through all the time, even if you are not using them. This would also allow to make it Express like middleware where you can modify the object and then just call next().

[ksmth]

So it'd either be:

UserService.prototype.after = {
    create : function (obj, callback) {
        // do stuff
        callback(null, obj);
    }
};

Or possibly more Express like:

UserService.prototype.after = {
    create : function (obj, next) {
        // modify obj directly
        next();
    }
};

I'd prefer the latter, as it'd make it harder to forget or break something.

[daffl]

I think we can do both. If the callback isn't passed any parameters use the original data object otherwise the one returned by the callback.

Like the idea, I think I might get to implementing it today or tomorrow.

[daffl]

I implemented the new API via e140451 (probably should've made it a pull request but oh well). Looking like this now:

Documentation is updated, too and looks like this:

todoService.before({
  find: function (hook, next) {
    if (!hook.params.user) {
      return next(new Error('You are not logged in'));
    }

    next();
  },

  create: function(hook, next) {
    hook.data.createdAt = new Date();

    next();
    // Or
    next(null, hook);
  }
});

Will add some examples and make a 0.2.0 if that looks good.

[daffl]

Closed via e140451