feathers icon indicating copy to clipboard operation
feathers copied to clipboard

Feature Request: Allow multiple publishers

Open panstromek opened this issue 5 years ago • 3 comments

At the moment, it's only possible to register one publisher per service+event

  app.service('messages').publish(() => { /* doesn't run */ }
  app.service('messages').publish(() => { /* overrides the previous publisher */ }

This is contrary to how registering hooks works - multiple calls to service.hooks() will register all of them, so I automatically assumed the same behaviour from publishers.

This has bitten me pretty bad today during test with users so I though I might as well post an issue :wink: I had some complicated logic for registering publishers so it surfaced only in some advanced usage scenario. It's mentioned in the docs, but I didn't notice it.

panstromek avatar Feb 12 '20 12:02 panstromek

To be honest I'd rather have it the other way around so you won't be able to register hooks multiple times in the future.

It's also difficult to formalize what should actually happen with multiple publishers. Will everything be merged together? What's the precedency? Errors? Etc. You can already return an array of channels if there is additional logic.

daffl avatar Feb 12 '20 17:02 daffl

To be honest I'd rather have it the other way around so you won't be able to register hooks multiple times in the future.

Yea, I understand, that makes sense. Hooks have definitely more stuff that can go wrong when it comes to registration order. The catch here was just the unexpected inconsistency between the two, but it was simpe to fix. I'd even prefer to throw error if you try to register publisher twice, that would make it harder to get things wrong.

It's also difficult to formalize what should actually happen with multiple publishers. Will everything be merged together? What's the precedency? Errors?

I'd expect just merge - basically the same as if you return an array from publisher, but with the benefit that you can register it at multiple places (separating some concerns). Order doesn't matter AFAIK?

Anyway, feel free to close this, I don't have strong opinions either way 😉 At least it's there if someone hit the same problem as me. Maybe some fat warning in the docs would help, too 😉

panstromek avatar Feb 12 '20 18:02 panstromek

All good, this isn't a bad suggestion, it would just be a breaking change since people might now rely on it being replaced.

daffl avatar Feb 13 '20 18:02 daffl