Implement .hooks service method

[daffl]

This pull request adds a .hooks service method that supports any kind of hook (without having to add each as a service method itself). service.before and service.after are kept for backwards compatibility. They can be used and chained like all hooks already used. Basically any service.before(anything); becomes service.hooks({ before: anything }). Some examples:

const todos = app.service('todos');

// before and after multiple hooks
todos.hooks({
  before: {
    all: [ beforeAllHooks ],
    create: [ beforeCreate ]
    // etc.
  },

  after: {
    all: [],
    create: []
    // etc.
  },

  error: { // new on error hooks
    all: [],
    create: []
    // etc.
  }
});

// before create single hook
todos.hooks({
  before: {
    create
  }
});

// before all hook
todos.hooks({
  before(hook) {

  }
});

[daffl]

As per discussion with @eddyystop, @marshallswain and @ekryski today.

[ekryski]

That sure cleaned up things nicely!

[ekryski]

Bump! @daffl you shipping this guy?

[eddyystop]

Was there discussion about removing or deprecating service.before({}) and service.after({})? Only they seem to dynamically add hooks. The hooks below with console # 2 are not run.

There does not seem to be a way for the performance tool to add an error hook dynamically.
Should we allow hooks to be added the way the # 2 are coded?

const feathers = require('feathers');
const hooks = require('feathers-hooks');

const app = feathers() .configure(hooks()) .configure(services);

function services() {
  this.use('/messages', {
    create(data, params) {
      console.log('create. throw', data.throw);
      return data.throw ? Promise.reject(new Error('x')) : Promise.resolve();
    },
  });

  const messages = this.service('messages');

  messages.hooks({
    before: {
      all: () => { console.log('#1 before all'); },
      create: () => { console.log('#1 before create'); },
    },
    after: {
      all: () => { console.log('#1 after all'); },
      create: () => { console.log('#1 after create'); },
    },
    error: {
      all: () => { console.log('#1 error all'); },
    }
  });

  messages.before({
    all: () => { console.log('#1a before all'); },
  });
}

const messages = app.service('messages');

messages.after({
  before: {
    all: () => { console.log('#2 before all'); },
    create: () => { console.log('#2 before create'); },
  },
  after: {
    all: () => { console.log('#2 after all'); },
    create: () => { console.log('#2 after create'); },
  },
  error: {
    all: () => { console.log('#2 error all'); },
  }
});

messages.before({
  all: () => { console.log('#3 before all'); },
  create: () => { console.log('#3 before create'); },
});
messages.after({
  all: () => { console.log('#3 after all'); },
  create: () => { console.log('#3 after create'); },
});

messages.create({ throw: false });

setTimeout(() => {
  console.log('=====');
  messages.create({ throw: true });
}, 100);

[ekryski]

@eddyystop We were thinking of removing service.before and service.after in favour of just using service.hooks, which should still allow you to dynamically add hooks. However, @daffl and I had noticed that it might be a pretty big breaking change because there are some things that are currently relying on checking for service.before and service.after methods in order to run things conditional.

[ekryski]

Your example 1 is how we were thinking it should be. However, looking at the output I would have expected:

#1 before all
#1a before all
#1 before create

[eddyystop]

I was expecting multiple service.hook() to concatenate but they don't seem
to. I would use them instead of service.before() if they did.

I added the service.before() only to test what was going on.

On Sat, Oct 22, 2016 at 12:14 PM, Eric Kryski notifications@github.com
wrote:

In your example #1 #1
is how we were thinking it should be. However, looking at the output I
would have expected:

#1 before all
#1a before all
#1 before create

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#110 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABezn-2knLEPYdw6gBHdqeddwF8H0idPks5q2jZ1gaJpZM4KWthf
.

[daffl]

They should and I'll make sure that they concatenate today. They shouldn't work differently at least because service.before(beforeHooks) and after are now just an alias for service.hooks({ before: beforeHooks }) and service.hooks({ after: afterHooks }).

[daffl]

@eddyystop I created a test in #117 that verifies that hooks get chained the same way they used to and it seems to work as expected (at least v1.5 with just .before and .after should behave the same).