Nice job! And FR: Swappable PSR-7-compliant HTTP client library

[simonhamp]

I came here to say great work on putting this together. It looks good and it's really nicely organised. I've only been reading code and haven't had a chance to try it out yet, but I feel like this could become a great package to make integrating with lots of APIs easier!

One thing I picked up on pretty quickly though is how dependent it is on Guzzle (I love Guzzle btw, so it's fine for me). I know it's early days for this package and I'm sure (/hope) you have plans in mind to make this not such a hard dependency as it feels quite useful for such a fundamental (and PSR-compliant) library to be easily replaceable.

To that end though, one thing that I spotted that will kind of make this harder is that the request config (and possibly other structures) is exposed directly to Guzzle: your SaloonRequest class (or more correctly, the CollectsConfig trait) gathers it all up and essentially passes it Guzzle unfiltered in your RequestManager.

It feels like a good goal would be to abstract this away so that Guzzle could be swapped out as needed.

Eventually you'd end up with a standard set of config options that work across HTTP client libraries. Even if your internal structure matches Guzzle's for convenience (and it may stay that way for a long time), you could introduce a transform layer between your structure and the HTTP client being used so that devs can use a consistent interface across libraries.

You and users of this package would be less susceptible to changes on Guzzle's side, for example if they suddenly removed an option, you could help maintain backwards compatibility for folks.

You could then also remove Guzzle as a dependency, which means fewer potential conflicts for people too.

[Sammyjo20]

Hi @simonhamp, thank you for your comments, I really appreciate it!

I'll start off with saying I really like this idea. I don't know a huge amount about PSR-7 (I think is the right PSR) but I do know that Guzzle follows it quite well. I'd like in the future to be able to make swappable clients for Saloon so you can easily build your own Saloon clients/adapters that it uses under the hood.

The main request flow for Saloon is as follows:

1. Request is created and sent method is called
2. Request goes to request manager
3. RequestManager builds up and merges headers, config, interceptors and handlers defined on both the request and the connector
4. RequestManager converts it to a Guzzle response
5. Guzzle sends the request
6. Saloon receives the response
7. Saloon sends you the response

I think Saloon could replace the RequestManager with a SaloonClient, with the default being GuzzleClient. Inside of the client, it would be responsible for building up the request, but it would have access to:

• All the merged headers, config, handlers and interceptors
• The request URL
• The ability to create a Saloon response

That way, you would be able to create any client you like and apply the headers/config/handlers in whichever way the client supports it. If a particular client didn't support middleware/handlers like Guzzle does, maybe you can specify that in the client so Saloon would know to disable that particular feature.

What are your thoughts?

I feel that this would take Saloon to the next level, from being a Guzzle wrapper to being the "Flysystem" of HTTP integrations. As exciting as this is, I'm happy to leave it dependant on Guzzle for the next few months at least, because it's really solid so far, and then as it matures I hope to use this client pattern. I've got a couple of things I want to build in Saloon first like mocking and caching. Once I've built that - I'll say Saloon is ready for version 1.0, after that I'll look into making it pluggable.

[simonhamp]

Totally agree! Only build it if/when it's needed.

Regardless, I'm looking forward to using this package soon

[Sammyjo20]

Sounds great! I’ll happily keep this issue open for now so others can know it’s on my mind.

Hope you have fun with it!

[Sammyjo20]

I will convert this into a discussion to move it away from the issues page 🚀

[Sammyjo20]

Hey @simonhamp

Just wanted to let you know that this work is currently in progress - I'm building this for v3 🥳

#199