Allow to register PSR17 and PSR18 classes as services #372
Conversation
dbu
left a comment
dbu
left a comment
There was a problem hiding this comment.
i am a bit worried of regressions for users that don't have a psr17 implementation installed. did i mis-think this or could this explode for some people?
| <argument type="service" id="httplug.message_factory"/> | ||
| <argument type="service" id="httplug.stream_factory"/> | ||
| <argument type="service" id="httplug.psr17_response_factory"/> | ||
| <argument type="service" id="httplug.psr17_stream_factory"/> |
There was a problem hiding this comment.
won't this lead to regressions for users that have no psr17 implementation available?
the factories throw exception when no implementation is present. should we have a compiler pass that checks if the factories find anything and removes the services if they don't?
There was a problem hiding this comment.
That is very true. I have to fix that.
There was a problem hiding this comment.
I just tested in Symfony 5.
It is not true, we are only using the factory if we are actually trying to use the service. So this is fine.
There was a problem hiding this comment.
but are we only trying to use the service when a psr-17 implementation is available?
There was a problem hiding this comment.
Ie, the curl client is using psr17. It is currently broken.
So if you use the httplug.factory.curl, then we will use these psr17 factory services.
There is nothing that is using these services out of the box.
We need a way to support PSR17 factories. Our CurlClient 2.0 does not use the deprecated php-http/message-factory.