Skip to content

[EventDispatcher] Fix removing listeners when using first-class callable syntax #46262

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

Merged
merged 1 commit into from
May 5, 2022

Conversation

nicolas-grekas
Copy link
Member

@nicolas-grekas nicolas-grekas commented May 5, 2022

Q A
Branch? 4.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets Fix #46260
License MIT
Doc PR -

Closures should be compared using non-strict comparison to account for the first-class callable syntax.
See https://github.com/php/php-src/blob/9a90bd705483004c2ef408ee9e9bb0902beade3f/Zend/zend_closures.c#L387-L423

Copy link
Member

@chalasr chalasr left a comment

Choose a reason for hiding this comment

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

Great

@stof
Copy link
Member

stof commented May 5, 2022

as this is considered a bugfix, should we merge it into lower branches ? Projects might use the first-class callable syntax with Symfony 5.4 too

@nicolas-grekas
Copy link
Member Author

Indeed. Now targeting 4.4

@stof
Copy link
Member

stof commented May 5, 2022

@nicolas-grekas I think you also need to fix the TraceableEventDispatcher class as it also contains a few === $listener

@nicolas-grekas
Copy link
Member Author

Updated.

@nicolas-grekas nicolas-grekas force-pushed the ed-remove branch 2 times, most recently from cb5dafb to f2dd6c8 Compare May 5, 2022 15:29
@nicolas-grekas
Copy link
Member Author

nicolas-grekas commented May 5, 2022

This behavior is new to PHP 8.1: https://3v4l.org/Zr0m7
See php/php-src@6a9daaf
Updated to skip the tests on PHP < 8.1

@nicolas-grekas
Copy link
Member Author

Thank you @javer.

@nicolas-grekas nicolas-grekas merged commit b368ce1 into symfony:4.4 May 5, 2022
@nicolas-grekas nicolas-grekas deleted the ed-remove branch May 5, 2022 16:41
@fabpot fabpot mentioned this pull request May 14, 2022
This was referenced May 27, 2022
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.

5 participants