Skip to content

Deprecate returning values from event listeners.#18196

Merged
markstory merged 2 commits into
5.nextfrom
task/5.2-event-listener-return-value
Feb 22, 2025
Merged

Deprecate returning values from event listeners.#18196
markstory merged 2 commits into
5.nextfrom
task/5.2-event-listener-return-value

Conversation

@ADmad

@ADmad ADmad commented Feb 20, 2025

Copy link
Copy Markdown
Member

This allows callback methods to have a consistent return type void.

@ADmad ADmad added this to the 5.2.0 milestone Feb 20, 2025
@ADmad
ADmad force-pushed the task/5.2-event-listener-return-value branch from 6cb9cc9 to 5889222 Compare February 20, 2025 05:17
This allows callback methods to have a consistent return type `void`.
@ADmad
ADmad force-pushed the task/5.2-event-listener-return-value branch from 5889222 to 3bfec23 Compare February 20, 2025 05:24
@dereuromark

Copy link
Copy Markdown
Member

Refs #16786 and #16854

@ADmad
ADmad requested a review from markstory February 21, 2025 06:24

@markstory markstory left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this makes sense. It removes a redundant way to return values from events and abort them. Does the docstring for setResult() need to change to reflect that it now stops events?

@ADmad

ADmad commented Feb 21, 2025

Copy link
Copy Markdown
Member Author

Does the docstring for setResult() need to change to reflect that it now stops events?

It stops the propagation only if the result is false. I am not a fan of this behavior but that's how it works when you return false from a callback and we can't change it in 5.x.

ADmad added a commit to cakephp/docs that referenced this pull request Feb 22, 2025
LordSimal pushed a commit to cakephp/docs that referenced this pull request Feb 22, 2025
@markstory
markstory merged commit 27648e2 into 5.next Feb 22, 2025
@markstory
markstory deleted the task/5.2-event-listener-return-value branch February 22, 2025 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants