search

picqer / exact-php-client

PHP Client library for Exact Online
MIT License
165 stars 201 forks source link

PSR 18 compatibility #632

Open faizanakram99 opened 8 months ago

faizanakram99 commented 8 months ago

Hi,

Thank you for this package. I see that this package depends upon guzzlehttp/guzzle, while it is a very good package it adds one more dependency to the projects using this package. Can this depend upon psr18 instead. I think guzzle already implements it (https://packagist.org/providers/psr/http-client-implementation).

Depending upon psr http client would allow projects to use this package using http client of their choice and if a psr 18 client is already installed that will be used.

DannyvdSluijs commented 8 months ago

@faizanakram99 I would think that is impossible due to the fact this library uses a specific part of Guzzle, namely the middleware support (see https://github.com/picqer/exact-php-client/blob/main/src/Picqer/Financials/Exact/Connection.php#L115)

As far as I can recall this isn't part of the PSR-18 so specifically Guzzle is needed for this lib to work. Do you think differently or have more in depth knowledge about PSR-18 in combination with middleware, that is applicable to this context?

faizanakram99 commented 8 months ago

@faizanakram99 I would think that is impossible due to the fact this library uses a specific part of Guzzle, namely the middleware support (see https://github.com/picqer/exact-php-client/blob/main/src/Picqer/Financials/Exact/Connection.php#L115)

As far as I can recall this isn't part of the PSR-18 so specifically Guzzle is needed for this lib to work. Do you think differently or have more in depth knowledge about PSR-18 in combination with middleware, that is applicable to this context?

I don't see any usage of insertMiddleware in this package, it looks like it has been added just as a convenience method. The inserted middlewares are used before instantiating $client. You can already set a client via setClient method, and folks setting client can also set middleware beforehand, nothing needs to change there, it is just this package won't be exposing any convenience method specific to guzzle. The package can depend upon php-http/discovery instead https://docs.php-http.org/en/latest/httplug/library-developers.html

remkobrenters commented 8 months ago

Like Danny I am not familiar enough with the PSR-18 specs to know for sure in the end functionally nothing changes for people who use the middleware. It is a breaking change and I am not sure if removing the dependency on Guzzle is worth the effort. Is there someone else with more insights on this one to come up with a balanced opinion about this?