Add rubberduck args to service events parameters#148
Add rubberduck args to service events parameters#148daffl merged 6 commits intofeathersjs:masterfrom
Conversation
|
Nice! This is a new features but it doesn't look like it is breaks compatibility. The first parameter is still the data so it shouldn't break any existing listeners. I'm wondering if the second parameters should look the same as a feathers-hooks after hook object though. |
|
For that to happen we might have to move https://github.com/feathersjs/feathers-hooks/blob/master/lib/utils.js#L42 into feathers-commons so that it can be shared. |
|
That would be great to make the parameter consistent with the hook one, I didn’t know about that codebase already existing. Moving it to |
|
I just made a 0.2.8 release of feathers-commons which should support getting the hookObject. You should be able to do: var hookObject = require('feathers-commons').hooks.hookObject;
var hook = hookObject(method, 'after', args);Can you try it and update the PR if it works? |
|
It works like a charm! Thanks |
|
Perfect. Are you going to use those locally or via websockets? I think it's fine on the server but I'm wondering if also sending the hookObject through websockets might pose a security risk (e.g. you might not want to send things like |
|
Right now, i’m not using websocket at all in my projects using feathers, but that will come. I will filter out sensitive data. But right now, my use case is mainly to attach contextual data like ip, userAgent, referrer, etc, and then use feather services event dispatcher to fetch those events and to several stuff with them (like storing them in DB, calling webhook, outputing to console, etc.) where contextual data is important. |
|
I didn't forget about this! I think the PR can be merged but I should probably update Side-note about scaling: If you want to use feathers-sync to scale your application to multiple instances and still make sure that all connected clients get all real-time events you might run into issues when handling events on the server. For example, if you scale your app to three instances (e.g. three Heroku dynos) and you are listening to an event which sends out an email, all three instances will send out that email. This is when hooks are the better choice since a hook will only run on the instance that handles the request. |
|
It looks like this is handled already in feathers-commons which only dispatches the data to sockets. I think we can get this and #151 in for a 1.2 |
Add rubberduck args to service events parameters
Update the event mixins to pass along the ruberduck args parameter.
It allows access to contextual data (such as data attached to
req.feathers) in event listeners, and makes it consistent with hooks.I know about backward incompatibility, so maybe for the next major release.