Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

[EventDispatcher] Improve method resolving when it is omitted in tag#50783

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
nikophil wants to merge1 commit intosymfony:7.4
base:7.4
Choose a base branch
Loading
fromnikophil:feat/resolve-listener-method

Conversation

nikophil
Copy link
Contributor

@nikophilnikophil commentedJun 26, 2023•
edited
Loading

QA
Branch?6.4
Bug fix?no
New feature?yes
Deprecations?no
LicenseMIT
Doc PRwill be done if bugfix gets accepted

(this PR is a split of#50775)

this PR proposes to improve the listener's method resolution by:

  1. checking if a method with the "camelized" name exist.
  2. If it does not, we evaluate if one unique public method exists which would take the event as unique parameter (ie: the listener definition above would resolvesomeMethod() as the listener's method).
  3. if an__invoke method exist we'll use this one without checking its parameters (this is also useful for BC)
  4. Otherwise, method name needs to be provided and an exception is thrown

ping@GromNaN

(CI failures seems unrelated to this PR 🤔 )

Copy link
Member

@nicolas-grekasnicolas-grekas left a comment

Choose a reason for hiding this comment

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

LGTM, Here are minor comments. Please rebase while addressing them.

nikophil reacted with thumbs up emoji
@nikophilnikophilforce-pushed thefeat/resolve-listener-method branch fromf2bcee9 tod2ebe37CompareAugust 29, 2024 07:32
@nikophilnikophilforce-pushed thefeat/resolve-listener-method branch fromd2ebe37 todcefe70CompareAugust 29, 2024 07:33
foreach ($publicMethods as $publicMethod) {
if (
!$publicMethod->isStatic()
&& 1 === $publicMethod->getNumberOfRequiredParameters()
Copy link
Member

Choose a reason for hiding this comment

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

This check is bad given that the EventDispatcher component actually provides 3 parameters to listeners:

  • the event object
  • the event name
  • the EventDispatcher instance

@fabpotfabpot modified the milestones:7.2,7.3Nov 20, 2024
@chalasr
Copy link
Member

@nikophil any chance you handle the remaining comments?

nikophil reacted with thumbs up emoji

@fabpotfabpot modified the milestones:7.3,7.4May 26, 2025
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@nicolas-grekasnicolas-grekasnicolas-grekas left review comments

@stofstofstof left review comments

Assignees
No one assigned
Projects
None yet
Milestone
7.4
Development

Successfully merging this pull request may close these issues.

7 participants
@nikophil@chalasr@nicolas-grekas@stof@fabpot@xabbuh@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp