Add LegacyEventDispatcherProxy for BC support #1537
Conversation
|
@j-mywheels maybe to avoid BC you should add if (!class_exists(Symfony\Component\EventDispatcher\LegacyEventDispatcherProxy::class))and use old syntax. What do you think about this solution? |
Yes would be possible to do. I will look at it at the end of the day. Thanks for your feedback @giovannialbero1992 |
|
Done @giovannialbero1992 should work now, tested on SF 3.4 and 4. |
|
@j-mywheels yeah :) ... You could rebase in one commit |
|
Nice job @j-mywheels :) |
|
@j-mywheels thanks for your work! |
|
I pulled the new master for my Symfony installation, but I'm still getting the same error: |
|
Have you done a composer update?
Il giorno mar 25 giu 2019 alle 20:48 Bo Roth <notifications@github.com> ha
scritto:
I pulled the new master for my Symfony installation, but I'm still getting
the same error:
[
"exception" => ErrorException {
#message: "User Deprecated: Calling the "Symfony\Component\EventDispatcher\EventDispatcherInterface::dispatch()" method with the event name as first argument is deprecated since Symfony 4.3, pass it second and provide the event object first instead."
#code: 0
#file: "./vendor/symfony/event-dispatcher/Debug/TraceableEventDispatcher.php"
#line: 148
#severity: E_USER_DEPRECATED
trace: {
./vendor/symfony/event-dispatcher/Debug/TraceableEventDispatcher.php:148 { …}
./vendor/friendsofsymfony/elastica-bundle/src/Persister/InPlacePagerPersister.php:78 { …}
./vendor/enqueue/elastica-bundle/Queue/PopulateProcessor.php:67 { …}
./vendor/enqueue/enqueue/Client/DelegateProcessor.php:38 { …}
./vendor/enqueue/enqueue/Consumption/QueueConsumer.php:197 { …}
Enqueue\Consumption\QueueConsumer->Enqueue\Consumption\{closure}() {}
./vendor/enqueue/enqueue/Consumption/FallbackSubscriptionConsumer.php:47 { …}
./vendor/enqueue/enqueue/Consumption/QueueConsumer.php:264 { …}
./vendor/enqueue/enqueue/Symfony/Client/ConsumeCommand.php:148 { …}
./vendor/symfony/console/Command/Command.php:255 { …}
./vendor/symfony/console/Application.php:939 { …}
./vendor/symfony/framework-bundle/Console/Application.php:87 { …}
./vendor/symfony/console/Application.php:273 { …}
./vendor/symfony/framework-bundle/Console/Application.php:73 { …}
./vendor/symfony/console/Application.php:149 { …}
./bin/console:42 {
› $application = new Application($kernel);
› $application->run($input);
›
}
}
}
]
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1537?email_source=notifications&email_token=ACYAUUI6CCASNYX5GECN32TP4JSADA5CNFSM4H27S4D2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYRHBZQ#issuecomment-505573606>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ACYAUUMBPEIFID6FIOQX653P4JSADANCNFSM4H27S4DQ>
.
--
*Giovanni Albero*
+393661860916
giovannialbero.solinf@gmail.com
full-stack developer
Skype: giovanni.albero
Linkedin: https://it.linkedin.com/pub/giovanni-albero/64/465/bba
Twitter: @albero92
|
|
Yeah, I had to switch my composer.json back to using Symfony 4.3, and then ran composer update. It seems like I have all the right vesrions:
I forgot that originally I did have to fork the enqueue/enqueue-bundle to make adjustments to their ->dispatch() calls, so maybe I need to do the same updates there with the LegacyEventDispatcherProxy for enqueue-bundle and use my forked version instead? I'm on the most recent dev branch for 0.9.* at the moment. I thought the decorate would handle that for us, and I really like the solution. I can tell that decorate() is being run, so I'm not sure why it's giving me the error. I'm also gonna try restarting my computer just incase something really weird is going on with my development environment/VMs. I'll have another update shortly. |
|
I'm not even sure what's going on anymore, but I need to move on so I guess I'm just going to have to run the populate command without the enqueue consumers. I'm not sure where it broke, or how, but even after going back to the older version of elastica-bundle, my consumers now show no output when I start them, and they aren't monitoring the queue. I realize this is outside the scope of this Issue/PR, so I'll bring this up with the enqueue-bundle team at a later time when I have more time to look into it. Thanks for your help! |
Thanks for your feedback. Keep me updated, i tried multiple installations with the PR and worked fine. If you have time you can create an example project? So we can have a look. |
|
It doesn't seem like PopulateCommand is calling dispatcher using the new format. That is, the LegacyEventDispatcherProxy exists and is being called, but the calls haven't been updated. I'll create a PR for at least that command and InPlacePersister. |
|
I am using |
|
Bumping this cause I get the same deprecation error on master. From my understanding, the point of the LegacyEventDispatcherProxy is to allow you to update your code to the Symfony 4.3 syntax, while still having backward compatibility. If you agree with this, I can probably make a PR to update the syntax |
|
I think its exactly this and agree with reworking the code to the current state of the art. |
|
@chypriote any news on preparing a PR? |
|
I was hoping to get a comment from @XWB or @giovannialbero1992 before working on it |
|
Hi @chypriote, yes i believe that it is useful an update on dispatch methods. |
|
I think this: public function __construct(EventDispatcherInterface $dispatcher)
{
$this->dispatcher = $dispatcher;
if (class_exists(LegacyEventDispatcherProxy::class)) {
$this->dispatcher = LegacyEventDispatcherProxy::decorate($dispatcher);
}
}Should be replaced by this: public function __construct(EventDispatcherInterface $dispatcher)
{
if (class_exists(LegacyEventDispatcherProxy::class)) {
$this->dispatcher = LegacyEventDispatcherProxy::decorate($dispatcher);
} else {
$this->dispatcher = $dispatcher;
}
} |
Fixes #1530 ?
See: #1531 (comment)