Support PSR-17 StreamFactoryInterface #56
Conversation
|
Build is failing because PHP 5.4 is still supported (unsupported |
|
I think you are right. The whole PHP-HTTP ecosystem is moving around the v2 so its time to deprecate some old PHP versions. Regarding that update, maybe supporting both HttplugFactory and PSR17Factory is not relevant anymore... PSR17 will be the de facto standard. |
|
not just "de facto" but "officially accepted" standard :-) afaik @Nyholm did a lot of thinking to have a clean upgrade path for the PSR, maybe he can chime in on this? |
deguif
force-pushed
the
psr17-compatibility
branch
from
16de5ac to
a282d11
Compare
|
Yes, let's wait for his comment(s). |
|
@Nyholm do you have input on this one? |
|
ping @Nyholm |
src/CachePlugin.php
Outdated
| { | ||
| if (!($streamFactory instanceof StreamFactory) && !($streamFactory instanceof StreamFactoryInterface)) { | ||
| throw new \LogicException(\sprintf('StreamFactory must be an instance of %s or %s', StreamFactory::class, StreamFactoryInterface::class)); |
There was a problem hiding this comment.
Why don't we throw a TypeError here?
There was a problem hiding this comment.
oh good idea. @deguif could you change this to throw a TypeError instead?
There was a problem hiding this comment.
Yes I can change this ;)
deguif
force-pushed
the
psr17-compatibility
branch
from
a282d11 to
f2b89be
Compare
src/CachePlugin.php
Outdated
| { | ||
| if (!($streamFactory instanceof StreamFactory) && !($streamFactory instanceof StreamFactoryInterface)) { | ||
| throw new \TypeError(\sprintf('StreamFactory must be an instance of %s or %s.', StreamFactory::class, StreamFactoryInterface::class)); |
There was a problem hiding this comment.
Should be:
throw new \TypeError(\sprintf('Argument 2 passed to %s::__construct() must be of type %s|%s, %s given.', self::class, StreamFactory::class, StreamFactoryInterface::class, gettype($streamFactory)));
To match the message given by PHP 8 for type errors with union types.
There was a problem hiding this comment.
Even better: \is_object($streamFactory) ? \get_class($streamFactory) : \gettype($streamFactory) for the given type ;)
deguif
force-pushed
the
psr17-compatibility
branch
from
f2b89be to
0a2115c
Compare
deguif
force-pushed
the
psr17-compatibility
branch
from
0a2115c to
a4e5617
Compare
| @@ -33,7 +34,7 @@ final class CachePlugin implements Plugin | |||
| private $pool; | |||
|
|
|||
| /** | |||
| * @var StreamFactory | |||
| * @var StreamFactory|StreamFactoryInterface | |||
There was a problem hiding this comment.
once php-http/message-factory#38 is released, we can raise our version constraint to get that version and only use the psr interface from then on.
There was a problem hiding this comment.
Yes, and let ping me at this moment, I will be glad to do the PR ;)
What's in this PR?
Support for PSR-17
StreamFactoryInterface