php-http / message

HTTP Message related tools
http://php-http.org
MIT License
1.29k stars 41 forks source link

PSR-17 compatibility ? #105

Closed Taluu closed 4 years ago

Taluu commented 5 years ago
Q A
Bug? no (not really)
New Feature? yes (kinda ?)
Version latest

Actual Behavior

Seems that this package doesn't implement the psr/http-factory interfaces, and thus does not provide psr/http-factory-implementation.

Expected Behavior

To implement psr-17 interfaces instead of php-http's

Possible Solutions

Use the new interfaces from PSR-17, but as it would probably introduce BC breaks (as the psr are php 7.0+, and uses return typehints), it should be on a new major.

sagikazarmark commented 5 years ago

Most probably message factories in this package will be deprecated in favor of official, PSR-17 ones provided by the original PSR-7 packages.

dbu commented 5 years ago

@sagikazarmark should we add deprecation notices to our factories, or straight remove them for version 2? or should we make our factories implement PSR-17, or have a PSR-17 adapter that takes a PSR-17 factory and implements our factory interface?

and should we rewrite discovery to return PSR-17 factories?

sagikazarmark commented 5 years ago

If we can make out factories implement PSR-17 that would be the best. Current consuming code wouldn't break and we can move to PSR-17 factories in our code. I'm not sure though if we can implement PSR-17.

In any case, I think every 2.x package should switch to PSR-17 factories.

As for 1.x packages: we should improve discovery so that it can detect PSR-17 implementations and return a factory with our compatibility layer added in php-http/message-factory#38. That removes the need for installing the message package for the sake of having a message factory. But I consider that a low priority given we want everyone use 2.x code and we can't remove factories from the message package without a major version bump anyway.

Eventually we should deprecate our factory interfaces and implementations entirely.

Nyholm commented 5 years ago

Note: We don't provide psr/http-factory-implementation. We provide php-http/http-factory-implementation.

We cannot be compatible with PSR-17 because of PHP7 return types. We could: A) Bump to version 2.0 and drop PHP5 in order to make our factories compatible with PSR17 B) Deprecate them and dont plan for a 2.0 release.

I vote for B because our factories does not serve great purpose anymore. They have been for 4 years but we succeeded and fig has created PSR-17.

dbu commented 5 years ago

okay, then lets deprecate our factories completely and remove them in 2.0.

for discovery, there is https://github.com/php-http/discovery/issues/116 btw