Migrating to PSR-18 Support #199
Replies: 7 comments 19 replies
-
Hey @aidan-casey just wanted to let you know that I’ve had a good read through your message and I’m going to think about it tonight. I’ll get back to you tomorrow! 😀 |
Beta Was this translation helpful? Give feedback.
-
Hey @aidan-casey This is an interesting proposal, so I can understand from your perspective, what is the why behind creating a PSR-18 compatible sender? I've had many people tell me "You should support PSR" but not many people explain why and most of the time it feels like it's "just because" and not give me any good examples but it would be good to fully understand and be in your shoes :) I would be interested in this - It would be great in the future if Saloon had even fewer dependencies but for the moment I would rather if the PSR sender was built as a separate library and then you can install the sender on a per-project basis when V3 comes around we can discuss moving the code into the core codebase or requiring the library in the main package and dropping Guzzle. This allows you to have your own versioning away from Saloon as well as let people install it without any breaking changes. You may use my package template if you want the same one that Saloon uses: https://github.com/Sammyjo20/package-template One of the biggest reasons I have stuck with Guzzle is because of its amazing feature list. You mentioned async requests but Guzzle comes with all the different options and the different stream wrappers are so useful - not to mention more complicated body types like multipart body are all supported. If we switched to the PSR sender being the primary one in v3 I would prefer if we matched the features that Guzzle had or better them (like making the internal codebase more modern, some of Guzzle still looks a bit old) Then that goes on to my next part which is - why rebuild Guzzle? If we built it to match the same number of features, eventually we're just going to have something very similar to Guzzle. Perhaps I'm not seeing something here. This is the conversation I've had in my head a few times when I had considered making a PSR client. Again, maybe you don't need all the features of Guzzle but if we were to make it the primary driver of v3 we would need to make sure it doesn't regress in functionality for most people as I think Saloon being built on top of Guzzle is a big selling point for people. One thing I do like though is that if we moved away from Guzzle, we could build our own standardised way to deal with sender config, headers, body and everything like that. Perhaps even Saloon can fill in on some of Guzzle's config so we don't have to implement it on every sender, like delay and of course request body. There are definitely many pros and cons. One final note to add - what are your thoughts on HTTPlug? Is this something that we should implement rather than PSR directly? |
Beta Was this translation helpful? Give feedback.
-
Just a side note on this - I wonder if it's worth creating a Saloon Github Org so we can move the packages away from |
Beta Was this translation helpful? Give feedback.
-
So I've been thinking about this whole PSR stuff and I think I've come up with something that could actually work really well, I'd love some feedback if you are happy to provide it. So let's say for v3 we wanted to migrate away from "senders" and the coupling to Guzzle directly. This is how the flow could work.
Here's a description of the flow
Also, where we have This is where people can apply Guzzle's config if they fancy, or even their own middleware. Like this: protected function resolveClient(): Client
{
$client = new Guzzle\Client([
'timeout' => 60,
'verify' => false, // Anything here
'handler' => ... // Custom handler stack
]);
return $client;
}
// Internally we'll do `$this->client ??= $this->resolveClient()` Then Saloon's configuration variables could become class-based and sexy like this: protected function defaultConfig(): array
{
return [
VerifyOption::create(false),
StreamRessponse::create(),
];
} Additionally, we could provide a hook on the request/connector to quickly change something with the PSR-7 request before it is sent. public function handlePsrRequest(Request $request): Request
{
$request->withHeaders();
// ...
return $request;
} Why keep
|
Beta Was this translation helpful? Give feedback.
-
Just to keep this discussion open, I'm currently researching about HttpPlug and how Saloon could use it. It would save a considerable amount of development time to rebuild certain things like streams, body handling etc and would mean that Saloon isn't tied to a specific implementation. Like with all things, as Saloon is my baby I'm going to do plenty of research and testing on it before I consider using it - but it does look good so far! |
Beta Was this translation helpful? Give feedback.
-
Hey everyone, so just an update from me over here. I've been having a fun long weekend here learning about PSR-7, PSR-17 and PSR-18 and I have actually managed to throw together a working prototype of a You can compare the changes I made here: v2...feature/psr-17-factories While at a surface level, it didn't require much to be changed - underneath the hood, I'd like to make a few changes to Saloon to improve the way this will be integrated - there are a few places that are still quite guzzle-y which will need to be migrated, but then for v3 we can remove Guzzle as a dependency completely, and Saloon can be HTTP client-agnostic! While I still believe that 90% of people using Saloon will want to continue using Guzzle, I think this is a great step forward because it'll allow Saloon to be free from implementation and be used by even more people. For those that will just want to get up and running, there will be a simple trait that they have to add to their connector like I've got a few things that I need to make sure I don't forget, all while wanting to keep Saloon's codebase as modern and tidy as possible. I'll keep this discussion updated, but here are some of the things I am thinking about: Building StreamsWe may need to require the multipart-stream-builder in order to build multipart Streams. The good news is that it requires a StreamFactory to be passed in. I don’t think it’s fair to install Providing Traits For DXWe could easily provide a <?php
class SpotifyConnector extends Connector
{
use Guzzle;
} Better way to Customise GuzzleWhen we switch to requiring a protected function resolveSender(): Sender
{
$sender = new GuzzleSender;
$sender->guzzle->addMiddleware(...);
return $sender;
} Forcing an async implementationWe should install something like Guzzle’s Promise library to force a Promise implementation. This makes our life even easier. Guzzle’s promise library doesn’t have any dependencies and it is the most used library, so I think we should go for this Building URI Interfaces instead of URLsGuzzle has a handy New toStream method on all body repositories which requires a StreamFactory to be providedThis allows us to keep all of the logic of converting body repositories into a stream. We could also implement an interface and if that interface does not exist, we could throw an exception - this will help us if someone has made their own body repository. Make sure we set the correct Content-Type based on the authenticatorWithout Guzzle, Saloon doesn’t automatically set the content type for all the body repositories. We should have a sensible default that gets set in the boot method and that way if someone wants a different content type, we can let them overwrite it. |
Beta Was this translation helpful? Give feedback.
-
I’ve been having another day of thinking and one thing I’m not a fan of is losing the ability to have configuration per-request for Guzzle, as well as the full asynchronous support that Guzzle provides so seamlessly. I do also think that there aren’t many HTTP clients out there that people still use. I’m fairly certain that Guzzle is the main HTTP client people reach for, with the Symfony Client being second on that list. All this work for potentially no benefit because people just install Guzzle anyway, and the downside is it makes saloon more generic and harder to install. Version 3 won’t come with Guzzle installed by default, BUT the GuzzleSender will be the default sender in Saloon’s core. This means for most people all you have to do is install Guzzle alongside Saloon and you’re done. Saloon can have Guzzle as a dev dependency to fully test this. Saloon will also ship with a PSR-18 sender which is completely generic which you have to install separately. This allows us to have lots of tests for it. this allows people to use any PSR-18 sender and then they install the sender that they like. All they have to do is either add the trait to “use Psr18Sender” or extend the “resolveSender” method. This provides us with the best of both worlds because Guzzle isn’t installed by default, but the simplest setup is just installing Guzzle. I’ll also make sure that Saloon’s core becomes more generic, with the ability to create PSR-7 requests and use PSR factories but it’ll use the sender to discover where those PSR-17 factories are. If you install the PSR-18 sender, then it will come with auto-discovery but you can provide all the factories separately you need. Thoughts guys? @aidan-casey @juse-less |
Beta Was this translation helpful? Give feedback.
-
Hey @Sammyjo20! I appreciate the work you have put into this project. Only one thing has really stopped my team from adopting its use in a few areas: lack of PSR-18 support.
I am currently working on a PR proposal that would allow any PSR-18 compatible client to work and simply require the
psr/http-client-implementation
andpsr/http-factory-implementation
virtual packages. These can be configured by the developer in the sender, of course, but in an environment where they are already installed (e.g., Laravel) they will be automatically discovered and used. Assuming it is agreeable, I would propose dropping the Guzzle requirement entirely in version 3 and making this the primary driver.There are a few challenges to this approach, however:
So I have a few questions before I continue to go too far down this path...
First, is this something that is even interesting to you as you develop the project? I had seen a previous discussion about this (#50) where you seemed interested, but I just wanted to confirm before too much work was put in.
Second, assuming it is an interesting addition, is are there any features that are utilizing Guzzle that I am missing? I would hate to go backward in functionality. 🙂
Beta Was this translation helpful? Give feedback.
All reactions