-
Notifications
You must be signed in to change notification settings - Fork 27.5k
Provide $scope.$onRootScope method #4574
Comments
I would rather provide a custom eventBus service that does this for us than
|
@petebacondarwin I'm not really sure. Then you would have events in two different places and you would always have to decide whether you want it to be this way or that way. If you later on decide that an event should bubble down the scope hierarchy, you would have to move it from the eventBus to the The only benefit of the bubbling currently is that you don't have to care about cleaning up the handler as that's automatically done for you when your local So when you say you would rather have a seperate |
You can do the decorator more easily: $provide.decorator('$rootScope', ['$delegate', function($rootScope) {
$rootScope.prototype.$onRootScope = function(eventName, callback) {
var unbind = $rootScope.$on(eventName, callback);
this.$on('$destroy', unbind);
});
}); The scopes will inherit the $onRootScope method, and now any scope can do I guess this would enhance performance in some situations ... but doesn't seem like a new method in core is the right way to do it. |
@ajoslin oh man, I didn't see the forest for the trees :) Great to see it's possible with the decorator. However, I really think this should be addressed in core. I think this should be the default for application wide events, no? Can you come up with any scenario where it's actually beneficial that |
I've found in my experience that you should only be using As for Modifying the core of |
@wesleycho I heard this claim before but I'm not ready to give up on event based architecture yet. We are in the process of building a SDK for www.couchcommerce.org. The SDK enables you to write web apps that run on our CouchCommerce platform. All of our core functionality is accessible by client side services that don't have any dependencies on Angular. We have an angular specific build though that wraps those services into modules and Let's take something like a Currently we raise events directly on the services their selves (our framework brings it's very lightweight own pub/sub mechanism) but we further plan to use a dedicated eventBus to raise events on. Since our whole library is built around the idea of dependency injection, we can simple inject the eventBus in all services which need to raise events. Thinking this a step further we could replace that eventBus with angular's This might be a very special use case but at least I hope i was able to provide some insights on why I'm not ready to give up on events as the main communication channel :) |
@ajoslin I had to roll back your edit in that SO post as |
@ajoslin fixed it so that it now works together with isolated scopes, too. It has to be:
|
^ FYI, this tactic will not get your issue more attention (and if it did, soon everyone would be using this "trick" and it would quickly become useless). We do our best to look at each issue already. Please don't do this in the future. |
aye aye, sir! What do you think about the issue itself? Considering how many CPU cycles are wasted by people using |
I'd be curious to see a benchmark of the difference. I think a separate service might be better, but I'm not convinced that this is that much cheaper than broadcasting on |
This very much depends on how many scopes you have on your page. I try to come up with a jsperf over the next days :) |
Hey @btford I put together a simple jsperf that compares the perf differences between Here is the link to the perf |
Just do add another thought here. I know that event handling in Angular is somewhat designed around the idea of browser events. Just I wonder if global events like You can fiddle with the I have a hard time finding use cases for |
Currently, every $scope event 'bubbles' upwards or downwards unless stopPropagation is called... I was thinking that it would be useful to be able to ask the event not to bubble (via a parameter or a separate function call). Since the code for $emit and $broadcast is very similar, we could also shrink the code a bit by having a single function performing most of the work, and using closures to customize the behaviour slightly for each. Using this strategy (similar to jQuery's event dispatch mechanism), it could be simple to prevent an event from bubbling, and also simplify maintenance in the future. This is kind of off topic for this issue, but I think it's probably a good idea, even if it introduces some minor breaking changes |
I think the upward bubbling is fine and maybe even the downward bubbling has it's use cases (even if I don't see them) but my main point is that application wide events should not bubble downwards through the entire scope tree. |
But who knows what is intended to be an "application-wide" event except for the thing which emits it? Anyways, I think $broadcast and $emit both return an event object, so stopPropagation() could be called at that point --- I just think it would be nicer/simpler to be able to prevent propagation during dispatch itself. edit I guess $broadcasted events are not cancellable, who knew. |
Well things like Regarding |
The $scope event hierarchy is separate from DOM events. Have any particular examples of third party libraries using $rootScope.$broadcast? I haven't come across that myself except maybe UI Router, but UI Router is built to be a more powerful extension of ngRoute, which also does $rootScope.$broadcast to propagate changes in the DOM due to state change. Some application wide events on $scope should bubble downwards I think - for example, let's take a look at something internal to $scope, the A custom event bus could be nice, but you run into likely having to rewrite a lot of code. Maybe an amenable proposition is to have a method called |
Yep, ui-router and angular-translate for instance use it. You are right, the And further more, I noticed quite a lot people rolling their own event systems to be used with Angular. That clearly shows something is wrong I think. If the core team decides that a dedicated event bus should be used for event communication in an angular app, then that's fine with me but it should be something the framework provides. I personally don't think that it has to be a dedicated event bus as I see |
Does Anyways, I still sort of think the ideal solution would shrink code (by moving the common event dispatch code into its own routine) -- Like I think this is probably the cleanest approach with the current codebase --- It would simplify things, shrink code, and also provide a nice, simple means of targetting a single scope without bubbling. I think that's pretty cool. I'll work on a patch for this this morning just as an experiment --- But I don't really see |
@caitp indeed it does :) It automatically unsubscribes from the event when your local If you use I just paste this in here again:
|
Since @wesleycho came up with a good use case for Use Let's use a metaphor. Say you are the government and you want to notify some specific people that they have to pay additional taxes, then you write them a letter. If however, you want to raise taxes for everyone, then you just put it on the media to let everyone know without explicitly writing a letter to everyone since it's cheaper. You can extend the metaphor and say that if you are a very rich country (desktop ;-)) then you might just don't care enough and send a letter to everyone but if you are a poor country (mobile) then you definitely want to avoid sending letters to everyone if you want to inform everyone about it. Does that make sense to you? |
I like this proposal. I think that we should look at it in 1.3 |
Great to hear that you like it! I'll prepare a PR soon. |
This adds an $onRootScope method to the scope type. This allows developers to use $rootScope.$emit + $scope.$onRootScope as a fast eventBus which doesn't use scope bubbleing. Fixes angular#4574, Relates to angular#5371
👍 |
3 similar comments
+1 |
+1 |
+1 |
+1 ! |
+1. I come from a Flash/Flex background where two of our key frameworks - Swiz and RobotLegs - all used a central event dispatcher. It was key in building very large, flexible/modular/all that stuff apps. |
+1 |
I've incorporated your stack overflow snippet (http://stackoverflow.com/a/19498009/288703) into my app. Thanks! |
I have no "problems" with the current use of $emit and $broadcast, but I see how it can be more efficient when it's a "flat event bus". In a lot of the scenarios I have come across, a "flat event bus" is enough and I don't need bubbling. But if you're not aware of the lesser performance this is easily overlooked. tl;dr 👍 |
👍 |
👍 for a flat event bus that auto unbinds. |
Use cases for a shared event bus might help :)
|
Hi, why hasn't this been merged yet? |
+1 |
+1, I guess we'll have to implement manually for now: https://gist.github.com/turtlemonvh/10686980 |
@WhatFreshHellIsThis are you sure this works? I'm registering $on listeners before emitting events, but they do not receive them. |
@maximvl Yup, definitely, using it now all over the place. Not sure if the code is identical, I've done a lot since then. Not an Angular service / directive expert so I'm just going to dump everything relevant here that is working for your reference. Here are some working snippets taken directly out of a project: Here is the service from it's own file:
Referenced in the start page:
And of course you do not need to declare it in your common declarations, at least I don't:
Controller that receives messages snippet:
And the sender controller...
[update: just thought of something that might be giving you grief] You can't send a message to a controller that's not instantiated, i.e. from one view to another view that is not currently active, that might be your problem? Hope this helps. |
@WhatFreshHellIsThis thanks for detailed answer! I actually found that something like |
Sorry @maximvl I don't have the experience to help you with your follow up question. I'm using v1.2.16 at the moment and it's definitely working if that makes any difference. |
+1 |
Just tried 1.2.16, its $rootScope is changing too. Can anyone explain this? |
Here is a quick fiddle of how root changes and listerens disappear: http://jsfiddle.net/L7Hm5/1/ |
@maximvl you should use $injector instead angular.injector, like this: http://jsfiddle.net/L7Hm5/2/ |
Moving the discussion to #5507 |
Many people are afraid of using angulars event mechanism as an
EventBus
as they fear of performance issues because of it's bubbling behavior.However, those concerns don't apply if only
$rootScope.$emit
/$rootScope.$on
is used as there is no bubbling happening since $emit only bubbles upwards and there is no scope above the$rootScope
.I addressed this in my answer on the same thread.
In fact if eventing is used like that in an angular application than
$rootScope
is no different than the typicalEventBus
pattern. This comes with the drawback of manually having to unregister handlers from within controllers. Why that? Because controllers aren't singletons in Angular and therefore one needs to listen to the local$scope
's$destroy
event and then unregister from the event emitted by the$rootScope
.So controller code would look like this:
As stated in my SO post one can monkey patch the
$rootScope
to provide an alternative to it's$on
method that takes an additional parameter with an$scope
to listen for it's$destroy
event to then do the deregistration for us.With this in place we can subscribe to events emitted by the
$rootScope
and have them automatically deregistered when our local$scope
is being destroyed.We can use it like this:
However, I think we could do much better by providing a
$onRootScope
method directly on theScope
type so that it's available on every$scope
. This would then automatically make the deregistration for us but we wouldn't have to pass the$scope
explicitly.It would simply look like this.
As far as I know, I can't monkey patch that directly.
Shamelessly pulling @IgorMinar @mhevery @btford and @petebacondarwin and @matsko into this issue ;-)
The text was updated successfully, but these errors were encountered: