Feat/event sender#19
Merged
Merged
Conversation
Instead of manually checking the flag in each handler.
There was a problem hiding this comment.
Pull request overview
This PR refines the static event system to better support instance-scoped listeners (via object senders) and centralizes “handled” short-circuiting inside the dispatcher, addressing previously confusing/incorrect behaviors around sender identity and cross-instance event delivery.
Changes:
- Add support for passing an object as the
$senderinEvents::trigger()/Events::on()/Events::off()to enable instance-scoped listener registration. - Stop dispatching additional handlers once an
Eventmarks itself ashandled. - Update tests to reflect the new handled semantics and to verify per-instance sender behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/Events.php |
Extends sender handling to accept objects, sets sender on events, and stops dispatch after handled; updates listener registration to support instance-scoped filtering. |
src/EventableTrait.php |
Adjusts instance helper methods to register listeners against the instance and restrict triggering against unrelated classes. |
tests/EventTest.php |
Updates handled expectations and adds coverage for instance-specific sender behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Although Event is abstract, this is valid and would work:
Events::on($instance, SomeEvent::class, function(Event $event) {});
Events::trigger($instance, new SomeEvent());
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The event system was a little rushed when we first wrote it. This polishes a few part of the events system that should have been done long ago.
Events are largely emitted from static interfaces, which makes the
senderproperty a bit weird and theEventableTraitrather misleading for a few behaviours. In particularly using$inst->on(...)would receive events from other instances which is very odd (and annoying).Events could also be triggered for other classes by using
$this->trigger(OtherClass::class)which is clearly incorrect usage.The
handledproperty was odd. It's baked into the base event class but requires each handler to check? Very silly. This is probably the most likely breaking change in this PR.Whereas the rest of the changes are bugfixes because no-one should be expecting those behaviours to work as they were. If their workflow break? Well good.