Decouple from "guzzlehttp/psr7" and "php-http/discovery", replace by "friendsofphp/well-known-implementations" #1418
Conversation
…"friendsofphp/well-known-implementations"
nicolas-grekas
force-pushed
the
well-known
branch
from
b707fc1
to
efbbb4a
Compare
| @@ -89,6 +89,7 @@ | |||
| "sort-packages": true, | |||
| "allow-plugins": { | |||
| "composer/package-versions-deprecated": true, | |||
| "friendsofphp/well-known-implementations": false, | |||
There was a problem hiding this comment.
Running the plugin is not needed for tests. Only the "lib" part of the package is used when running the CI.
| Psr17FactoryDiscovery::findResponseFactory(), | ||
| $streamFactory, | ||
| null, | ||
| null, |
There was a problem hiding this comment.
The first two arguments of HttpClientFactory are never used. We can thus make them nullable and skip injecting factories there.
| UriFactoryInterface $uriFactory, | ||
| ResponseFactoryInterface $responseFactory, | ||
| ?UriFactoryInterface $uriFactory, | ||
| ?ResponseFactoryInterface $responseFactory, |
There was a problem hiding this comment.
Those arguments are never used, no need to force passing anything.
| @@ -110,7 +111,7 @@ public function create(Options $options): HttpAsyncClientInterface | |||
| */ | |||
| private function resolveClient(Options $options) | |||
| { | |||
| if (class_exists(SymfonyHttplugClient::class)) { | |||
| if (ConcreteImplementation::VENDOR_SYMFONY === ConcreteImplementation::HTTPLUG_VENDOR) { | |||
There was a problem hiding this comment.
Using these consts gives more control over which implementation should be used (the consts are generated by the plugin xor computed at autoload-time when the plugin is disabled)
| @@ -22,6 +22,6 @@ public function fetchRequest(): ?ServerRequestInterface | |||
| return null; | |||
| } | |||
|
|
|||
| return ServerRequest::fromGlobals(); | |||
| return (new WellKnownPsr17Factory())->createServerRequestFromGlobals(); | |||
There was a problem hiding this comment.
This removes the coupling to guzzlehttp/psr7: "well-known-implem" provides the same feature, but compatible with any PSR-7 implementations.
| @@ -12,8 +10,6 @@ | |||
|
|
|||
| require_once __DIR__ . '/../vendor/autoload.php'; | |||
|
|
|||
| ClassDiscovery::appendStrategy(MockClientStrategy::class); | |||
There was a problem hiding this comment.
"well-known-implem" will discover and auto-load php-http/mock-client when no other implem is found.
|
Thanks for proposing this @nicolas-grekas This is definitely something we will consider when making a decision on the path forward in the next major version of the SDK. |
|
After talking to @nicolas-grekas at SymfonyCon, I'm going to close this. |
|
@cleptric here is the result of what we talked about at SymfonyCon: php-http/discovery#208 |
Related to #1411 and FriendsOfPHP/well-known-implementations#2
In this PR, I propose to rely on FriendsOfPHP/well-known-implementations to ensure a working out-of-the-box experience. This package is a composer-plugin that will install the most suited
async-http-implementationdepending on the existing dependencies of the projects that require sentry-php.The package also provides a discovery mechanism that covers roughly the same purpose as
php-http/discovery, but that plays well with the plugin part of the package.The attached patch removes
php-http/discoveryfrom the mandatory dependencies, but also removes the dependency onguzzlehttp/psr7.