-
Notifications
You must be signed in to change notification settings - Fork 255
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Pub/Sub as part of core #100
Comments
One of the concepts we initially wished to maintain was making it flexible enough for any piece of the Aura architecture to be swapped out. This covered both the pub/sub implementation we were using as well as the mediator implementation to an extent. Sindre makes an interesting point that perhaps the Pub/Sub we expose is sufficient and should remain something tied to core, that isn't swappable. I should point out, I believe at the moment it doesn't cost us to make it swappable, but whether we should be documenting this or not is up for discussion. Does anyone else have thoughts on this? |
My first impression is also keep our current, What will be the expectation for community-built extensions like What are the current pub/subs implementations to look at if one wanted to do some HW? I know Backbone.Events, Marionette's EventBinder and EventAggregator. A few other frameworks have them. How are they different? Let's see what the delta is, when doing so we'll learn enough about them to either improve Aura's implementation, create an abstraction layer, etc. |
To an extent, they follow the same pattern, but there are also standalone implementations like AmplifyJS, PubSubJS and a few others implemented on blog posts and the like. The only place I'd really imagine it (maybe) making sense to swap the implementation out is if you want to say, tie into something specific to a framework. Like, maybe a Backbone developer would want to use Backbone.Events for communication instead. The flip-side of this however is that if our own PubSub comes with Aura and you're a) getting it for free and b) it's not going to return what the Backbone.Events object would, maybe a developer would be better off just sticking to using Backbone.Events for anything in the Backbone extension or their Backbone module and the Aura PubSub for communication between widgets/views/anything else. |
The problem I see if the pub/sub implementation is swappable is that someone who would use another implementation could end up being dependent on that particular implementation doing something specific. If we do that, there is a big chance that we will no longer have "...modules that can exist on their own or be dropped into other projects..." (quote from here :)) So yeah, I agree with @sindresorhus that it would be better to just make it great and make people use it. |
^ I'm happy for us to go with that decision. I guess the next question is, what do we think can be improved in our Pub/Sub implementation :) |
^ agreed. Does Aura support namespaced events? Take a look at EventEmitter2. It has some great ideas. |
Goal: Improve our Publish/Subscribe system in core
a) Permissions completely switched off. I'd like a way to allow all communications between all widgets if the permissions layer seems overkill for my app. b) enabled, but having a way to use pub/sub for events within a specific widget. So, lets say that I'm in the calendar widget and I want to be able to publish and subscribe to events within that widget. I'm not going to be touching other modules with this communication but I also dont want to have to enable every event/topic I'm going to publish. It would be nice if there was a way for me to do in-widget Pub/Sub free of permissions. Maybe there's a way we could enable pub/sub in this case but only for the current context? i.e
same with publish.
|
Any thoughts on what (if anything) in the above we should implement? |
I am not sure what would the subscription priority be used for. Any use cases ? Wildcards could be nice, someone wanted to be able to log everything that goes around, subscribing to everything could solve that case. I agree about the permissions, we should have a way to disable them. They are really useful when you are using someone else's widget in your application. @addyosmani whait would be in the We should stop using a simple object ( |
To be honest, I haven't seen many P/S systems implementing priority other than Amplify. I guess a use case is that you have a dense UI composed of lots of components with a high level of messaging and you wish to give priority over certain types of communications more than others. imo, its an edge case. Wildcards would be lovely to have. RE: |
Yes to namespacing and wildcards. I don't see the point of having the namespace delimited configurable though. No to priority. I can't think of any the real-world use-case. Btw. Any reason publish/subscribe is used instead of pub/sub or emit/on? |
I think when we started the project a year ago that the ++ to no on priority and yes on namespacing + wildcards. |
If everyone is in agreement, lets do namespacing, wildcards and the change to emit/on. On wilcards, are the cases we want to support:
? The first is relatively easy to implement as a catch-all. I'm guessing we could use some regex-fu on the second. Any other cases? Hmm. Namespaces. So, I haven't written a Pub/Sub solution before that supports namespacing in topics so I'm wondering whether this is something we implement as a generalised way to include special characters in a topic-name or make it more specific than that. Any opinions on this would be appreciated before we go ahead and implement. |
Me neither, but could just borrow some ideas from EventEmitter2 or even jQuery. |
Renamed Whilst going through, I noticed that (unless I'm missing it because its late) we don't seem to offer a direct |
Simplistic wildcard support based on how EventEmitter2 handles the |
@addyosmani: looking good. Tested in grunt and firebug. I wonder if we could This is kind of free-flowing, aura doesn't spec to this but could allow people to do a: obj.emit('nav.link.click')
obj.emit('nav.button.click')
obj.on('nav.*.click') or obj.emit('nav.*.enable')
obj.on('nav.buttons.enable')
obj.on('nav.search.enable') There is also a |
Hey @addyosmani, with your implementation, calling |
@tony We could! I would love for us to solve namespacing sometime soon and I absolutely love the suggestions you made of how and where wildcards could fit in. minimatch (just looking at it for the first time) seems like a fairly heavy implementation for client, but there may well be some nice regex in there we could use. Would you be interested in taking a look at implementing a POC of namespaced topics? :) @rumpl Good catch! I'd thought about this use case when I was implementing but figured we'd solve it later, however I do very much want to support both existing and future. If there are no further comments on the emit/on changes, I can land those so we can continue iterating on the rest of this ticket without rebasing on branch. Did anyone else have comments on |
Hello all, You can find here (it's on the ns-wildcard-pubsub branch) a working POC of what we are talking here. I don't have the time to do the So, we have:
You can do something like this : sandbox.subscribe('test.*', 'todos', function() {
console.log('YAY');
});
sandbox.publish('test.itworks', 'todos');
sandbox.publish('test.itworksagain', 'todos');
sandbox.publish('test.dont.yay', 'todos'); And you will see two Just realized I used the master and not your branch with on/emit... Sorry about that. |
@rumpl That is so awesome. Nice work! I'm sure we can iterate on it a little further but I'm really stoked to see what you have in place now working. I'll play around with it further. Re: master vs branch - no worries!. I can rebase my branch before landing. I'm more excited to see the above land :) |
@rumpl Clutch, man 8-) I'll play w this branch more too. |
Hey all, thanks for the heads up. Apreciate it. Indeed, it is a very good examaple of the kind of functionality that needs thorough testing. That is way I didn't do a PR. I wanted to see if everybody was OK with what I was doing. It is just a POC for now, meaning the code is probably bad and could be enhanced so any help is welcome :) Sent from mobile - please excuse tone and brevity On 10 sept. 2012, at 20:00, Tony Narlock notifications@github.com wrote:
|
Pertinent reading on namespaced / wildcard pubsub: There is a codereview of EventEmitter2 at http://dailyjs.com/2011/09/19/code-review/. EventEmitter2 is based on a conversation about namespaced events. |
Today I merged in a few changes we've been discussing lately including very basic wildcard support, emit/on renaming etc. I still need to go through the work by @rumpl and bring it in as a PR or something that's had a code review done to it. @rumpl would love for you to get the credit for the work. Would you mind doing a PR so that we can leave inline code comments and iterate on? :) |
Sure, I just have to update it to the new emit/on function names. Thanks. |
@rumpl think you might have a chance to look at this again? :) |
Should there be a mechanism for the emitter of an event to optionally consolidate feedback from consumers? PubSub is a great mechanism for agnostic collaboration among modules, but the Service Locator pattern (as we see in Eclipse, Android, etc.) goes a further step by allowing information collection, in addition to broadcasting. Use case: A navigation module that queries the service locator for other modules that would like to contribute navigation items. It's awkward doing this with pure PubSub eventing (or I'm missing the ideal solution). One can always implement a feedback consolidation mechanism over PubSub (for example by making sure the event is always consumed synchronously, and then attaching some kind of callback to the event payload, and praying for consumers to call your callback with their data). But I just feel that it would be nice to see a generic mechanism baked into this framework... |
@atesgoral I'm a little silly in that I think better in terms of code. Do you think you might have time to throw together a proof of concept / PR that introduces what you're describing so that we can play around with it and discuss how this might work its way into the current PubSub system? |
@addyosmani I could surely give it a shot! Actually, @gorillatron's sandbox.requestInterface idea comes close to what I was mentioning: sandbox.registerInterface("data::users", users);
// then...
sandbox.requestInterface("data::users", callback); I'm merely taking it a step forward to allow for multiple results (Array) to be returned. |
@addyosmani I did have a comment on It seems pretty straight forward to tease this capability out of |
@solenoid Thanks for your interest. Could you clarify what you meant by wash out to? I would place the |
By 'wash out to', I meant how it would integrate into the sandbox specifically. I like many aspects of the pub / sub channel being wrapped over there but have found myself transposing arguments around permissions often so I'm less comfortable in the sandbox. Also I wanted to let you know that using aura at my place of work is going great and in general has helped conceptually and with the quality of code at the same time. |
I feel like we've addressed this (or rather, are addressing this) as a part of the aura-express work landed in master. Closing for now. |
There has been some questions about how to swap out the existing pub/sub implementation:
What is the benefit of doing something like that?
IMHO, since pub/sub is such an integral part of Aura, wouldn't it be better to focus on making it really good instead of abstracting it away?
The text was updated successfully, but these errors were encountered: