Use ClientInterface instead of HttpClient interface when registering aliases
#394
Conversation
ruudk
changed the title
ClientInterface instead of HttpClient interfaceClientInterface instead of HttpClient interface when registering aliases
dbu
left a comment
dbu
left a comment
There was a problem hiding this comment.
thanks for the update and sorry for the late reply - i was on holidays last week.
i agree we should do this. however, i am a bit worried about creating false deprecations and wiring the psr interface if somebody manages to install this change along with the psr client but an old version of httplug.
| if (method_exists($container, 'registerAliasForArgument')) { | ||
| $container->registerAliasForArgument($serviceId, HttpClient::class, $clientName); | ||
| $alias = $container->registerAliasForArgument($serviceId, HttpClient::class, $clientName); | ||
| if (interface_exists(ClientInterface::class)) { |
There was a problem hiding this comment.
i think the existence of the psr interface is not the correct way to know which version of httplug we are using. we should additionally check if the httplug client implements the psr interface or something else that tells us if we are installed with a httplug version that implements the interface.
dbu
left a comment
dbu
left a comment
There was a problem hiding this comment.
thanks, this looks robust to me!
|
|
||
| $interfaces = class_implements(HttpClient::class) ?? []; | ||
| if (isset($interfaces[ClientInterface::class])) { | ||
| $alias->setDeprecated('php-http/httplug-bundle', '1.22', 'The "%alias_id%" alias is deprecated, use "Psr\Http\Client\ClientInterface" instead.'); |
There was a problem hiding this comment.
@ruudk somebody reported that symfony complains about the deprecation warning, saying it needs an %alias_id% (though i see that here)... https://twitter.com/PP_Vortex/status/1419599426084032519
can you please check if the deprecation warning works correctly for you and there is no symfony warning about alias_id?
There was a problem hiding this comment.
I did some deep dive into vendors and found out the following fact:
the method setDeprecated in Alias.php changed in symfony 5.3 compared to symfony 4.4:
https://github.com/symfony/dependency-injection/blob/5.3/Alias.php
https://github.com/symfony/dependency-injection/blob/4.4/Alias.php
In symfony 4.4, the second parameter is the template. So when using http plug bundle with symfony 4.4 executing assets:install interpretes this line as "deprecation template is '1.22'" and this string obviously doesn't contain %alias_id%.
$alias->setDeprecated('php-http/httplug-bundle', '1.22', 'The "%alias_id%" alias is deprecated, use "Psr\Http\Client\ClientInterface" instead.'); is fine for symfony 5.3 but not for symfony 4.4
My conclusion: Currently, php-http/httplug-bundle 1.22 is not compatible with symfony 4.4 but seems to be compatible with symfony 5.3
What would be the next steps here? Would you like to be compatible with both versions?
There was a problem hiding this comment.
I'll check it out and make a PR.
There was a problem hiding this comment.
thanks for reporting the issue and for the analysis @franziskahahn. the deprecation is removed in https://github.com/php-http/HttplugBundle/releases/tag/1.22.1
3 participants
What's in this PR?
After #393 was merged I started using it in my project and soon discovered that
HttpClientis deprecated and that theClientInterfacefrom Psr should be used instead.Why?
This will deprecate the alias registration for HttpClient when the ClientInterface is found.
It will also register the alias for the ClientInterface.