Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Is there a reason why Feathers sends all events to all of its clients by default? #421

Closed
sunabozu opened this issue Oct 6, 2016 · 5 comments
Labels
Milestone

Comments

@sunabozu
Copy link

sunabozu commented Oct 6, 2016

I'm trying to understand why Feathers can't automatically ignore the events user didn't subscribe to. From what I understand everything is pretty straightforward.

When we subscribe on an event, the client automatically emits a socketio event, something like this: feathers.io.emit('subscribe', {service, event}). On the server side we catch it and save like this: socket.feathers.subscriptions.push({service, event}). After that we have all the information we need to filter all the events implicitly, sending to the client only relevant data. You just need to add that filter by default to every service.

Is there a reason why you don't do this? Are you planning to implement such a mechanism?

@sunabozu sunabozu changed the title Is there a reason why Feathers sends all events to all of its client by default? Is there a reason why Feathers sends all events to all of its clients by default? Oct 6, 2016
@daffl
Copy link
Member

daffl commented Oct 6, 2016

This is indeed a very good question. We have some updates for the filtering mechanism lined up (see #388 (comment)) but I like what you are suggesting and unlike #388 I think we might be able to get this into a non-breaking release.

With a subscribe event we could also return some meta information about a service (e.g. if it exists at all and what methods it supports) so Socket clients would finally get a decent error message instead of just a timeout in those cases. If you'd check the subscription in a filter it would also be backwards compatible (just don't register the filter).

Do you think it should be on a per event basis though? I'd probably make it per-service because it would be less to keep track of and in a filter you can just check connection.isSubscribed instead of something like connection.isSubscribed[eventName].

@ekryski ekryski mentioned this issue Oct 17, 2016
18 tasks
@ekryski ekryski modified the milestone: Buzzard Oct 17, 2016
@sunabozu
Copy link
Author

@daffl I'm not sure about that. It's always a tradeoff. In my opinion we can go even further and make those subscriptions more granular. In real life in most cases we want to get updates related to a particular document. For example, when the user opens a chat room, he wants to get the updates made in that particular room. So in addition to the service and event fields we could also send a key-value field (something like feathers.io.emit('subscribe', { service: 'messages', event: 'updated', condition: { _id: 'id-of-the-room' } })). So while filtering the events, we could look at a particular field of the updated/created/deleted document and check its value, and based on that decide wether we should send a notification.

It would really make life easier for developers. It's a bit more complicated to implement, but the concept is the same. And all the three levels of granularity may be optional.

@paulophp
Copy link

Hi there mates!

I just created something about it on my app, I created my own view controller side with angular 1.x and angular routes, when I get the event I only update user_id screen, or if you are admin or it is an dashboard route, you know what I mean?
feather already does so much! :)

@daffl
Copy link
Member

daffl commented Nov 14, 2017

This can now be done pretty nicely with channels:

// server app.js
const feathers = require('@feathersjs/feathers');
const socketio = require('@feathersjs/socketio');

const app = feathers();

app.configure(socketio(io => {
  io.on('connection', socket => {
    socket.on('subscribe', data => {
      const connection = socket.feathers;
      const { event, path } = data;
      const channelName = `${service} ${event}`;

      app.channel(channelName).join(connection);

      app.service(path).publish(event, data => app.channel(channelName));
    });
  }); 
}));

On the client:

const io = require('socket.io-client');
const feathers = require('@feathersjs/feathers');
const socketio = require('@feathersjs/socketio-client');

const socket = io();
const app = feathers();

app.configure(socketio(socket));

app.mixins.push((service, path) => {
  service.mixin({
    on(... args) {
      const event = args[0];

      // If it is a socket client service
      if(service.connection && typeof service.connection.emit === 'function') {
        service.connection.emit('subscribe', {
          path, event
        });
      }

      // Call the old `.on`
      return this._super(... args);
    }
  })
});

Closing this issue but would be happy to help if someone would like to turn this into a plugin 😄

@daffl daffl closed this as completed Nov 14, 2017
@lock
Copy link

lock bot commented Feb 7, 2019

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue with a link to this issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Feb 7, 2019
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants