-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[EventDispatcher] Add resumePropagation to events #37556
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
Conversation
Thanks for proposing. I'm not seeing this as useful on my side. More real-world examples would be needed to get why this could be useful. The fact that this hasn't been proposed in years in a good hint to me that there are other ways. |
Changing the contracts class opens it for a mutation that was explicitly not intended before. I don't think that this would be a good idea. That being said, the |
Thanks for the feedback. I understand the point @derrabus makes that nobody expects an event to be resumable by now which can break behaviour. Although breaking behaviours can be good and has been made in the past I don't want to push this further the "BC is needed" point. TL;DR: I can't redispatch any event I'd like to and make sure its propagation state is set correctly to be dispatched with expected behaviour as it depends on the implementation. As I am not in control of the implementation I can't make it myself. For a better understanding I expand the TL;DR: |
To give a little context here: The event dispatcher is build around the assumption that the event is dispatched synchronously and that it has been processed completely once the dispatcher returns. This was one of the reasons why the Messenger component despite all similarities was not built around the event dispatcher, IIRC.
If you modify that flag via closure-binding and re-dispatch the modified object, you make the assumption that after flipping the flag, the object is in a valid state again. I don't think that this assumption is always correct – especially if (like in your case) we're dealing with error states. Adding a generic mutator won't solve this issue. Whoever implemented the event class you're dealing with, should imho be in control and decide if and how an event object can be dispatched again once the propagation has been stopped. |
Thank you very much @derrabus . That is some great input. Yes, this is done to have the synchronous event moved into an asynchronously flow to delay it. I keep in mind that the propagation state is as "volatile" as the other fields. Maybe a deep-clone/serialized copy should be kept in more secure place against changes. Regarding the discussion it looks like the wrong place to change something. I'll have a look how I can patch things in my code or at the framework. Sorry for taking your time 🙇 |
I'm closing as I agree with the others that this is not something we want to change. Thanks for proposing. |
It is useful when you want to dispatch an event twice but it has already been stopped by a listener. Current solution is to re-construct the event. As I am unsure about the acceptance I just publish this pull request for discussion. I will add tests and docs when this feature will be considered.