Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changed some return types #24

Closed
wants to merge 3 commits into from

Conversation

tswestendorp
Copy link

\Http\Client\HttpClient doesn't implement \Psr\Http\Client\ClientInterface in php-http/guzzle6-adapter ^1.1 which makes the package incompatible with recent versions of laravel/framework

\Http\Client\HttpClient doesn't implement \Psr\Http\Client\ClientInterface in php-http/guzzle6-adapter ^1.1
\Http\Client\HttpClient doesn't implement \Psr\Http\Client\ClientInterface in php-http/guzzle6-adapter ^1.1
@tswestendorp tswestendorp changed the title Removed return type from createClient() Changed some return types Feb 18, 2019
@DivineOmega
Copy link
Owner

Hi there @tswestendorp,

Thanks for your contribution, though I'd prefer to keep this following the PSR-18 Client Interface if possible. It is a new feature of the recent major release (v3.0.0). I think we may be able to find an alternative solution.

Could I ask you which version of Laravel you are using?

Could I also ask you to run composer why php-http/guzzle6-adapter and copy-paste the output here? We'll then be able to see why you have the older version of the Guzzle adapter installed.

Repository owner deleted a comment from coveralls Feb 18, 2019
Repository owner deleted a comment from coveralls Feb 18, 2019
@tswestendorp
Copy link
Author

tswestendorp commented Feb 18, 2019

Hi @DivineOmega,

Structure is as following

laravel/framework:5.7.26
    laravel/nexmo-notification-channel:^1.0
        nexmo/client:1.0
            php-http/guzzle6-adapter:^1.0

Output (ignore dev-master, I'm using my fork temporarily)

composer why php-http/guzzle6-adapter
divineomega/password_exposed  dev-master  requires  php-http/guzzle6-adapter (^1.1|^2.0)  
nexmo/client                  1.6.2       requires  php-http/guzzle6-adapter (^1.0)

@DivineOmega
Copy link
Owner

Thanks for the info @tswestendorp.

It seems this package should not indicate support for v1 of the guzzle6-adapter package, because the type of object returned differs between v1 and v2 of the guzzle6-adapter.

So, the Laravel incompatibility affects Laravel 5.7. It is caused by Laravel 5.7 requiring the laravel/nexmo-notification-channel as standard, which depends on the nexmo/client which enforces the use v1 of the guzzle6-adapter.

My plan of action is going to be:

  • Create a simple PSR18 Guzzle adapter package, that returns an object which implements the correct PSR18 client interface.
  • Modify this package to make use of the new PSR18 Guzzle adapter package.

@DivineOmega
Copy link
Owner

I'm going to close this PR now, but thanks very much for bringing this to my attention.

Further discussion will be occurring #25, so follow that issue for updates. If you need to use this in Laravel 5.7 immediately, feel free to install v2.8.0, which is not affected by this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants